[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 07/11] block: Accept node-name arguments for
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v2 07/11] block: Accept node-name arguments for block-commit |
Date: |
Tue, 27 May 2014 11:46:03 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.5.0 |
On 05/27/2014 08:28 AM, 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 "device" argument now becomes optional as well, because with
> a node-name we can identify the block device chain. It is only
> optional if a node-name is not specified.
>
> 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 | 83
> +++++++++++++++++++++++++++++++++++++++++++++++++-------
> qapi-schema.json | 38 +++++++++++++++++++-------
> qmp-commands.hx | 33 ++++++++++++++++------
> 3 files changed, 126 insertions(+), 28 deletions(-)
>
> + /* default top_bs is the active layer, if NULL */
> + top_bs = top_bs ?: bs;
> +
> + if (has_top && top) {
> if (strcmp(bs->filename, top) != 0) {
Cool! This part fixes the complaint I had in 6/11. If you have a
reason to rebase, you could hoist this one-line if change into that
patch, while still keeping my R-b on both patches.
Everything else here looks sane (lots of conditions before starting, but
all of them made sense, and while there are five separate optional
arguments that can interplay, my read of your logic says you correctly
covered all the cases)
> +++ b/qapi-schema.json
> @@ -2097,14 +2097,30 @@
> # 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: #optional The name of the device. Optional only if
> +# node-names are used for both base and top
More precisely, it is also optional if node-name is used for base and
top/top-node-name is omitted, or if node-name is used for top and
base/base-node-name is omitted; but that's splitting hairs, so keep your
current wording.
> +# 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.
> +#
We aren't consistent on whether to use a trailing '.'; if you have a
respin for other reasons, it's worth thinking about; but it's not a
cause for a respin by itself.
Reviewed-by: Eric Blake <address@hidden>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [PATCH v2 04/11] block: add helper function to find the active layer of any BDS, (continued)
- [Qemu-devel] [PATCH v2 04/11] block: add helper function to find the active layer of any BDS, Jeff Cody, 2014/05/27
- [Qemu-devel] [PATCH v2 05/11] block: simplify bdrv_find_base(), Jeff Cody, 2014/05/27
- [Qemu-devel] [PATCH v2 06/11] block: make 'top' argument to block-commit optional, Jeff Cody, 2014/05/27
- [Qemu-devel] [PATCH v2 08/11] block: extend block-commit to accept a string for the backing file, Jeff Cody, 2014/05/27
- [Qemu-devel] [PATCH v2 07/11] block: Accept node-name arguments for block-commit, Jeff Cody, 2014/05/27
- Re: [Qemu-devel] [PATCH v2 07/11] block: Accept node-name arguments for block-commit,
Eric Blake <=
- [Qemu-devel] [PATCH v2 09/11] block: add ability for block-stream to use node-name, Jeff Cody, 2014/05/27
- [Qemu-devel] [PATCH v2 10/11] block: add backing-file option to block-stream, Jeff Cody, 2014/05/27
- [Qemu-devel] [PATCH v2 11/11] block: add QAPI command to allow live backing file change, Jeff Cody, 2014/05/27