Re: [Qemu-devel] [PATCH V5 7/7] qmp: Allow to take external snapshots on bs graphs node.

[Kevin Wolf]

Am 22.01.2014 um 22:33 hat Benoît Canet geschrieben:
> Le Tuesday 21 Jan 2014 à 15:28:49 (+0100), Kevin Wolf a écrit :
> > Am 12.12.2013 um 16:34 hat Benoît Canet geschrieben:
> > > Signed-off-by: Benoit Canet <address@hidden>
> > > ---
> > >  blockdev.c       | 55 
> > > ++++++++++++++++++++++++++++++++++++++++++++++++-------
> > >  hmp.c            |  4 +++-
> > >  qapi-schema.json | 13 ++++++++++---
> > >  qmp-commands.hx  | 11 ++++++++++-
> > >  4 files changed, 71 insertions(+), 12 deletions(-)
> > 
> > > diff --git a/qapi-schema.json b/qapi-schema.json
> > > index 3977619..d7afb69 100644
> > > --- a/qapi-schema.json
> > > +++ b/qapi-schema.json
> > > @@ -1759,18 +1759,25 @@
> > >  ##
> > >  # @BlockdevSnapshot
> > >  #
> > > -# @device:  the name of the device to generate the snapshot from.
> > > +# Either @device or @node-name must be set but not both.
> > > +#
> > > +# @device: #optional the name of the device to generate the snapshot 
> > > from.
> > > +#
> > > +# @node-name: #optional graph node name to generate the snapshot from 
> > > (Since 2.0)
> > >  #
> > >  # @snapshot-file: the target of the new image. A new file will be 
> > > created.
> > >  #
> > > +# @snapshot-node-name: #optional the graph node name of the new image 
> > > (Since 2.0)
> > > +#
> > 
> > I think we should document how this plays together with snapshot-file,
> > format and mode.
> 
> What kind of interactions do you expect ?
> The only kind of interaction I see is that setting node-name imply that the
> user really want to manipulate the graph and that snapshot-node-name is
> mandatory as a consequence.

Aha, I misunderstood. I thought you could pass the node name of an
existing block device that should be used as the new overlay image. But
in fact, you still create a new image and only assign the
snapshot-node-name to the newly created node.

Nothing wrong with the behaviour, it's just that device/node-name and
snapshot-file/snapshot-node-name suggested a parallelity that isn't
there. Perhaps someone has a good idea to improve the naming.

Kevin