Re: [Qemu-devel] [PATCH 07/10] block: Accept device model name for eject

From: Eric Blake <eblake@redhat.com>
To: Kevin Wolf <kwolf@redhat.com>, qemu-block@nongnu.org
Cc: mreitz@redhat.com, qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH 07/10] block: Accept device model name for eject
Date: Wed, 14 Sep 2016 16:09:12 -0500	[thread overview]
Message-ID: <156c5ff3-73c7-cefb-82b8-d0f6b8a6bb9f@redhat.com> (raw)
In-Reply-To: <1471625435-6190-8-git-send-email-kwolf@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 2079 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 eject to accept a qdev device name.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  blockdev.c      | 10 +++++++---
>  hmp.c           |  2 +-
>  qapi/block.json |  7 ++++++-
>  qmp-commands.hx |  8 +++++---
>  4 files changed, 19 insertions(+), 8 deletions(-)
> 

> +++ b/blockdev.c

> @@ -2272,14 +2274,16 @@ void qmp_eject(const char *device, bool has_force, bool force, Error **errp)
>          force = false;
>      }
>  
> -    rc = do_open_tray(device, NULL, force, &local_err);
> +    rc = do_open_tray(has_device ? device : NULL,
> +                      has_id ? id : NULL,
> +                      force, &local_err);
>      if (rc && rc != -ENOSYS) {
>          error_propagate(errp, local_err);
>          return;
>      }
>      error_free(local_err);
>  
> -    qmp_x_blockdev_remove_medium(true, device, false, NULL, errp);
> +    qmp_x_blockdev_remove_medium(has_device, device, has_id, id, errp);

Hmm. We have to call into the underlying x- command, without breaking
'eject' which must indeed keep the deprecated 'device' parameter.  So
maybe that answers my question on 5 and 6.

Or can we teach qmp_eject() to do the lookup now, so that it can call
into qmp_x_blockdev_remove_medium() with just id, even if the user
called in with device?  Do we even have that information readily
accessible (given a device, resolve it to an id that would work as if we
were passing an id in the first place)?

> +++ b/qapi/block.json
> @@ -127,6 +127,8 @@
>  #
>  # @device:  The name of the device
>  #
> +# @id:      The name or QOM path of the guest device (since: 2.8)
> +#

Missing mention of '#optional' in both parameters, as well as the
deprecation warning you had in 4/10.

-- 
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 --]