Re: [Qemu-devel] [PATCH 3/6] rename blockdev-group-snapshot-sync

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 3714 bytes --]

On 02/29/2012 06:37 AM, Paolo Bonzini wrote:
> We will add other kinds of operation.  Prepare for this by adjusting
> the schema and renaming some types/variables.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  blockdev.c       |   73 +++++++++++++++++++++++++++++------------------------
>  qapi-schema.json |   33 ++++++++++++++++--------
>  qmp-commands.hx  |   51 ++++++++++++++++++++-----------------
>  3 files changed, 90 insertions(+), 67 deletions(-)
> 

>  ##
> -# @blockdev-group-snapshot-sync
> +# @blockdev-transaction
>  #
> -# Generates a synchronous snapshot of a group of one or more block devices,
> -# as atomically as possible.  If the snapshot of any device in the group
> -# fails, then the entire group snapshot will be abandoned and the
> -# appropriate error returned.
> +# Atomically operate on a group of one or more block devices.  If
> +# any operation fails, then the entire set of actions will be
> +# abandoned and the appropriate error returned.  The only operation
> +# supported is currently snapshot.
>  #
>  #  List of:
> -#  @SnapshotDev: information needed for the device snapshot
> +#  @BlockdevAction: information needed for the device snapshot

>  Arguments:
>  
> -devlist array:
> -    - "device": device name to snapshot (json-string)
> -    - "snapshot-file": name of new image file (json-string)
> -    - "format": format of new image (json-string, optional)
> +actions array:
> +    - "type": the operation to perform.  The only supported
> +      value is "snapshot". (json-string)
> +    - "data": a dictionary.  The contents depend on the value
> +      of "type".  When "type" is "snapshot":
> +      - "device": device name to snapshot (json-string)
> +      - "snapshot-file": name of new image file (json-string)
> +      - "format": format of new image (json-string, optional)

I like it.  Of course, this means I still have a moving target for
implementing the libvirt side of things, so I'd like consensus sooner
rather than later.  More importantly, as long as
blockdev-group-snapshot-sync is not part of a release, renaming it is
fine; but if we get to the qemu 1.1 release with Jeff's patches but not
Paolo's rename, then libvirt has a harder job to cope with both names.


>  
>  Example:
>  
> --> { "execute": "blockdev-group-snapshot-sync", "arguments":
> -                      { "devlist": [{ "device": "ide-hd0",
> -                                      "snapshot-file": "/some/place/my-image",
> -                                      "format": "qcow2" },
> -                                    { "device": "ide-hd1",
> -                                      "snapshot-file": "/some/place/my-image2",
> -                                      "format": "qcow2" }] } }
> +-> { "execute": "blockdev-transaction",
> +     "arguments": { "actions": [
> +         { 'type': 'snapshot, 'data' : { "device": "ide-hd0",
> +                                         "snapshot-file": "/some/place/my-image",
> +                                         "format": "qcow2" } },
> +         { 'type': 'snapshot, 'data' : { "device": "ide-hd1",
> +                                         "snapshot-file": "/some/place/my-image2",
> +                                         "format": "qcow2",
> +                                         "op": "snapshot" } } ] } }

Drop the "op":"snapshot".  It's a leftover from before your conversion
to a union type.

Question - when reading/writing these examples, are 'type' and "type"
interchangeable (like in XML)?

-- 
Eric Blake   eblake@redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 620 bytes --]