[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 4/5] block: Accept node-name arguments for block
From: |
Benoît Canet |
Subject: |
Re: [Qemu-devel] [PATCH 4/5] block: Accept node-name arguments for block-commit |
Date: |
Thu, 15 May 2014 14:09:50 +0200 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
The Wednesday 14 May 2014 à 23:20:18 (-0400), Jeff Cody wrote :
> This 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.
>
> Signed-off-by: Jeff Cody <address@hidden>
> ---
> blockdev.c | 35 +++++++++++++++++++++++++++++++++--
> qapi-schema.json | 35 ++++++++++++++++++++++++++---------
> qmp-commands.hx | 29 ++++++++++++++++++++++-------
> 3 files changed, 81 insertions(+), 18 deletions(-)
>
> diff --git a/blockdev.c b/blockdev.c
> index 02c6525..d8cb396 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -1869,7 +1869,9 @@ void qmp_block_stream(const char *device, bool has_base,
>
> 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_speed, int64_t speed,
> Error **errp)
> {
> @@ -1888,6 +1890,15 @@ void qmp_block_commit(const char *device,
> /* drain all i/o before commits */
> bdrv_drain_all();
>
> + 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;
> + }
> +
> bs = bdrv_find(device);
> if (!bs) {
> error_set(errp, QERR_DEVICE_NOT_FOUND, device);
> @@ -1897,7 +1908,14 @@ void qmp_block_commit(const char *device,
> /* default top_bs is the active layer */
> top_bs = bs;
>
> - if (top) {
> + /* Find the 'top' image file for the commit */
> + 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);
> }
> @@ -1908,7 +1926,14 @@ void qmp_block_commit(const char *device,
> return;
> }
>
> - if (has_base && base) {
> + /* Find the 'base' image file for the commit */
> + 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);
> @@ -1919,6 +1944,12 @@ void qmp_block_commit(const char *device,
> return;
> }
>
> + /* Verify that 'base' is in the same chain as 'top' */
> + if (!bdrv_is_in_chain(top_bs, base_bs)) {
> + error_setg(errp, "'base' and 'top' are not in the same chain");
> + return;
> + }
> +
> if (top_bs == bs) {
> commit_active_start(bs, base_bs, speed, on_error, block_job_cb,
> bs, &local_err);
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 06a9b5d..eddb2b8 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -2096,12 +2096,27 @@
> #
> # @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.
> +#
> +# @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.
> +#
> +# @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)
> #
> # If top == base, that is an error.
> # If top == active, the job will not be completed by
> itself,
> @@ -2120,17 +2135,19 @@
> #
> # 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
> #
> # Since: 1.3
> #
> ##
> { 'command': 'block-commit',
> - 'data': { 'device': 'str', '*base': 'str', '*top': 'str',
> - '*speed': 'int' } }
> + 'data': { 'device': 'str', '*base': 'str', '*base-node-name': 'str',
> + '*top': 'str', '*top-node-name': 'str', '*speed': 'int' } }
>
> ##
> # @drive-backup
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 1aa3c65..2b9b1dc 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -985,7 +985,7 @@ EQMP
>
> {
> .name = "block-commit",
> - .args_type = "device:B,base:s?,top:s?,speed:o?",
> + .args_type =
> "device:B,base:s?,base-node-name:s?,top:s?,top-node-name:s?,speed:o?",
> .mhandler.cmd_new = qmp_marshal_input_block_commit,
> },
>
> @@ -999,12 +999,27 @@ 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)
> +
> +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.
> +
> +- "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)
> +
> +- "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)
^ (cosmetic) this block is not aligned with the upper
ones.
>
> If top == base, that is an error.
> If top == active, the job will not be completed by itself,
> --
> 1.8.3.1
>