Re: [Qemu-devel] [PATCH 04/10] block: Accept device model name for blockdev-open/close-tray

[Eric Blake <eblake@redhat.com>]

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

On 08/19/2016 11:50 AM, Kevin Wolf wrote:
> In order to remove the necessity to use BlockBackend names in the
> external API, we want to allow qdev device names in all device related
> commands.
> 
> This converts blockdev-open/close-tray to accept a qdev device name.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  blockdev.c           | 60 +++++++++++++++++++++++++++++++++++++++-------------
>  qapi/block-core.json | 14 ++++++++----
>  qmp-commands.hx      | 12 +++++++----
>  3 files changed, 63 insertions(+), 23 deletions(-)
> 

> +++ b/qapi/block-core.json
> @@ -2316,7 +2316,9 @@
>  #   to it
>  # - if the guest device does not have an actual tray
>  #
> -# @device: block device name
> +# @device:  block device name (deprecated, use @id instead)
> +#
> +# @id:      the name or QOM path of the guest device (since: 2.8)

Wish there were an easier way to write mutually-exclusive pairs in JSON,
but without that, your approach is fine.

> +++ b/qmp-commands.hx
> @@ -4277,7 +4277,7 @@ EQMP
>  
>      {
>          .name       = "blockdev-open-tray",
> -        .args_type  = "device:s,force:b?",
> +        .args_type  = "device:s?,id:s?,force:b?",
>          .mhandler.cmd_new = qmp_marshal_blockdev_open_tray,
>      },

Will conflict with Marc-Andre's work to remove qmp-commands.hx; but we
can figure it out based on what merges first.

>  
> @@ -4302,7 +4302,9 @@ which no such event will be generated, these include:
>  
>  Arguments:
>  
> -- "device": block device name (json-string)
> +- "device": block device name (deprecated, use @id instead)
> +            (json-string, optional)
> +- "id": the name or QOM path of the guest device (json-string, optional)
>  - "force": if false (the default), an eject request will be sent to the guest if
>             it has locked the tray (and the tray will not be opened immediately);
>             if true, the tray will be opened regardless of whether it is locked

Are there any example code snippets that should be updated alongside
this? If not, should we be thinking of adding an example?

But I can live with this patch as an incremental improvement, even if we
decide we want more as a followup based on my question above, so:

Reviewed-by: Eric Blake <eblake@redhat.com>

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