Re: [Qemu-devel] [PATCH] qmp-commands.hx: Update the supported 'transaction' operations

[Kashyap Chamarthy]

[Sorry, I noticed your question only just now, as I was briefly away
from this list.]

On Fri, Oct 02, 2015 at 07:59:48PM +0200, Max Reitz wrote:
> On 02.10.2015 14:12, Kashyap Chamarthy wrote:
> > Although the canonical source of reference for QMP commands is
> > qapi-schema.json, for consistency's sake, update qmp-commands.hx to
> > state the list of supported transactionable operations, namely:
> > 
> >     drive-backup
> >     blockdev-backup
> >     blockdev-snapshot-internal-sync
> >     abort
> >     block-dirty-bitmap-add
> >     block-dirty-bitmap-clear
> > 
> > Also update the possible values for the "type" action array.
> > 
> > Signed-off-by: Kashyap Chamarthy <address@hidden>
> > Reviewed-by: Eric Blake <address@hidden>
> > ---
> > In v8 ("v8" because Fam included this as part of a series that is
> > in its "v7" edition):
> >  - Update the possible values for supported 'type' of operation
> > ---
> >  qmp-commands.hx | 29 ++++++++++++++++++++++-------
> >  1 file changed, 22 insertions(+), 7 deletions(-)
> > 
> > diff --git a/qmp-commands.hx b/qmp-commands.hx
> > index 
> > d2ba800d5effaddcae4c437452ef1f67c0aaaaa9..2b52980cfc2f80904a3c7375a382b638e8a51161
> >  100644
> > --- a/qmp-commands.hx
> > +++ b/qmp-commands.hx
> > @@ -1270,11 +1270,22 @@ SQMP
> >  transaction
> >  -----------
> >  
> > -Atomically operate on one or more block devices.  The only supported 
> > operations
> > -for now are drive-backup, internal and external snapshotting.  A list of
> > -dictionaries is accepted, that contains the actions to be performed.
> > -If there is any failure performing any of the operations, all operations
> > -for the group are abandoned.
> > +Atomically operate on one or more block devices.  Operations that are
> > +currently supported:
> > +
> > +    - drive-backup
> > +    - blockdev-backup
> > +    - blockdev-snapshot-sync
> > +    - blockdev-snapshot-internal-sync
> > +    - abort
> > +    - block-dirty-bitmap-add
> > +    - block-dirty-bitmap-clear
> > +
> > +Refer to the qemu/qapi-schema.json file for minimum required QEMU
> > +versions for these operations.  A list of dictionaries is accepted,
> > +that contains the actions to be performed.  If there is any failure
> > +performing any of the operations, all operations for the group are
> > +abandoned.
> >  
> >  For external snapshots, the dictionary contains the device, the file to 
> > use for
> >  the new snapshot, and the format.  The default format, if not specified, is
> > @@ -1301,8 +1312,12 @@ it later with qemu-img or other command.
> >  Arguments:
> >  
> >  actions array:
> > -    - "type": the operation to perform.  The only supported
> > -      value is "blockdev-snapshot-sync". (json-string)
> > +    - "type": the operation to perform (json-string).  Possible
> > +              values: "drive-backup", "blockdev-backup",
> > +                      "blockdev-snapshot-sync",
> > +                      "blockdev-snapshot-internal-sync",
> > +                      "abort", "block-dirty-bitmap-add",
> > +                      "block-dirty-bitmap-clear"
> >      - "data": a dictionary.  The contents depend on the value
> >        of "type".  When "type" is "blockdev-snapshot-sync":
> >        - "device": device name to snapshot (json-string)
> > 
> 
> Do you want to extend this list, too? Right now it only contains
> parameter information for blockdev-snapshot-sync and
> blockdev-snapshot-internal-sync.

You mean, all the parameters for rest of the transactionable commands
that one can possibly add (taking block-core.json)?

E.g. for 'drive-backup' command, all the possible data ('device',
target', 'format', 'sync', etc) one can provide (which are enumerated in
the DriveBackup struct):


    $ less qapi/block-core.json
    [...]
    { 'struct': 'DriveBackup',
      'data': { 'device': 'str', 'target': 'str', '*format': 'str',
                'sync': 'MirrorSyncMode', '*mode': 'NewImageMode',
                '*speed': 'int', '*bitmap': 'str',
                '*on-source-error': 'BlockdevOnError',
                '*on-target-error': 'BlockdevOnError' } }
    [...]

And, data provided by 'BlockdevBackup' struct for 'blockdev-backup', so
on.

If so, I can do try that if that's the preferred way, but maybe it can
be done as a separate commit, since it's been that way for a while?  


-- 
/kashyap