[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PATCH 04/16] block: Allow references for
From: |
Eric Blake |
Subject: |
Re: [Qemu-block] [Qemu-devel] [PATCH 04/16] block: Allow references for backing files |
Date: |
Wed, 2 Sep 2015 12:50:14 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 |
On 09/02/2015 02:51 AM, Wen Congyang wrote:
> Usage:
> -drive file=xxx,id=Y, \
> -drive file=xxxx,id=X,backing.backing_reference=Y
>
> It will create such backing chain:
> {virtio-blk dev 'Y'} {virtio-blk dev 'X'}
> | |
> | |
> v v
>
> [base] <- [mid] <- ( Y ) <----------------- ( X )
This makes any changes to 'Y' have unspecified effects on 'X'. While we
may have a valid reason to use a backing BDS in more than one chain, I
seriously doubt anyone will ever want to have two guest-visible -drive's
that are both read-write where one can corrupt the other. I can totally
see the point of having BDS 'Y' exist for checkpoints or some other
non-guest-visible action, so I'm not saying this patch is wrong, just
that the commit message is picking a poor example of how it would be used.
>
> Signed-off-by: Wen Congyang <address@hidden>
> Signed-off-by: zhanghailiang <address@hidden>
> Signed-off-by: Gonglei <address@hidden>
> ---
> block.c | 39 +++++++++++++++++++++++++++++++++++----
> include/block/block.h | 1 +
> 2 files changed, 36 insertions(+), 4 deletions(-)
>
> diff --git a/block.c b/block.c
> index d02d9e9..f93c50d 100644
> --- a/block.c
> +++ b/block.c
> @@ -1179,6 +1179,7 @@ out:
> }
>
> #define ALLOW_WRITE_BACKING_FILE "allow-write-backing-file"
> +#define BACKING_REFERENCE "backing_reference"
Why the inconsistency in '-' vs. '_'? I'd stick with dash here.
> static QemuOptsList backing_file_opts = {
> .name = "backing_file",
> .head = QTAILQ_HEAD_INITIALIZER(backing_file_opts.head),
> @@ -1188,6 +1189,11 @@ static QemuOptsList backing_file_opts = {
> .type = QEMU_OPT_BOOL,
> .help = "allow write to backing file",
> },
> + {
> + .name = BACKING_REFERENCE,
> + .type = QEMU_OPT_STRING,
> + .help = "reference to the exsiting BDS",
s/exsiting/existing/
But why do we need this? In qapi, BlockdevOptionsGenericCOWFormat
already has '*backing':'BlockdevRef', and BlockdevRef already has a
choice between 'definition' (object) and 'reference' (string). Or is
this just a matter of teaching the command line to do what QMP can
already do? In which case, wouldn't:
-drive file=xxx,id=Y, -drive file=xxxx,id=X,backing=Y
be the natural mapping of 'backing' being a string rather than a dictionary?
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature