Use node-names for commit, stream
v7 cover Use node-names for commit, stream
This is part 3 of the series "Modify block jobs to use node-names"
This has the auto-generated node names, and the addition of node-names to block-commit and block-stream.
This maintains the same blocker convention that is in master, i.e. checking the active layer blocker for commit.
Jeff Cody (4):
block: Auto-generate node_names for each BDS entry
block: Accept node-name arguments for block-commit
block: add ability for block-stream to use node-name
block: Add QMP documentation for block-stream
block.c | 16 ++++++++- blockdev.c | 97 +++++++++++++++++++++++++++++++++++++++++++--------- hmp.c | 3 +- qapi/block-core.json | 57 ++++++++++++++++++++++-------- qmp-commands.hx | 91 +++++++++++++++++++++++++++++++++++++++++++----- 5 files changed, 223 insertions(+), 41 deletions(-)
Signature · 1 line
1.9.3
PATCH v7 1/4 block: Auto-generate node_names for each BDS entry
Currently, node_name is only filled in when done so explicitly by the user. If no node_name is specified, then the node name field is not populated.
If node_names are automatically generated when not specified, that means that all block job operations can be done by reference to the unique node_name field. This eliminates ambiguity in resolving filenames (relative filenames, or file descriptors, symlinks, mounts, etc..) that qemu currently needs to deal with.
If a node name is specified, then it will not be automatically generated for that BDS entry.
If it is automatically generated, it will be prefaced with "__qemu##", followed by 8 characters of a unique number, followed by 8 random ASCII characters in the range of 'A-Z'. Some sample generated node-name strings:
__qemu##00000000IAIYNXXR
__qemu##00000002METXTRBQ
__qemu##00000001FMBORDWGThe prefix is to aid in identifying it as a qemu-generated name, the numeric portion is to guarantee uniqueness in a given qemu session, and the random characters are to further avoid any accidental collisions with user-specified node-names.
Reviewed-by Eric Blake <[email protected]>
Signed-off-by Jeff Cody <[email protected]>
--- block.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)
block.c +15 -1
@@ -843,12 +843,26 @@ static int bdrv_open_flags(BlockDriverState *bs, int flags)return open_flags; } +#define GEN_NODE_NAME_PREFIX "__qemu##" +#define GEN_NODE_NAME_MAX_LEN (sizeof(GEN_NODE_NAME_PREFIX) + 8 + 8)static void bdrv_assign_node_name (BlockDriverState *bs,const char *node_name,Error **errp) { +char gen_node_name[GEN_NODE_NAME_MAX_LEN ]; +static uint32_t counter;/* simple counter to guarantee uniqueness */ + +/* if node_name is NULL, auto-generate a node name */ if (!node_name) { -return ; +int len; +snprintf (gen_node_name,GEN_NODE_NAME_MAX_LEN , +"%s%08x" ,GEN_NODE_NAME_PREFIX , counter++); + len =strlen (gen_node_name); +while (len <GEN_NODE_NAME_MAX_LEN -1 ) { + gen_node_name[len++] =g_random_int_range ('A' ,'Z' ); + } + gen_node_name[GEN_NODE_NAME_MAX_LEN -1 ] ='\0' ; + node_name = gen_node_name; }/* empty string node name is invalid */
Signature · 1 line
1.9.3
PATCH v7 2/4 block: Accept node-name arguments for block-commit
In reply to Jeff CodyThis modifies the block operation block-commit so that it will accept node-name arguments for either 'top' or 'base' BDS.
The filename and node-name are mutually exclusive to each other; i.e.:
"top" and "top-node-name" are mutually exclusive (enforced) "base" and "base-node-name" are mutually exclusive (enforced)
The preferred and recommended method now of using block-commit is to specify the BDS to operate on via their node-name arguments.
This also adds an explicit check that base_bs is in the chain of top_bs, because if they are referenced by their unique node-name arguments, the discovery process does not intrinsically verify they are in the same chain.
Reviewed-by Eric Blake <[email protected]>
Signed-off-by Jeff Cody <[email protected]>
--- blockdev.c | 54 ++++++++++++++++++++++++++++++++++++++++++++-------- qapi/block-core.json | 40 ++++++++++++++++++++++++++++---------- qmp-commands.hx | 31 ++++++++++++++++++++++-------- 3 files changed, 99 insertions(+), 26 deletions(-)
blockdev.c +46 -8
@@ -1924,13 +1924,16 @@ void qmp_block_stream(const char *device,void qmp_block_commit (const char *device,bool has_base,const char *base, +bool has_base_node_name,const char *base_node_name,bool has_top,const char *top, +bool has_top_node_name,const char *top_node_name,bool has_backing_file,const char *backing_file,bool has_speed,int64_t speed,Error **errp) { -BlockDriverState *bs; -BlockDriverState *base_bs, *top_bs; +BlockDriverState *bs =NULL ; +BlockDriverState *base_bs =NULL ; +BlockDriverState *top_bs =NULL ;Error *local_err =NULL ;/* This will be part of the QMP command, if/when the * BlockdevOnError change for blkmirror makes it in
@@ -1944,6 +1947,16 @@ void qmp_block_commit(const char *device,/* drain all i/o before commits */ bdrv_drain_all (); +/* argument combination validation */ +if (has_base && has_base_node_name) { +error_setg (errp,"'base' and 'base-node-name' are mutually exclusive" ); +return ; + } +if (has_top && has_top_node_name) { +error_setg (errp,"'top' and 'top-node-name' are mutually exclusive" ); +return ; + } +/* Important Note: * libvirt relies on the DeviceNotFound error class in order to probe for * live commit feature versions; for this to work, we must make sure to
@@ -1955,14 +1968,16 @@ void qmp_block_commit(const char *device,return ; } -if (bdrv_op_is_blocked (bs,BLOCK_OP_TYPE_COMMIT , errp)) { -return ; - } -/* default top_bs is the active layer */ top_bs = bs; -if (has_top && top) { +if (has_top_node_name) { + top_bs =bdrv_lookup_bs (NULL , top_node_name, &local_err); +if (local_err) { +error_propagate (errp, local_err); +return ; + } + }else if (has_top && top) {if (strcmp (bs->filename, top) !=0 ) { top_bs =bdrv_find_backing_image (bs, top); }
@@ -1973,7 +1988,13 @@ void qmp_block_commit(const char *device,return ; } -if (has_base && base) { +if (has_base_node_name) { + base_bs =bdrv_lookup_bs (NULL , base_node_name, &local_err); +if (local_err) { +error_propagate (errp, local_err); +return ; + } + }else if (has_base && base) { base_bs =bdrv_find_backing_image (top_bs, base); }else { base_bs =bdrv_find_base (top_bs);
@@ -1990,6 +2011,23 @@ void qmp_block_commit(const char *device,return ; } +/* Verify that 'base' is in the same chain as 'top' */ +if (!bdrv_chain_contains (top_bs, base_bs)) { +error_setg (errp,"'base' and 'top' are not in the same chain" ); +return ; + } + +/* This should technically be caught in commit_start, but +* check here for validation completeness */ +if (!bdrv_chain_contains (bs, top_bs)) { +error_setg (errp,"'%s' and 'top' are not in the same chain" , device); +return ; + } + +if (bdrv_op_is_blocked (bs,BLOCK_OP_TYPE_COMMIT , errp)) { +return ; + } +if (top_bs == bs) {if (has_backing_file) { error_setg(errp, "'backing-file' specified,"
qapi/block-core.json +30 -10
@@ -745,14 +745,29 @@ # Live commit of data from overlay image nodes into backing nodes - i.e., # writes data between 'top' and 'base' into 'base'. # -# @device: the name of the device +# @device: The name of the device. # -# @base: #optional The file name of the backing image to write data into. -# If not specified, this is the deepest backing image +# For 'base', either @base or @base-node-name may be set but not both. If +# neither is specified, this is the deepest backing image. # -# @top: #optional The file name of the backing image within the image chain, -# which contains the topmost data to be committed down. If -# not specified, this is the active layer. +# @base: #optional The file name of the backing image to write data +# into. +# +# @base-node-name #optional The name of the block driver state node of the +# backing image to write data into. +# (Since 2.1) +# +# For 'top', either @top or @top-node-name must be set but not both. If +# neither is specified, this is the active layer. +# +# @top: #optional The file name of the backing image within the image +# chain, which contains the topmost data to be +# committed down. +# +# @top-node-name: #optional The block driver state node name of the backing +# image within the image chain, which contains the +# topmost data to be committed down. +# (Since 2.1) # # @backing-file: #optional The backing file string to write into the overlay # image of 'top'. If 'top' is the active layer,
@@ -787,17 +802,22 @@ # # Returns: Nothing on success # If commit or stream is already active on this device, DeviceInUse -# If @device does not exist, DeviceNotFound +# If @device does not exist or cannot be determined, DeviceNotFound # If image commit is not supported by this device, NotSupported -# If @base or @top is invalid, a generic error is returned +# If @base, @top, @base-node-name, @top-node-name invalid, GenericError # If @speed is invalid, InvalidParameter +# If both @base and @base-node-name are specified, GenericError +# If both @top and @top-node-name are specified, GenericError +# If @top or @top-node-name is the active layer, and @backing-file is +# specified, GenericError # # Since: 1.3 # ## { 'command': 'block-commit', - 'data': { 'device': 'str', '*base': 'str', '*top': 'str', - '*backing-file': 'str', '*speed': 'int' } } + 'data': { 'device': 'str', '*base': 'str', '*base-node-name': 'str', + '*top': 'str', '*top-node-name': 'str', '*backing-file': 'str', + '*speed': 'int' } } ## # @drive-backup
qmp-commands.hx +23 -8
@@ -985,7 +985,7 @@ EQMP { .name = "block-commit", - .args_type = "device:B,base:s?,top:s?,backing-file:s?,speed:o?", + .args_type = "device:B,base:s?,base-node-name:s?,top:s?,top-node-name:s?,backing-file:s?,speed:o?", .mhandler.cmd_new = qmp_marshal_input_block_commit, },
@@ -998,13 +998,28 @@ data between 'top' and 'base' into 'base'. Arguments: -- "device": The device's ID, must be unique (json-string) -- "base": The file name of the backing image to write data into. - If not specified, this is the deepest backing image - (json-string, optional) -- "top": The file name of the backing image within the image chain, - which contains the topmost data to be committed down. If - not specified, this is the active layer. (json-string, optional) +- "device": The device's ID, must be unique (json-string) + +For 'base', either base or base-node-name may be set but not both. If +neither is specified, this is the deepest backing image + +- "base": The file name of the backing image to write data into. + (json-string, optional) +- "base-node-name": The name of the block driver state node of the + backing image to write data into. + (json-string, optional) (Since 2.1) + +For 'top', either top or top-node-name must be set but not both. If +neither is specified, this is the active layer + +- "top": The file name of the backing image within the image chain, + which contains the topmost data to be committed down. + (json-string, optional) + +- "top-node-name": The block driver state node name of the backing + image within the image chain, which contains the + topmost data to be committed down. + (json-string, optional) (Since 2.1) - backing-file: The backing file string to write into the overlay image of 'top'. If 'top' is the active layer,
Signature · 1 line
1.9.3
PATCH v7 3/4 block: add ability for block-stream to use node-name
In reply to Jeff CodyThis adds the ability for block-stream to use node-name arguments for base, to specify the backing image to stream from.
Both 'base' and 'base-node-name' are optional, but mutually exclusive. Either can be specified, but not both together.
Reviewed-by Eric Blake <[email protected]>
Signed-off-by Jeff Cody <[email protected]>
--- blockdev.c | 43 +++++++++++++++++++++++++++++++++++-------- hmp.c | 3 ++- qapi/block-core.json | 17 +++++++++++++---- qmp-commands.hx | 2 +- 4 files changed, 51 insertions(+), 14 deletions(-)
blockdev.c +35 -8
@@ -1868,13 +1868,15 @@ static void block_job_cb(void *opaque, int ret)void qmp_block_stream (const char *device,bool has_base,const char *base, +bool has_base_node_name,const char *base_node_name,bool has_backing_file,const char *backing_file,bool has_speed,int64_t speed,bool has_on_error,BlockdevOnError on_error,Error **errp) { -BlockDriverState *bs; +BlockDriverState *bs =NULL ;BlockDriverState *base_bs =NULL ; +BlockDriverState *tmp_bs;Error *local_err =NULL ;const char *base_name =NULL ;
@@ -1882,25 +1884,53 @@ void qmp_block_stream(const char *device, on_error =BLOCKDEV_ON_ERROR_REPORT ; } +if (has_base && has_base_node_name) { +error_setg (errp,"'base' and 'base-node-name' are mutually exclusive" ); +return ; + } + bs =bdrv_find (device);if (!bs) {error_set (errp,QERR_DEVICE_NOT_FOUND , device);return ; } +if (has_base_node_name) { + base_bs =bdrv_lookup_bs (NULL , base_node_name, &local_err); +if (local_err) { +error_propagate (errp, local_err); +return ; + } + tmp_bs =bdrv_find_overlay (bs, base_bs); +if (tmp_bs) { + base_name = tmp_bs->backing_file; + } + } +if (bdrv_op_is_blocked (bs,BLOCK_OP_TYPE_STREAM , errp)) {return ; }if (has_base) { base_bs =bdrv_find_backing_image (bs, base); -if (base_bs ==NULL ) { -error_set (errp,QERR_BASE_NOT_FOUND , base); -return ; - } base_name = base; } +if (base_bs ==NULL && (has_base || has_base_node_name)) { +error_set (errp,QERR_BASE_NOT_FOUND , base); +return ; + } + +/* backing_file string overrides base bs filename */ + base_name = has_backing_file ? backing_file : base_name; + +/* Verify that 'base' is in the same chain as 'bs', if 'base' was +* specified */ +if (base_bs && !bdrv_chain_contains (bs, base_bs)) { +error_setg (errp,"'device' and 'base' are not in the same chain" ); +return ; + } +/* if we are streaming the entire chain, the result will have no backing * file, and specifying one is therefore an error */ if (base_bs ==NULL && has_backing_file) {
@@ -1909,9 +1939,6 @@ void qmp_block_stream(const char *device,return ; } -/* backing_file string overrides base bs filename */ - base_name = has_backing_file ? backing_file : base_name; -stream_start (bs, base_bs, base_name, has_speed ? speed :0 , on_error, block_job_cb, bs, &local_err);if (local_err) {
hmp.c +2 -1
@@ -1175,7 +1175,8 @@ void hmp_block_stream(Monitor *mon, const QDict *qdict)const char *base =qdict_get_try_str (qdict,"base" );int64_t speed =qdict_get_try_int (qdict,"speed" ,0 ); -qmp_block_stream (device, base !=NULL , base, false,NULL , +qmp_block_stream (device, base !=NULL , base, + false,NULL , false,NULL ,qdict_haskey (qdict,"speed" ), speed, true,BLOCKDEV_ON_ERROR_REPORT , &error);
qapi/block-core.json +13 -4
@@ -964,9 +964,17 @@ # On successful completion the image file is updated to drop the backing file # and the BLOCK_JOB_COMPLETED event is emitted. # -# @device: the device name +# @device: The device name. +# +# For 'base', either @base or @base-node-name may be set but not both. If +# neither is specified, the entire chain will be streamed into the active image, +# and the chain will consist of a single image (the current active layer) with +# no backing file. # -# @base: #optional the common backing file name +# @base: #optional the common backing file name +# +# @base-node-name: #optional the block driver state node name of the +# common backing file. (Since 2.1) # # @backing-file: #optional The backing file string to write into the active # layer. This filename is not validated.
@@ -995,8 +1003,9 @@ # Since: 1.1 ## { 'command': 'block-stream', - 'data': { 'device': 'str', '*base': 'str', '*backing-file': 'str', - '*speed': 'int', '*on-error': 'BlockdevOnError' } } + 'data': { 'device': 'str', '*base': 'str', '*base-node-name': 'str', + '*backing-file': 'str', '*speed': 'int', + '*on-error': 'BlockdevOnError' } } ## # @block-job-set-speed:
qmp-commands.hx +1 -1
@@ -979,7 +979,7 @@ EQMP { .name = "block-stream", - .args_type = "device:B,base:s?,speed:o?,backing-file:s?,on-error:s?", + .args_type = "device:B,base:s?,base-node-name:s?,speed:o?,backing-file:s?,on-error:s?", .mhandler.cmd_new = qmp_marshal_input_block_stream, },
Signature · 1 line
1.9.3
PATCH v7 4/4 block: Add QMP documentation for block-stream
In reply to Jeff CodyThe QMP command 'block-stream' was missing QMP documentation. Add that documentation.
Reviewed-by Eric Blake <[email protected]>
Reviewed-by Benoit Canet <[email protected]>
Signed-off-by Jeff Cody <[email protected]>
--- qmp-commands.hx | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+)
qmp-commands.hx +58 -0
@@ -983,6 +983,64 @@ EQMP .mhandler.cmd_new = qmp_marshal_input_block_stream, }, +SQMP +block-stream +------------ + +Copy data from a backing file into a block device. + +The block streaming operation is performed in the background until the entire +backing file has been copied. This command returns immediately once streaming +has started. The status of ongoing block streaming operations can be checked +with query-block-jobs. The operation can be stopped before it has completed +using the block-job-cancel command. + +If a base file is specified then sectors are not copied from that base file and +its backing chain. When streaming completes the image file will have the base +file as its backing file. This can be used to stream a subset of the backing +file chain instead of flattening the entire image. + +On successful completion the image file is updated to drop the backing file +and the BLOCK_JOB_COMPLETED event is emitted. + +- "device": The device name. + (json-string) + +For base, either 'base' or 'base-node-name' may be set but not both. If +neither is specified, the entire chain will be streamed into the active image, +and the chain will consist of a single image (the current active layer) with +no backing file. + +- "base": The common backing file name. + (json-string, optional) + +- "base-node-name": The block driver state node name of the common backing file. + (json-string, optional) (Since 2.1) + +- "backing-file": The backing file string to write into the active layer. + This filename is not validated. + + If a pathname string is such that it cannot be resolved by + QEMU, that means that subsequent QMP or HMP commands must + use node-names for the image in question, as filename + lookup methods will fail. + + If not specified, QEMU will automatically determine the + backing file string to use, or error out if there is no + obvious choice. Care should be taken when specifying the + string, to specify a valid filename or protocol. + (json-string, optional) + (Since 2.1) + +- "speed": The maximum speed, in bytes per second. + (json-int, optional) + +- "on-error": The action to take on an error (default report). + 'stop' and 'enospc' can only be used if the block device + supports io-status (see BlockInfo). + (json-enum, optional) (Since 1.3) +EQMP + { .name = "block-commit", .args_type = "device:B,base:s?,base-node-name:s?,top:s?,top-node-name:s?,backing-file:s?,speed:o?",
Signature · 1 line
1.9.3