[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 04/16] block: Allow references for backing files
From: |
Wen Congyang |
Subject: |
Re: [Qemu-devel] [PATCH 04/16] block: Allow references for backing files |
Date: |
Wed, 9 Sep 2015 16:51:56 +0800 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 |
On 09/03/2015 02:50 AM, Eric Blake wrote:
> 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.
Sorry, this graph is wrong. virtio-blk dev 'Y' can not use the BDS Y if it
is referenced by X. That is why I need patch 1 and 2.
Thanks
Wen Congyang
>
>>
>> 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?
>
- [Qemu-devel] [PATCH 00/16] Block replication for continuous checkpoints, Wen Congyang, 2015/09/02
- [Qemu-devel] [PATCH 01/16] introduce a new API to enable/disable attach device model, Wen Congyang, 2015/09/02
- [Qemu-devel] [PATCH 03/16] allow writing to the backing file, Wen Congyang, 2015/09/02
- [Qemu-devel] [PATCH 04/16] block: Allow references for backing files, Wen Congyang, 2015/09/02
- [Qemu-devel] [PATCH 07/16] Backup: clear all bitmap when doing block checkpoint, Wen Congyang, 2015/09/02
- [Qemu-devel] [PATCH 02/16] introduce a new API to check if blk is attached, Wen Congyang, 2015/09/02
- [Qemu-devel] [PATCH 06/16] quorum: allow ignoring child errors, Wen Congyang, 2015/09/02
- [Qemu-devel] [PATCH 09/16] Allow creating backup jobs when opening BDS, Wen Congyang, 2015/09/02