Re: [Qemu-devel] [PATCH 6/7] block: add target-id option to drive-backup QMP command

[Eric Blake <eblake@redhat.com>]

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

On 07/01/2013 11:59 PM, Fam Zheng wrote:
> Add target-id (optional) to drive-backup command, to make the target bs
> a named drive so that we can operate on it (e.g. export with NBD).
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  blockdev.c       | 4 +++-
>  qapi-schema.json | 7 +++++--
>  qmp-commands.hx  | 3 ++-
>  3 files changed, 10 insertions(+), 4 deletions(-)
> 

> +++ b/qapi-schema.json
> @@ -1654,7 +1654,8 @@
>  # Since: 1.6
>  ##
>  { 'type': 'DriveBackup',
> -  'data': { 'device': 'str', 'target': 'str', '*format': 'str',
> +  'data': { 'device': 'str', 'target': 'str',
> +            '*target-id': 'str', '*format': 'str',

Seems undocumented...

>              '*mode': 'NewImageMode', '*speed': 'int',
>              '*on-source-error': 'BlockdevOnError',
>              '*on-target-error': 'BlockdevOnError' } }
> @@ -1807,6 +1808,7 @@
>  #          is a device, the existing file/device will be used as the new
>  #          destination.  If it does not exist, a new file will be created.
>  #
> +# @target-id: #optional the drive id of the target.

...until I read this.  Hmm, I think we should first consolidate things
for DriveBackup (so that documentation is listed only once, prior to the
DriveBackup 'type' declaration), by rebasing things on top of
in the same was as Kevin's series "[PATCH v3 0/3] qapi: Top-level type
reference for command definitions" does for BlockdevSnapshot.

The documentation is not incorrect, but it also isn't very helpful -
what is the "drive id of the target" and when would I want to set it?
What do I gain by overriding the drive id, and what is the default
behavior when I don't pass in the option?

> +++ b/qmp-commands.hx
> @@ -913,7 +913,7 @@ EQMP
>  
>      {
>          .name       = "drive-backup",
> -        .args_type  = "device:B,target:s,speed:i?,mode:s?,format:s?,"
> +        .args_type  = "device:B,target:s,target-id:s?,speed:i?,mode:s?,format:s?,"
>                        "on-source-error:s?,on-target-error:s?",
>          .mhandler.cmd_new = qmp_marshal_input_drive_backup,
>      },
> @@ -936,6 +936,7 @@ Arguments:
>              device, the existing file/device will be used as the new
>              destination.  If it does not exist, a new file will be created.
>              (json-string)
> +- "target-id": the drive id of the target image.

Should probably mention (json-string, optional), as done elsewhere in
this command.

>  - "format": the format of the new destination, default is to probe if 'mode' is
>              'existing', else the format of the source
>              (json-string, optional)
> 

-- 
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: 621 bytes --]