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

[Benoît Canet]

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.

> 
> Perhaps this is actually different enough that it would justify creating
> a new QMP command 'blockdev-snapshot' that never creates image files but
> only ever takes existing nodes. The implementation of 'blockdev-snapshot-
> sync' could then become a wrapper around it.

What would be the benefits ?

Best regards

Benoît
> 
> >  # @format: #optional the format of the snapshot image, default is 'qcow2'.
> >  #
> >  # @mode: #optional whether and how QEMU should create a new image, default 
> > is
> >  #        'absolute-paths'.
> >  ##
> >  { 'type': 'BlockdevSnapshot',
> > -  'data': { 'device': 'str', 'snapshot-file': 'str', '*format': 'str',
> > -            '*mode': 'NewImageMode' } }
> > +  'data': { '*device': 'str', '*node-name': 'str',
> > +            'snapshot-file': 'str', '*snapshot-node-name': 'str',
> > +            '*format': 'str', '*mode': 'NewImageMode' } }
> >  
> >  ##
> >  # @BlockdevSnapshotInternal
> 
> Kevin