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

2016-09-20 Thread Kevin Wolf
Am 19.09.2016 um 21:28 hat Eric Blake geschrieben:
> On 09/19/2016 11:54 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 
> > ---
> >  blockdev.c   | 61 
> > +++-
> >  qapi/block-core.json | 14 
> >  qmp-commands.hx  | 16 --
> >  3 files changed, 66 insertions(+), 25 deletions(-)
> > 
> 
> > +++ b/qmp-commands.hx
> > @@ -4295,7 +4295,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,
> >  },
> 
> Let the merge conflicts begin! Commit 33e1666b has landed.

Oh joy. I guess the rebase invalidates all of the R-bs, so I'll drop all
of them and resend a v3.

Kevin


pgpzlnfjAyRR1.pgp
Description: PGP signature


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

2016-09-19 Thread Eric Blake
On 09/19/2016 11:54 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 
> ---
>  blockdev.c   | 61 
> +++-
>  qapi/block-core.json | 14 
>  qmp-commands.hx  | 16 --
>  3 files changed, 66 insertions(+), 25 deletions(-)
> 

> +++ b/qmp-commands.hx
> @@ -4295,7 +4295,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,
>  },

Let the merge conflicts begin! Commit 33e1666b has landed.

> @@ -4329,7 +4331,7 @@ Arguments:
>  Example:
>  
>  -> { "execute": "blockdev-open-tray",
> - "arguments": { "device": "ide1-cd0" } }
> + "arguments": { "id": "ide0-1-0" } }

The changes to the examples look good.  Overall, I'm fine with:

Reviewed-by: Eric Blake 

but I'm afraid it may be easiest for you to respin v3 and do the rebase
work here and elsewhere in the series.

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



signature.asc
Description: OpenPGP digital signature


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

2016-09-19 Thread Kevin Wolf
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 
---
 blockdev.c   | 61 +++-
 qapi/block-core.json | 14 
 qmp-commands.hx  | 16 --
 3 files changed, 66 insertions(+), 25 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index fb207cd..046f9c6 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -56,7 +56,8 @@
 static QTAILQ_HEAD(, BlockDriverState) monitor_bdrv_states =
 QTAILQ_HEAD_INITIALIZER(monitor_bdrv_states);
 
-static int do_open_tray(const char *device, bool force, Error **errp);
+static int do_open_tray(const char *blk_name, const char *qdev_id,
+bool force, Error **errp);
 
 static const char *const if_name[IF_COUNT] = {
 [IF_NONE] = "none",
@@ -1198,6 +1199,29 @@ static BlockDriverState *qmp_get_root_bs(const char 
*name, Error **errp)
 return bs;
 }
 
+static BlockBackend *qmp_get_blk(const char *blk_name, const char *qdev_id,
+ Error **errp)
+{
+BlockBackend *blk;
+
+if (!blk_name == !qdev_id) {
+error_setg(errp, "Need exactly one of 'device' and 'id'");
+return NULL;
+}
+
+if (qdev_id) {
+blk = blk_by_qdev_id(qdev_id, errp);
+} else {
+blk = blk_by_name(blk_name);
+if (blk == NULL) {
+error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
+  "Device '%s' not found", blk_name);
+}
+}
+
+return blk;
+}
+
 void hmp_commit(Monitor *mon, const QDict *qdict)
 {
 const char *device = qdict_get_str(qdict, "device");
@@ -2250,7 +2274,7 @@ void qmp_eject(const char *device, bool has_force, bool 
force, Error **errp)
 force = false;
 }
 
-rc = do_open_tray(device, force, &local_err);
+rc = do_open_tray(device, NULL, force, &local_err);
 if (rc && rc != -ENOSYS) {
 error_propagate(errp, local_err);
 return;
@@ -2295,15 +2319,15 @@ void qmp_block_passwd(bool has_device, const char 
*device,
  * If the guest was asked to open the tray, return -EINPROGRESS.
  * Else, return 0.
  */
-static int do_open_tray(const char *device, bool force, Error **errp)
+static int do_open_tray(const char *blk_name, const char *qdev_id,
+bool force, Error **errp)
 {
 BlockBackend *blk;
+const char *device = qdev_id ?: blk_name;
 bool locked;
 
-blk = blk_by_name(device);
+blk = qmp_get_blk(blk_name, qdev_id, errp);
 if (!blk) {
-error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
-  "Device '%s' not found", device);
 return -ENODEV;
 }
 
@@ -2339,7 +2363,9 @@ static int do_open_tray(const char *device, bool force, 
Error **errp)
 return 0;
 }
 
-void qmp_blockdev_open_tray(const char *device, bool has_force, bool force,
+void qmp_blockdev_open_tray(bool has_device, const char *device,
+bool has_id, const char *id,
+bool has_force, bool force,
 Error **errp)
 {
 Error *local_err = NULL;
@@ -2348,7 +2374,9 @@ void qmp_blockdev_open_tray(const char *device, bool 
has_force, bool force,
 if (!has_force) {
 force = false;
 }
-rc = do_open_tray(device, force, &local_err);
+rc = do_open_tray(has_device ? device : NULL,
+  has_id ? id : NULL,
+  force, &local_err);
 if (rc && rc != -ENOSYS && rc != -EINPROGRESS) {
 error_propagate(errp, local_err);
 return;
@@ -2356,19 +2384,22 @@ void qmp_blockdev_open_tray(const char *device, bool 
has_force, bool force,
 error_free(local_err);
 }
 
-void qmp_blockdev_close_tray(const char *device, Error **errp)
+void qmp_blockdev_close_tray(bool has_device, const char *device,
+ bool has_id, const char *id,
+ Error **errp)
 {
 BlockBackend *blk;
 
-blk = blk_by_name(device);
+device = has_device ? device : NULL;
+id = has_id ? id : NULL;
+
+blk = qmp_get_blk(device, id, errp);
 if (!blk) {
-error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
-  "Device '%s' not found", device);
 return;
 }
 
 if (!blk_dev_has_removable_media(blk)) {
-error_setg(errp, "Device '%s' is not removable", device);
+error_setg(errp, "Device '%s' is not removable", device ?: id);
 return;
 }
 
@@ -2564,7 +2595,7 @@ void qmp_blockdev_change_medium(const char *device, const 
char *filename,
 goto fail;
 }
 
-rc = do_open_tray(device, false, &err);
+rc = do_open_tray(device, NULL, false, &err);
 if (rc && rc != -ENOSYS) {
 error_propagate(errp, err);
 goto fail;
@@ -2586,7 +2617,7 @@ void qmp_blockdev_change_medium(cons