qapi: block-stream: add "bottom" argument

The code already don't freeze base node and we try to make it prepared
for the situation when base node is changed during the operation. In
other words, block-stream doesn't own base node.

Let's introduce a new interface which should replace the current one,
which will in better relations with the code. Specifying bottom node
instead of base, and requiring it to be non-filter gives us the
following benefits:

 - drop difference between above_base and base_overlay, which will be
   renamed to just bottom, when old interface dropped

 - clean way to work with parallel streams/commits on the same backing
   chain, which otherwise become a problem when we introduce a filter
   for stream job

 - cleaner interface. Nobody will surprised the fact that base node may
   disappear during block-stream, when there is no word about "base" in
   the interface.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20201216061703.70908-11-vsementsov@virtuozzo.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
This commit is contained in:
Vladimir Sementsov-Ogievskiy 2020-12-16 09:17:00 +03:00 committed by Max Reitz
parent 000e5a1cda
commit 7f4a396d76
5 changed files with 94 additions and 31 deletions

View File

@ -507,7 +507,8 @@ void hmp_block_stream(Monitor *mon, const QDict *qdict)
int64_t speed = qdict_get_try_int(qdict, "speed", 0); int64_t speed = qdict_get_try_int(qdict, "speed", 0);
qmp_block_stream(true, device, device, base != NULL, base, false, NULL, qmp_block_stream(true, device, device, base != NULL, base, false, NULL,
false, NULL, qdict_haskey(qdict, "speed"), speed, true, false, NULL, false, NULL,
qdict_haskey(qdict, "speed"), speed, true,
BLOCKDEV_ON_ERROR_REPORT, false, NULL, false, false, false, BLOCKDEV_ON_ERROR_REPORT, false, NULL, false, false, false,
false, &error); false, &error);

View File

@ -221,6 +221,7 @@ static const BlockJobDriver stream_job_driver = {
void stream_start(const char *job_id, BlockDriverState *bs, void stream_start(const char *job_id, BlockDriverState *bs,
BlockDriverState *base, const char *backing_file_str, BlockDriverState *base, const char *backing_file_str,
BlockDriverState *bottom,
int creation_flags, int64_t speed, int creation_flags, int64_t speed,
BlockdevOnError on_error, BlockdevOnError on_error,
const char *filter_node_name, const char *filter_node_name,
@ -230,25 +231,42 @@ void stream_start(const char *job_id, BlockDriverState *bs,
BlockDriverState *iter; BlockDriverState *iter;
bool bs_read_only; bool bs_read_only;
int basic_flags = BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED; int basic_flags = BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED;
BlockDriverState *base_overlay = bdrv_find_overlay(bs, base); BlockDriverState *base_overlay;
BlockDriverState *above_base; BlockDriverState *above_base;
if (!base_overlay) { assert(!(base && bottom));
error_setg(errp, "'%s' is not in the backing chain of '%s'", assert(!(backing_file_str && bottom));
base->node_name, bs->node_name);
return;
}
/* if (bottom) {
* Find the node directly above @base. @base_overlay is a COW overlay, so /*
* it must have a bdrv_cow_child(), but it is the immediate overlay of * New simple interface. The code is written in terms of old interface
* @base, so between the two there can only be filters. * with @base parameter (still, it doesn't freeze link to base, so in
*/ * this mean old code is correct for new interface). So, for now, just
above_base = base_overlay; * emulate base_overlay and above_base. Still, when old interface
if (bdrv_cow_bs(above_base) != base) { * finally removed, we should refactor code to use only "bottom", but
above_base = bdrv_cow_bs(above_base); * not "*base*" things.
while (bdrv_filter_bs(above_base) != base) { */
above_base = bdrv_filter_bs(above_base); assert(!bottom->drv->is_filter);
base_overlay = above_base = bottom;
} else {
base_overlay = bdrv_find_overlay(bs, base);
if (!base_overlay) {
error_setg(errp, "'%s' is not in the backing chain of '%s'",
base->node_name, bs->node_name);
return;
}
/*
* Find the node directly above @base. @base_overlay is a COW overlay,
* so it must have a bdrv_cow_child(), but it is the immediate overlay
* of @base, so between the two there can only be filters.
*/
above_base = base_overlay;
if (bdrv_cow_bs(above_base) != base) {
above_base = bdrv_cow_bs(above_base);
while (bdrv_filter_bs(above_base) != base) {
above_base = bdrv_filter_bs(above_base);
}
} }
} }

View File

@ -2500,6 +2500,7 @@ void qmp_block_stream(bool has_job_id, const char *job_id, const char *device,
bool has_base, const char *base, bool has_base, const char *base,
bool has_base_node, const char *base_node, bool has_base_node, const char *base_node,
bool has_backing_file, const char *backing_file, bool has_backing_file, const char *backing_file,
bool has_bottom, const char *bottom,
bool has_speed, int64_t speed, bool has_speed, int64_t speed,
bool has_on_error, BlockdevOnError on_error, bool has_on_error, BlockdevOnError on_error,
bool has_filter_node_name, const char *filter_node_name, bool has_filter_node_name, const char *filter_node_name,
@ -2507,12 +2508,31 @@ void qmp_block_stream(bool has_job_id, const char *job_id, const char *device,
bool has_auto_dismiss, bool auto_dismiss, bool has_auto_dismiss, bool auto_dismiss,
Error **errp) Error **errp)
{ {
BlockDriverState *bs, *iter; BlockDriverState *bs, *iter, *iter_end;
BlockDriverState *base_bs = NULL; BlockDriverState *base_bs = NULL;
BlockDriverState *bottom_bs = NULL;
AioContext *aio_context; AioContext *aio_context;
Error *local_err = NULL; Error *local_err = NULL;
int job_flags = JOB_DEFAULT; int job_flags = JOB_DEFAULT;
if (has_base && has_base_node) {
error_setg(errp, "'base' and 'base-node' cannot be specified "
"at the same time");
return;
}
if (has_base && has_bottom) {
error_setg(errp, "'base' and 'bottom' cannot be specified "
"at the same time");
return;
}
if (has_bottom && has_base_node) {
error_setg(errp, "'bottom' and 'base-node' cannot be specified "
"at the same time");
return;
}
if (!has_on_error) { if (!has_on_error) {
on_error = BLOCKDEV_ON_ERROR_REPORT; on_error = BLOCKDEV_ON_ERROR_REPORT;
} }
@ -2525,12 +2545,6 @@ void qmp_block_stream(bool has_job_id, const char *job_id, const char *device,
aio_context = bdrv_get_aio_context(bs); aio_context = bdrv_get_aio_context(bs);
aio_context_acquire(aio_context); aio_context_acquire(aio_context);
if (has_base && has_base_node) {
error_setg(errp, "'base' and 'base-node' cannot be specified "
"at the same time");
goto out;
}
if (has_base) { if (has_base) {
base_bs = bdrv_find_backing_image(bs, base); base_bs = bdrv_find_backing_image(bs, base);
if (base_bs == NULL) { if (base_bs == NULL) {
@ -2554,8 +2568,33 @@ void qmp_block_stream(bool has_job_id, const char *job_id, const char *device,
bdrv_refresh_filename(base_bs); bdrv_refresh_filename(base_bs);
} }
/* Check for op blockers in the whole chain between bs and base */ if (has_bottom) {
for (iter = bs; iter && iter != base_bs; bottom_bs = bdrv_lookup_bs(NULL, bottom, errp);
if (!bottom_bs) {
goto out;
}
if (!bottom_bs->drv) {
error_setg(errp, "Node '%s' is not open", bottom);
goto out;
}
if (bottom_bs->drv->is_filter) {
error_setg(errp, "Node '%s' is a filter, use a non-filter node "
"as 'bottom'", bottom);
goto out;
}
if (!bdrv_chain_contains(bs, bottom_bs)) {
error_setg(errp, "Node '%s' is not in a chain starting from '%s'",
bottom, device);
goto out;
}
assert(bdrv_get_aio_context(bottom_bs) == aio_context);
}
/*
* Check for op blockers in the whole chain between bs and base (or bottom)
*/
iter_end = has_bottom ? bdrv_filter_or_cow_bs(bottom_bs) : base_bs;
for (iter = bs; iter && iter != iter_end;
iter = bdrv_filter_or_cow_bs(iter)) iter = bdrv_filter_or_cow_bs(iter))
{ {
if (bdrv_op_is_blocked(iter, BLOCK_OP_TYPE_STREAM, errp)) { if (bdrv_op_is_blocked(iter, BLOCK_OP_TYPE_STREAM, errp)) {
@ -2579,7 +2618,7 @@ void qmp_block_stream(bool has_job_id, const char *job_id, const char *device,
} }
stream_start(has_job_id ? job_id : NULL, bs, base_bs, backing_file, stream_start(has_job_id ? job_id : NULL, bs, base_bs, backing_file,
job_flags, has_speed ? speed : 0, on_error, bottom_bs, job_flags, has_speed ? speed : 0, on_error,
filter_node_name, &local_err); filter_node_name, &local_err);
if (local_err) { if (local_err) {
error_propagate(errp, local_err); error_propagate(errp, local_err);

View File

@ -1161,6 +1161,7 @@ int is_windows_drive(const char *filename);
*/ */
void stream_start(const char *job_id, BlockDriverState *bs, void stream_start(const char *job_id, BlockDriverState *bs,
BlockDriverState *base, const char *backing_file_str, BlockDriverState *base, const char *backing_file_str,
BlockDriverState *bottom,
int creation_flags, int64_t speed, int creation_flags, int64_t speed,
BlockdevOnError on_error, BlockdevOnError on_error,
const char *filter_node_name, const char *filter_node_name,

View File

@ -2517,10 +2517,14 @@
# @device: the device or node name of the top image # @device: the device or node name of the top image
# #
# @base: the common backing file name. # @base: the common backing file name.
# It cannot be set if @base-node is also set. # It cannot be set if @base-node or @bottom is also set.
# #
# @base-node: the node name of the backing file. # @base-node: the node name of the backing file.
# It cannot be set if @base is also set. (Since 2.8) # It cannot be set if @base or @bottom is also set. (Since 2.8)
#
# @bottom: the last node in the chain that should be streamed into
# top. It cannot be set if @base or @base-node is also set.
# It cannot be filter node. (Since 6.0)
# #
# @backing-file: The backing file string to write into the top # @backing-file: The backing file string to write into the top
# image. This filename is not validated. # image. This filename is not validated.
@ -2576,8 +2580,8 @@
## ##
{ 'command': 'block-stream', { 'command': 'block-stream',
'data': { '*job-id': 'str', 'device': 'str', '*base': 'str', 'data': { '*job-id': 'str', 'device': 'str', '*base': 'str',
'*base-node': 'str', '*backing-file': 'str', '*speed': 'int', '*base-node': 'str', '*backing-file': 'str', '*bottom': 'str',
'*on-error': 'BlockdevOnError', '*speed': 'int', '*on-error': 'BlockdevOnError',
'*filter-node-name': 'str', '*filter-node-name': 'str',
'*auto-finalize': 'bool', '*auto-dismiss': 'bool' } } '*auto-finalize': 'bool', '*auto-dismiss': 'bool' } }