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 <we...@cn.fujitsu.com> >> Signed-off-by: zhanghailiang <zhang.zhanghaili...@huawei.com> >> Signed-off-by: Gonglei <arei.gong...@huawei.com> >> --- >> 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? >