Re: [Qemu-block] [PATCH 3/3] block: Add qdev ID to DEVICE_TRAY_MOVED

2016-10-05 Thread Max Reitz
On 05.10.2016 11:26, Kevin Wolf wrote:
> The event currently only contains the BlockBackend name. However, with
> anonymous BlockBackends, this is always the empty string. Add the qdev
> ID (or if none was given, the QOM path) so that the user can still see
> which device caused the event.
> 
> Event generation has to be moved from bdrv_eject() to the BlockBackend
> because the BDS doesn't know the attached device, but that's easy
> because blk_eject() is the only user of it.
> 
> Signed-off-by: Kevin Wolf 
> ---
>  block.c   |  7 ---
>  block/block-backend.c | 33 -
>  docs/qmp-commands.txt |  3 +++
>  docs/qmp-events.txt   |  6 +-
>  qapi/block.json   |  8 ++--
>  5 files changed, 46 insertions(+), 11 deletions(-)

I didn't even know we use the same event for both virtual and physical
trays. Fun, fun.

Even more fun in that blk_eject() always issues the event, regardless of
whether there actually is a physical tray. Therefore, I assume it's
effectively just an event for the virtual tray which has to hook up in
blk_eject() because a guest eject won't go through
blk_dev_change_media_cb(), but it will go through blk_eject() (which QMP
ejects do not). Totally makes sense.

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature


[Qemu-block] [PATCH 3/3] block: Add qdev ID to DEVICE_TRAY_MOVED

2016-10-05 Thread Kevin Wolf
The event currently only contains the BlockBackend name. However, with
anonymous BlockBackends, this is always the empty string. Add the qdev
ID (or if none was given, the QOM path) so that the user can still see
which device caused the event.

Event generation has to be moved from bdrv_eject() to the BlockBackend
because the BDS doesn't know the attached device, but that's easy
because blk_eject() is the only user of it.

Signed-off-by: Kevin Wolf 
---
 block.c   |  7 ---
 block/block-backend.c | 33 -
 docs/qmp-commands.txt |  3 +++
 docs/qmp-events.txt   |  6 +-
 qapi/block.json   |  8 ++--
 5 files changed, 46 insertions(+), 11 deletions(-)

diff --git a/block.c b/block.c
index bb1f1ec..b0bdad6 100644
--- a/block.c
+++ b/block.c
@@ -3360,17 +3360,10 @@ int bdrv_media_changed(BlockDriverState *bs)
 void bdrv_eject(BlockDriverState *bs, bool eject_flag)
 {
 BlockDriver *drv = bs->drv;
-const char *device_name;
 
 if (drv && drv->bdrv_eject) {
 drv->bdrv_eject(bs, eject_flag);
 }
-
-device_name = bdrv_get_device_name(bs);
-if (device_name[0] != '\0') {
-qapi_event_send_device_tray_moved(device_name,
-  eject_flag, &error_abort);
-}
 }
 
 /**
diff --git a/block/block-backend.c b/block/block-backend.c
index 39c5613..6859791 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -566,6 +566,23 @@ void *blk_get_attached_dev(BlockBackend *blk)
 return blk->dev;
 }
 
+/* Return the qdev ID, or if no ID is assigned the QOM path, of the block
+ * device attached to the BlockBackend. */
+static char *blk_get_attached_dev_id(BlockBackend *blk)
+{
+DeviceState *dev;
+
+assert(!blk->legacy_dev);
+dev = blk->dev;
+
+if (!dev) {
+return g_strdup("");
+} else if (dev->id) {
+return g_strdup(dev->id);
+}
+return object_get_canonical_path(OBJECT(dev));
+}
+
 /*
  * Return the BlockBackend which has the device model @dev attached if it
  * exists, else null.
@@ -613,13 +630,17 @@ void blk_dev_change_media_cb(BlockBackend *blk, bool load)
 if (blk->dev_ops && blk->dev_ops->change_media_cb) {
 bool tray_was_open, tray_is_open;
 
+assert(!blk->legacy_dev);
+
 tray_was_open = blk_dev_is_tray_open(blk);
 blk->dev_ops->change_media_cb(blk->dev_opaque, load);
 tray_is_open = blk_dev_is_tray_open(blk);
 
 if (tray_was_open != tray_is_open) {
-qapi_event_send_device_tray_moved(blk_name(blk), tray_is_open,
+char *id = blk_get_attached_dev_id(blk);
+qapi_event_send_device_tray_moved(blk_name(blk), id, tray_is_open,
   &error_abort);
+g_free(id);
 }
 }
 }
@@ -1325,9 +1346,19 @@ void blk_lock_medium(BlockBackend *blk, bool locked)
 void blk_eject(BlockBackend *blk, bool eject_flag)
 {
 BlockDriverState *bs = blk_bs(blk);
+char *id;
+
+/* blk_eject is only called by qdevified devices */
+assert(!blk->legacy_dev);
 
 if (bs) {
 bdrv_eject(bs, eject_flag);
+
+id = blk_get_attached_dev_id(blk);
+qapi_event_send_device_tray_moved(blk_name(blk), id,
+  eject_flag, &error_abort);
+g_free(id);
+
 }
 }
 
diff --git a/docs/qmp-commands.txt b/docs/qmp-commands.txt
index e0adceb..e044029 100644
--- a/docs/qmp-commands.txt
+++ b/docs/qmp-commands.txt
@@ -3239,6 +3239,7 @@ Example:
 "microseconds": 716996 },
  "event": "DEVICE_TRAY_MOVED",
  "data": { "device": "ide1-cd0",
+   "id": "ide0-1-0",
"tray-open": true } }
 
 <- { "return": {} }
@@ -3267,6 +3268,7 @@ Example:
 "microseconds": 272147 },
  "event": "DEVICE_TRAY_MOVED",
  "data": { "device": "ide1-cd0",
+   "id": "ide0-1-0",
"tray-open": false } }
 
 <- { "return": {} }
@@ -3303,6 +3305,7 @@ Example:
 "microseconds": 549958 },
  "event": "DEVICE_TRAY_MOVED",
  "data": { "device": "ide1-cd0",
+   "id": "ide0-1-0",
"tray-open": true } }
 
 <- { "return": {} }
diff --git a/docs/qmp-events.txt b/docs/qmp-events.txt
index 25307ab..4d56022 100644
--- a/docs/qmp-events.txt
+++ b/docs/qmp-events.txt
@@ -218,12 +218,16 @@ or by HMP/QMP commands.
 
 Data:
 
-- "device": device name (json-string)
+- "device": Block device name. This is always present for compatibility
+reasons, but it can be empty ("") if the image does not have a
+device name associated. (json-string)
+- "id": The name or QOM path of the guest device (json-string)
 - "tray-open": true if the tray has been opened or false if it has been closed
(json-bool)
 
 { "event": "DEVICE_TRAY_MOVED",
   "data": { "device": "ide1-cd0",
+"id": "/machine/unattached/device[22]",