[Qemu-devel] [PATCH v3 0/5]: QMP: add DEVICE_TRAY_MOVED event

[Qemu-devel] [PATCH v3 0/5]: QMP: add DEVICE_TRAY_MOVED event

[Luiz Capitulino]

The event name changed, which caused the subject to change too, hope this
won't cause confusion.

v3

o Rename the event to DEVICE_TRAY_MOVED
o Rename the 'ejected' event data to 'tray-open'
o Only call bdrv_eject() if the tray state changed
o Drop ide_tray_state_post_load()

 QMP/qmp-events.txt |   18 +++++++++++
 block.c            |   84 +++++++++++++++++++++++++++++++++------------------
 block.h            |    8 ++--
 block/raw-posix.c  |    6 ++--
 block/raw.c        |    2 +-
 block_int.h        |    2 +-
 hw/ide/atapi.c     |    7 +++-
 hw/ide/core.c      |   16 ++--------
 hw/scsi-disk.c     |   13 +++++---
 hw/virtio-blk.c    |    6 ++--
 monitor.c          |    3 ++
 monitor.h          |    1 +
 12 files changed, 104 insertions(+), 62 deletions(-)

[Qemu-devel] [PATCH 1/5] block: Rename bdrv_mon_event() & BlockMonEventAction

[Luiz Capitulino]

They are QMP events, not monitor events. Rename them accordingly.

Also, move bdrv_emit_qmp_error_event() up in the file. A new event will
be added soon and it's good to have them next each other.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 block.c         |   58 +++++++++++++++++++++++++++---------------------------
 block.h         |    6 ++--
 hw/ide/core.c   |    6 ++--
 hw/scsi-disk.c  |    6 ++--
 hw/virtio-blk.c |    6 ++--
 5 files changed, 41 insertions(+), 41 deletions(-)

diff --git a/block.c b/block.c
index ae297bb..8d4cfea 100644
--- a/block.c
+++ b/block.c
@@ -943,6 +943,35 @@ void bdrv_set_dev_ops(BlockDriverState *bs, const BlockDevOps *ops,
     }
 }
 
+void bdrv_emit_qmp_error_event(const BlockDriverState *bdrv,
+                               BlockQMPEventAction action, int is_read)
+{
+    QObject *data;
+    const char *action_str;
+
+    switch (action) {
+    case BDRV_ACTION_REPORT:
+        action_str = "report";
+        break;
+    case BDRV_ACTION_IGNORE:
+        action_str = "ignore";
+        break;
+    case BDRV_ACTION_STOP:
+        action_str = "stop";
+        break;
+    default:
+        abort();
+    }
+
+    data = qobject_from_jsonf("{ 'device': %s, 'action': %s, 'operation': %s }",
+                              bdrv->device_name,
+                              action_str,
+                              is_read ? "read" : "write");
+    monitor_protocol_event(QEVENT_BLOCK_IO_ERROR, data);
+
+    qobject_decref(data);
+}
+
 static void bdrv_dev_change_media_cb(BlockDriverState *bs, bool load)
 {
     if (bs->dev_ops && bs->dev_ops->change_media_cb) {
@@ -2293,35 +2322,6 @@ int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
     return data.ret;
 }
 
-void bdrv_mon_event(const BlockDriverState *bdrv,
-                    BlockMonEventAction action, int is_read)
-{
-    QObject *data;
-    const char *action_str;
-
-    switch (action) {
-    case BDRV_ACTION_REPORT:
-        action_str = "report";
-        break;
-    case BDRV_ACTION_IGNORE:
-        action_str = "ignore";
-        break;
-    case BDRV_ACTION_STOP:
-        action_str = "stop";
-        break;
-    default:
-        abort();
-    }
-
-    data = qobject_from_jsonf("{ 'device': %s, 'action': %s, 'operation': %s }",
-                              bdrv->device_name,
-                              action_str,
-                              is_read ? "read" : "write");
-    monitor_protocol_event(QEVENT_BLOCK_IO_ERROR, data);
-
-    qobject_decref(data);
-}
-
 BlockInfoList *qmp_query_block(Error **errp)
 {
     BlockInfoList *head = NULL, *cur_item = NULL;
diff --git a/block.h b/block.h
index 60ea730..ec0a6c8 100644
--- a/block.h
+++ b/block.h
@@ -85,15 +85,15 @@ typedef enum {
 
 typedef enum {
     BDRV_ACTION_REPORT, BDRV_ACTION_IGNORE, BDRV_ACTION_STOP
-} BlockMonEventAction;
+} BlockQMPEventAction;
 
 void bdrv_iostatus_enable(BlockDriverState *bs);
 void bdrv_iostatus_reset(BlockDriverState *bs);
 void bdrv_iostatus_disable(BlockDriverState *bs);
 bool bdrv_iostatus_is_enabled(const BlockDriverState *bs);
 void bdrv_iostatus_set_err(BlockDriverState *bs, int error);
-void bdrv_mon_event(const BlockDriverState *bdrv,
-                    BlockMonEventAction action, int is_read);
+void bdrv_emit_qmp_error_event(const BlockDriverState *bdrv,
+                               BlockQMPEventAction action, int is_read);
 void bdrv_info_print(Monitor *mon, const QObject *data);
 void bdrv_info(Monitor *mon, QObject **ret_data);
 void bdrv_stats_print(Monitor *mon, const QObject *data);
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 56b219b..0856385 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -519,7 +519,7 @@ static int ide_handle_rw_error(IDEState *s, int error, int op)
     BlockErrorAction action = bdrv_get_on_error(s->bs, is_read);
 
     if (action == BLOCK_ERR_IGNORE) {
-        bdrv_mon_event(s->bs, BDRV_ACTION_IGNORE, is_read);
+        bdrv_emit_qmp_error_event(s->bs, BDRV_ACTION_IGNORE, is_read);
         return 0;
     }
 
@@ -527,7 +527,7 @@ static int ide_handle_rw_error(IDEState *s, int error, int op)
             || action == BLOCK_ERR_STOP_ANY) {
         s->bus->dma->ops->set_unit(s->bus->dma, s->unit);
         s->bus->error_status = op;
-        bdrv_mon_event(s->bs, BDRV_ACTION_STOP, is_read);
+        bdrv_emit_qmp_error_event(s->bs, BDRV_ACTION_STOP, is_read);
         vm_stop(RUN_STATE_IO_ERROR);
         bdrv_iostatus_set_err(s->bs, error);
     } else {
@@ -537,7 +537,7 @@ static int ide_handle_rw_error(IDEState *s, int error, int op)
         } else {
             ide_rw_error(s);
         }
-        bdrv_mon_event(s->bs, BDRV_ACTION_REPORT, is_read);
+        bdrv_emit_qmp_error_event(s->bs, BDRV_ACTION_REPORT, is_read);
     }
 
     return 1;
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index c12e3a6..a5d2fd1 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -233,14 +233,14 @@ static int scsi_handle_rw_error(SCSIDiskReq *r, int error)
     BlockErrorAction action = bdrv_get_on_error(s->qdev.conf.bs, is_read);
 
     if (action == BLOCK_ERR_IGNORE) {
-        bdrv_mon_event(s->qdev.conf.bs, BDRV_ACTION_IGNORE, is_read);
+        bdrv_emit_qmp_error_event(s->qdev.conf.bs, BDRV_ACTION_IGNORE, is_read);
         return 0;
     }
 
     if ((error == ENOSPC && action == BLOCK_ERR_STOP_ENOSPC)
             || action == BLOCK_ERR_STOP_ANY) {
 
-        bdrv_mon_event(s->qdev.conf.bs, BDRV_ACTION_STOP, is_read);
+        bdrv_emit_qmp_error_event(s->qdev.conf.bs, BDRV_ACTION_STOP, is_read);
         vm_stop(RUN_STATE_IO_ERROR);
         bdrv_iostatus_set_err(s->qdev.conf.bs, error);
         scsi_req_retry(&r->req);
@@ -259,7 +259,7 @@ static int scsi_handle_rw_error(SCSIDiskReq *r, int error)
             scsi_check_condition(r, SENSE_CODE(IO_ERROR));
             break;
         }
-        bdrv_mon_event(s->qdev.conf.bs, BDRV_ACTION_REPORT, is_read);
+        bdrv_emit_qmp_error_event(s->qdev.conf.bs, BDRV_ACTION_REPORT, is_read);
     }
     return 1;
 }
diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index a5a4396..49990f8 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -69,7 +69,7 @@ static int virtio_blk_handle_rw_error(VirtIOBlockReq *req, int error,
     VirtIOBlock *s = req->dev;
 
     if (action == BLOCK_ERR_IGNORE) {
-        bdrv_mon_event(s->bs, BDRV_ACTION_IGNORE, is_read);
+        bdrv_emit_qmp_error_event(s->bs, BDRV_ACTION_IGNORE, is_read);
         return 0;
     }
 
@@ -77,14 +77,14 @@ static int virtio_blk_handle_rw_error(VirtIOBlockReq *req, int error,
             || action == BLOCK_ERR_STOP_ANY) {
         req->next = s->rq;
         s->rq = req;
-        bdrv_mon_event(s->bs, BDRV_ACTION_STOP, is_read);
+        bdrv_emit_qmp_error_event(s->bs, BDRV_ACTION_STOP, is_read);
         vm_stop(RUN_STATE_IO_ERROR);
         bdrv_iostatus_set_err(s->bs, error);
     } else {
         virtio_blk_req_complete(req, VIRTIO_BLK_S_IOERR);
         bdrv_acct_done(s->bs, &req->acct);
         g_free(req);
-        bdrv_mon_event(s->bs, BDRV_ACTION_REPORT, is_read);
+        bdrv_emit_qmp_error_event(s->bs, BDRV_ACTION_REPORT, is_read);
     }
 
     return 1;
-- 
1.7.9.111.gf3fb0.dirty

[Qemu-devel] [PATCH 2/5] block: bdrv_eject(): Make eject_flag a real bool

[Luiz Capitulino]

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 block.c           |    2 +-
 block.h           |    2 +-
 block/raw-posix.c |    6 +++---
 block/raw.c       |    2 +-
 block_int.h       |    2 +-
 5 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/block.c b/block.c
index 8d4cfea..bfb0dec 100644
--- a/block.c
+++ b/block.c
@@ -3609,7 +3609,7 @@ int bdrv_media_changed(BlockDriverState *bs)
 /**
  * If eject_flag is TRUE, eject the media. Otherwise, close the tray
  */
-void bdrv_eject(BlockDriverState *bs, int eject_flag)
+void bdrv_eject(BlockDriverState *bs, bool eject_flag)
 {
     BlockDriver *drv = bs->drv;
 
diff --git a/block.h b/block.h
index ec0a6c8..49bca5a 100644
--- a/block.h
+++ b/block.h
@@ -265,7 +265,7 @@ int bdrv_enable_write_cache(BlockDriverState *bs);
 int bdrv_is_inserted(BlockDriverState *bs);
 int bdrv_media_changed(BlockDriverState *bs);
 void bdrv_lock_medium(BlockDriverState *bs, bool locked);
-void bdrv_eject(BlockDriverState *bs, int eject_flag);
+void bdrv_eject(BlockDriverState *bs, bool eject_flag);
 void bdrv_get_format(BlockDriverState *bs, char *buf, int buf_size);
 BlockDriverState *bdrv_find(const char *name);
 BlockDriverState *bdrv_next(BlockDriverState *bs);
diff --git a/block/raw-posix.c b/block/raw-posix.c
index 2ee5d69..2d1bc13 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -994,7 +994,7 @@ static int floppy_media_changed(BlockDriverState *bs)
     return ret;
 }
 
-static void floppy_eject(BlockDriverState *bs, int eject_flag)
+static void floppy_eject(BlockDriverState *bs, bool eject_flag)
 {
     BDRVRawState *s = bs->opaque;
     int fd;
@@ -1084,7 +1084,7 @@ static int cdrom_is_inserted(BlockDriverState *bs)
     return 0;
 }
 
-static void cdrom_eject(BlockDriverState *bs, int eject_flag)
+static void cdrom_eject(BlockDriverState *bs, bool eject_flag)
 {
     BDRVRawState *s = bs->opaque;
 
@@ -1194,7 +1194,7 @@ static int cdrom_is_inserted(BlockDriverState *bs)
     return raw_getlength(bs) > 0;
 }
 
-static void cdrom_eject(BlockDriverState *bs, int eject_flag)
+static void cdrom_eject(BlockDriverState *bs, bool eject_flag)
 {
     BDRVRawState *s = bs->opaque;
 
diff --git a/block/raw.c b/block/raw.c
index 6098070..1cdac0c 100644
--- a/block/raw.c
+++ b/block/raw.c
@@ -61,7 +61,7 @@ static int raw_media_changed(BlockDriverState *bs)
     return bdrv_media_changed(bs->file);
 }
 
-static void raw_eject(BlockDriverState *bs, int eject_flag)
+static void raw_eject(BlockDriverState *bs, bool eject_flag)
 {
     bdrv_eject(bs->file, eject_flag);
 }
diff --git a/block_int.h b/block_int.h
index 7946cf6..04f4b83 100644
--- a/block_int.h
+++ b/block_int.h
@@ -197,7 +197,7 @@ struct BlockDriver {
     /* removable device specific */
     int (*bdrv_is_inserted)(BlockDriverState *bs);
     int (*bdrv_media_changed)(BlockDriverState *bs);
-    void (*bdrv_eject)(BlockDriverState *bs, int eject_flag);
+    void (*bdrv_eject)(BlockDriverState *bs, bool eject_flag);
     void (*bdrv_lock_medium)(BlockDriverState *bs, bool locked);
 
     /* to control generic scsi devices */
-- 
1.7.9.111.gf3fb0.dirty

[Qemu-devel] [PATCH 3/5] block: Don't call bdrv_eject() if the tray state didn't change

[Luiz Capitulino]

It's not needed. Besides we can then assume that bdrv_eject() is
only called when there's a tray state change, which is useful to
the DEVICE_TRAY_MOVED event (going to be added in a future
commit).

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 hw/ide/atapi.c |    7 +++++--
 hw/scsi-disk.c |    7 +++++--
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index 0adb27b..5919cf5 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -883,8 +883,11 @@ static void cmd_start_stop_unit(IDEState *s, uint8_t* buf)
             ide_atapi_cmd_error(s, sense, ASC_MEDIA_REMOVAL_PREVENTED);
             return;
         }
-        bdrv_eject(s->bs, !start);
-        s->tray_open = !start;
+
+        if (s->tray_open != !start) {
+            bdrv_eject(s->bs, !start);
+            s->tray_open = !start;
+        }
     }
 
     ide_atapi_cmd_ok(s);
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index a5d2fd1..091ecdc 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -1050,8 +1050,11 @@ static int scsi_disk_emulate_start_stop(SCSIDiskReq *r)
                                  : SENSE_CODE(NOT_READY_REMOVAL_PREVENTED));
             return -1;
         }
-        bdrv_eject(s->qdev.conf.bs, !start);
-        s->tray_open = !start;
+
+        if (s->tray_open != !start) {
+            bdrv_eject(s->qdev.conf.bs, !start);
+            s->tray_open = !start;
+        }
     }
     return 0;
 }
-- 
1.7.9.111.gf3fb0.dirty

[Qemu-devel] [PATCH 4/5] ide: drop ide_tray_state_post_load()

[Luiz Capitulino]

This is used to sync the physical tray state after migration when
using CD-ROM passthrough. However, migrating when using passthrough
is broken anyway and shouldn't be supported...

So, drop this function as it causes a problem with the DEVICE_TRAY_MOVED
event, which is going to be introduced by the next commit.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 hw/ide/core.c |   10 ----------
 1 files changed, 0 insertions(+), 10 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 0856385..ce570a7 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -2077,15 +2077,6 @@ static bool ide_drive_pio_state_needed(void *opaque)
         || (s->bus->error_status & BM_STATUS_PIO_RETRY);
 }
 
-static int ide_tray_state_post_load(void *opaque, int version_id)
-{
-    IDEState *s = opaque;
-
-    bdrv_eject(s->bs, s->tray_open);
-    bdrv_lock_medium(s->bs, s->tray_locked);
-    return 0;
-}
-
 static bool ide_tray_state_needed(void *opaque)
 {
     IDEState *s = opaque;
@@ -2125,7 +2116,6 @@ static const VMStateDescription vmstate_ide_tray_state = {
     .version_id = 1,
     .minimum_version_id = 1,
     .minimum_version_id_old = 1,
-    .post_load = ide_tray_state_post_load,
     .fields = (VMStateField[]) {
         VMSTATE_BOOL(tray_open, IDEState),
         VMSTATE_BOOL(tray_locked, IDEState),
-- 
1.7.9.111.gf3fb0.dirty

[Qemu-devel] [PATCH 5/5] qmp: add DEVICE_TRAY_MOVED event

[Luiz Capitulino]

It's emitted whenever the tray is moved by the guest or by HMP/QMP
commands.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 QMP/qmp-events.txt |   18 ++++++++++++++++++
 block.c            |   24 ++++++++++++++++++++++++
 monitor.c          |    3 +++
 monitor.h          |    1 +
 4 files changed, 46 insertions(+), 0 deletions(-)

diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
index 06cb404..9286af5 100644
--- a/QMP/qmp-events.txt
+++ b/QMP/qmp-events.txt
@@ -26,6 +26,24 @@ Example:
 Note: If action is "stop", a STOP event will eventually follow the
 BLOCK_IO_ERROR event.
 
+DEVICE_TRAY_MOVED
+-----------------
+
+It's emitted whenever the tray of a removable device is moved by the guest
+or by HMP/QMP commands.
+
+Data:
+
+- "device": device name (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",
+            "tray-open": true
+  },
+  "timestamp": { "seconds": 1265044230, "microseconds": 450486 } }
+
 RESET
 -----
 
diff --git a/block.c b/block.c
index bfb0dec..e27d528 100644
--- a/block.c
+++ b/block.c
@@ -972,10 +972,30 @@ void bdrv_emit_qmp_error_event(const BlockDriverState *bdrv,
     qobject_decref(data);
 }
 
+static void bdrv_emit_qmp_eject_event(BlockDriverState *bs, bool ejected)
+{
+    QObject *data;
+
+    data = qobject_from_jsonf("{ 'device': %s, 'tray-open': %i }",
+                              bdrv_get_device_name(bs), ejected);
+    monitor_protocol_event(QEVENT_DEVICE_TRAY_MOVED, data);
+
+    qobject_decref(data);
+}
+
 static void bdrv_dev_change_media_cb(BlockDriverState *bs, bool load)
 {
     if (bs->dev_ops && bs->dev_ops->change_media_cb) {
+        bool tray_was_closed = !bdrv_dev_is_tray_open(bs);
         bs->dev_ops->change_media_cb(bs->dev_opaque, load);
+        if (tray_was_closed) {
+            /* tray open */
+            bdrv_emit_qmp_eject_event(bs, true);
+        }
+        if (load) {
+            /* tray close */
+            bdrv_emit_qmp_eject_event(bs, false);
+        }
     }
 }
 
@@ -3616,6 +3636,10 @@ void bdrv_eject(BlockDriverState *bs, bool eject_flag)
     if (drv && drv->bdrv_eject) {
         drv->bdrv_eject(bs, eject_flag);
     }
+
+    if (bs->device_name[0] != '\0') {
+        bdrv_emit_qmp_eject_event(bs, eject_flag);
+    }
 }
 
 /**
diff --git a/monitor.c b/monitor.c
index aadbdcb..38d876e 100644
--- a/monitor.c
+++ b/monitor.c
@@ -485,6 +485,9 @@ void monitor_protocol_event(MonitorEvent event, QObject *data)
         case QEVENT_BLOCK_JOB_CANCELLED:
             event_name = "BLOCK_JOB_CANCELLED";
             break;
+        case QEVENT_DEVICE_TRAY_MOVED:
+             event_name = "DEVICE_TRAY_MOVED";
+            break;
         default:
             abort();
             break;
diff --git a/monitor.h b/monitor.h
index b72ea07..b6f700f 100644
--- a/monitor.h
+++ b/monitor.h
@@ -38,6 +38,7 @@ typedef enum MonitorEvent {
     QEVENT_SPICE_DISCONNECTED,
     QEVENT_BLOCK_JOB_COMPLETED,
     QEVENT_BLOCK_JOB_CANCELLED,
+    QEVENT_DEVICE_TRAY_MOVED,
     QEVENT_MAX,
 } MonitorEvent;
 
-- 
1.7.9.111.gf3fb0.dirty

Re: [Qemu-devel] [PATCH 3/5] block: Don't call bdrv_eject() if the tray state didn't change

[Markus Armbruster]

Luiz Capitulino <lcapitulino@redhat.com> writes:

> It's not needed. Besides we can then assume that bdrv_eject() is
> only called when there's a tray state change, which is useful to
> the DEVICE_TRAY_MOVED event (going to be added in a future
> commit).

We can assume that only after the next patch "ide: drop
ide_tray_state_post_load()", actually.  I'd swap the two.  Doubtful
whether it's worth a respin, though.

Re: [Qemu-devel] [PATCH 4/5] ide: drop ide_tray_state_post_load()

[Markus Armbruster]

Luiz Capitulino <lcapitulino@redhat.com> writes:

> This is used to sync the physical tray state after migration when
> using CD-ROM passthrough. However, migrating when using passthrough
> is broken anyway and shouldn't be supported...
>
> So, drop this function as it causes a problem with the DEVICE_TRAY_MOVED
> event, which is going to be introduced by the next commit.

I added ide_tray_state_post_load() mostly to satisfy an urge to always
call bdrv_eject() when tray_state changes, not because it's actually
useful.  I'm fine with dropping it.  Perhaps preventing migration while
the host CD-ROM is passed through would be even better.

Re: [Qemu-devel] [PATCH 5/5] qmp: add DEVICE_TRAY_MOVED event

[Markus Armbruster]

Luiz Capitulino <lcapitulino@redhat.com> writes:

> It's emitted whenever the tray is moved by the guest or by HMP/QMP
> commands.
>
> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> ---
>  QMP/qmp-events.txt |   18 ++++++++++++++++++
>  block.c            |   24 ++++++++++++++++++++++++
>  monitor.c          |    3 +++
>  monitor.h          |    1 +
>  4 files changed, 46 insertions(+), 0 deletions(-)
>
> diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
> index 06cb404..9286af5 100644
> --- a/QMP/qmp-events.txt
> +++ b/QMP/qmp-events.txt
> @@ -26,6 +26,24 @@ Example:
>  Note: If action is "stop", a STOP event will eventually follow the
>  BLOCK_IO_ERROR event.
>  
> +DEVICE_TRAY_MOVED
> +-----------------
> +
> +It's emitted whenever the tray of a removable device is moved by the guest
> +or by HMP/QMP commands.
> +
> +Data:
> +
> +- "device": device name (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",
> +            "tray-open": true
> +  },
> +  "timestamp": { "seconds": 1265044230, "microseconds": 450486 } }
> +
>  RESET
>  -----
>  
> diff --git a/block.c b/block.c
> index bfb0dec..e27d528 100644
> --- a/block.c
> +++ b/block.c
> @@ -972,10 +972,30 @@ void bdrv_emit_qmp_error_event(const BlockDriverState *bdrv,
>      qobject_decref(data);
>  }
>  
> +static void bdrv_emit_qmp_eject_event(BlockDriverState *bs, bool ejected)
> +{
> +    QObject *data;
> +
> +    data = qobject_from_jsonf("{ 'device': %s, 'tray-open': %i }",
> +                              bdrv_get_device_name(bs), ejected);
> +    monitor_protocol_event(QEVENT_DEVICE_TRAY_MOVED, data);
> +
> +    qobject_decref(data);
> +}
> +
>  static void bdrv_dev_change_media_cb(BlockDriverState *bs, bool load)
>  {
>      if (bs->dev_ops && bs->dev_ops->change_media_cb) {
> +        bool tray_was_closed = !bdrv_dev_is_tray_open(bs);
>          bs->dev_ops->change_media_cb(bs->dev_opaque, load);
> +        if (tray_was_closed) {
> +            /* tray open */
> +            bdrv_emit_qmp_eject_event(bs, true);
> +        }
> +        if (load) {
> +            /* tray close */
> +            bdrv_emit_qmp_eject_event(bs, false);
> +        }
>      }
>  }
>  

This one's subtle.  I discussed why it works in my review of v2, but
I'll be hanged if I remember now.  I'd ask for a comment, if I hadn't
already asked for replacing the BlockDevOps method by one that's easier
to use, in a followup patch.

> @@ -3616,6 +3636,10 @@ void bdrv_eject(BlockDriverState *bs, bool eject_flag)
>      if (drv && drv->bdrv_eject) {
>          drv->bdrv_eject(bs, eject_flag);
>      }
> +
> +    if (bs->device_name[0] != '\0') {
> +        bdrv_emit_qmp_eject_event(bs, eject_flag);
> +    }

Condition needed when we stack "raw" on top of another driver.
raw_eject() uses bdrv_eject() to pass down the call.

Only the topmost driver ("raw" in this case) has a non-empty
device_name.

Perhaps worth a comment.

>  }
>  
>  /**
> diff --git a/monitor.c b/monitor.c
> index aadbdcb..38d876e 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -485,6 +485,9 @@ void monitor_protocol_event(MonitorEvent event, QObject *data)
>          case QEVENT_BLOCK_JOB_CANCELLED:
>              event_name = "BLOCK_JOB_CANCELLED";
>              break;
> +        case QEVENT_DEVICE_TRAY_MOVED:
> +             event_name = "DEVICE_TRAY_MOVED";
> +            break;
>          default:
>              abort();
>              break;
> diff --git a/monitor.h b/monitor.h
> index b72ea07..b6f700f 100644
> --- a/monitor.h
> +++ b/monitor.h
> @@ -38,6 +38,7 @@ typedef enum MonitorEvent {
>      QEVENT_SPICE_DISCONNECTED,
>      QEVENT_BLOCK_JOB_COMPLETED,
>      QEVENT_BLOCK_JOB_CANCELLED,
> +    QEVENT_DEVICE_TRAY_MOVED,
>      QEVENT_MAX,
>  } MonitorEvent;

Re: [Qemu-devel] [PATCH v3 0/5]: QMP: add DEVICE_TRAY_MOVED event

[Markus Armbruster]

Luiz Capitulino <lcapitulino@redhat.com> writes:

> The event name changed, which caused the subject to change too, hope this
> won't cause confusion.
>
> v3
>
> o Rename the event to DEVICE_TRAY_MOVED
> o Rename the 'ejected' event data to 'tray-open'
> o Only call bdrv_eject() if the tray state changed
> o Drop ide_tray_state_post_load()

Reviewed-by: Markus Armbruster <armbru@redhat.com>

Re: [Qemu-devel] [PATCH v3 0/5]: QMP: add DEVICE_TRAY_MOVED event

[Luiz Capitulino]

On Mon, 20 Feb 2012 09:28:15 +0100
Markus Armbruster <armbru@redhat.com> wrote:

> Luiz Capitulino <lcapitulino@redhat.com> writes:
> 
> > The event name changed, which caused the subject to change too, hope this
> > won't cause confusion.
> >
> > v3
> >
> > o Rename the event to DEVICE_TRAY_MOVED
> > o Rename the 'ejected' event data to 'tray-open'
> > o Only call bdrv_eject() if the tray state changed
> > o Drop ide_tray_state_post_load()
> 
> Reviewed-by: Markus Armbruster <armbru@redhat.com>

Kevin, I'd like to get your ack before applying it to the qmp branch.

Re: [Qemu-devel] [PATCH 5/5] qmp: add DEVICE_TRAY_MOVED event

[Kevin Wolf]

Am 17.02.2012 20:21, schrieb Luiz Capitulino:
> It's emitted whenever the tray is moved by the guest or by HMP/QMP
> commands.
> 
> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> ---
>  QMP/qmp-events.txt |   18 ++++++++++++++++++
>  block.c            |   24 ++++++++++++++++++++++++
>  monitor.c          |    3 +++
>  monitor.h          |    1 +
>  4 files changed, 46 insertions(+), 0 deletions(-)
> 
> diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
> index 06cb404..9286af5 100644
> --- a/QMP/qmp-events.txt
> +++ b/QMP/qmp-events.txt
> @@ -26,6 +26,24 @@ Example:
>  Note: If action is "stop", a STOP event will eventually follow the
>  BLOCK_IO_ERROR event.
>  
> +DEVICE_TRAY_MOVED
> +-----------------
> +
> +It's emitted whenever the tray of a removable device is moved by the guest
> +or by HMP/QMP commands.
> +
> +Data:
> +
> +- "device": device name (json-string)

For me, a device name is something related to qdev. 'device' is a
misnomer consistently used in all QMP commands so far and we can't fix
it any more, but at least the documentation should clarify what is meant
(that's for a follow-up patch).

Kevin

Re: [Qemu-devel] [PATCH v3 0/5]: QMP: add DEVICE_TRAY_MOVED event

[Kevin Wolf]

Am 22.02.2012 13:53, schrieb Luiz Capitulino:
> On Mon, 20 Feb 2012 09:28:15 +0100
> Markus Armbruster <armbru@redhat.com> wrote:
> 
>> Luiz Capitulino <lcapitulino@redhat.com> writes:
>>
>>> The event name changed, which caused the subject to change too, hope this
>>> won't cause confusion.
>>>
>>> v3
>>>
>>> o Rename the event to DEVICE_TRAY_MOVED
>>> o Rename the 'ejected' event data to 'tray-open'
>>> o Only call bdrv_eject() if the tray state changed
>>> o Drop ide_tray_state_post_load()
>>
>> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> 
> Kevin, I'd like to get your ack before applying it to the qmp branch.

Acked-by: Kevin Wolf <kwolf@redhat.com>

Re: [Qemu-devel] [PATCH 5/5] qmp: add DEVICE_TRAY_MOVED event

[Markus Armbruster]

Kevin Wolf <kwolf@redhat.com> writes:

> Am 17.02.2012 20:21, schrieb Luiz Capitulino:
>> It's emitted whenever the tray is moved by the guest or by HMP/QMP
>> commands.
>> 
>> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
>> ---
>>  QMP/qmp-events.txt |   18 ++++++++++++++++++
>>  block.c            |   24 ++++++++++++++++++++++++
>>  monitor.c          |    3 +++
>>  monitor.h          |    1 +
>>  4 files changed, 46 insertions(+), 0 deletions(-)
>> 
>> diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
>> index 06cb404..9286af5 100644
>> --- a/QMP/qmp-events.txt
>> +++ b/QMP/qmp-events.txt
>> @@ -26,6 +26,24 @@ Example:
>>  Note: If action is "stop", a STOP event will eventually follow the
>>  BLOCK_IO_ERROR event.
>>  
>> +DEVICE_TRAY_MOVED
>> +-----------------
>> +
>> +It's emitted whenever the tray of a removable device is moved by the guest
>> +or by HMP/QMP commands.
>> +
>> +Data:
>> +
>> +- "device": device name (json-string)
>
> For me, a device name is something related to qdev. 'device' is a
> misnomer consistently used in all QMP commands so far and we can't fix
> it any more, but at least the documentation should clarify what is meant
> (that's for a follow-up patch).

We can fix it if we really want to: rename, then add the old name as
alias for backward compatibility.  Pick your favourite flavor of cruft.

Re: [Qemu-devel] [PATCH 5/5] qmp: add DEVICE_TRAY_MOVED event

[Luiz Capitulino]

On Thu, 23 Feb 2012 08:50:08 +0100
Markus Armbruster <armbru@redhat.com> wrote:

> Kevin Wolf <kwolf@redhat.com> writes:
> 
> > Am 17.02.2012 20:21, schrieb Luiz Capitulino:
> >> It's emitted whenever the tray is moved by the guest or by HMP/QMP
> >> commands.
> >> 
> >> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> >> ---
> >>  QMP/qmp-events.txt |   18 ++++++++++++++++++
> >>  block.c            |   24 ++++++++++++++++++++++++
> >>  monitor.c          |    3 +++
> >>  monitor.h          |    1 +
> >>  4 files changed, 46 insertions(+), 0 deletions(-)
> >> 
> >> diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
> >> index 06cb404..9286af5 100644
> >> --- a/QMP/qmp-events.txt
> >> +++ b/QMP/qmp-events.txt
> >> @@ -26,6 +26,24 @@ Example:
> >>  Note: If action is "stop", a STOP event will eventually follow the
> >>  BLOCK_IO_ERROR event.
> >>  
> >> +DEVICE_TRAY_MOVED
> >> +-----------------
> >> +
> >> +It's emitted whenever the tray of a removable device is moved by the guest
> >> +or by HMP/QMP commands.
> >> +
> >> +Data:
> >> +
> >> +- "device": device name (json-string)
> >
> > For me, a device name is something related to qdev. 'device' is a
> > misnomer consistently used in all QMP commands so far and we can't fix
> > it any more, but at least the documentation should clarify what is meant
> > (that's for a follow-up patch).
> 
> We can fix it if we really want to: rename, then add the old name as
> alias for backward compatibility.  Pick your favourite flavor of cruft.

I like it, new events won't have the cruft.

Re: [Qemu-devel] [PATCH 5/5] qmp: add DEVICE_TRAY_MOVED event

[Markus Armbruster]

Luiz Capitulino <lcapitulino@redhat.com> writes:

> On Thu, 23 Feb 2012 08:50:08 +0100
> Markus Armbruster <armbru@redhat.com> wrote:
>
>> Kevin Wolf <kwolf@redhat.com> writes:
>> 
>> > Am 17.02.2012 20:21, schrieb Luiz Capitulino:
>> >> It's emitted whenever the tray is moved by the guest or by HMP/QMP
>> >> commands.
>> >> 
>> >> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
>> >> ---
>> >>  QMP/qmp-events.txt |   18 ++++++++++++++++++
>> >>  block.c            |   24 ++++++++++++++++++++++++
>> >>  monitor.c          |    3 +++
>> >>  monitor.h          |    1 +
>> >>  4 files changed, 46 insertions(+), 0 deletions(-)
>> >> 
>> >> diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
>> >> index 06cb404..9286af5 100644
>> >> --- a/QMP/qmp-events.txt
>> >> +++ b/QMP/qmp-events.txt
>> >> @@ -26,6 +26,24 @@ Example:
>> >>  Note: If action is "stop", a STOP event will eventually follow the
>> >>  BLOCK_IO_ERROR event.
>> >>  
>> >> +DEVICE_TRAY_MOVED
>> >> +-----------------
>> >> +
>> >> +It's emitted whenever the tray of a removable device is moved by the guest
>> >> +or by HMP/QMP commands.
>> >> +
>> >> +Data:
>> >> +
>> >> +- "device": device name (json-string)
>> >
>> > For me, a device name is something related to qdev. 'device' is a
>> > misnomer consistently used in all QMP commands so far and we can't fix
>> > it any more, but at least the documentation should clarify what is meant
>> > (that's for a follow-up patch).
>> 
>> We can fix it if we really want to: rename, then add the old name as
>> alias for backward compatibility.  Pick your favourite flavor of cruft.
>
> I like it, new events won't have the cruft.

If we reserve "device" for device models, we need sensible names for
device backends.  One each for block, net and char.  There's some
precedence for "blockdev", "netdev", "chardev", but they contain "dev",
so there's still some overloading of the name "device".  Better ideas?

Re: [Qemu-devel] [PATCH 5/5] qmp: add DEVICE_TRAY_MOVED event

[Anthony Liguori]

On 02/23/2012 08:08 AM, Markus Armbruster wrote:
> Luiz Capitulino<lcapitulino@redhat.com>  writes:
>
>> On Thu, 23 Feb 2012 08:50:08 +0100
>> Markus Armbruster<armbru@redhat.com>  wrote:
>>
>>> Kevin Wolf<kwolf@redhat.com>  writes:
>>>
>>>> Am 17.02.2012 20:21, schrieb Luiz Capitulino:
>>>>> It's emitted whenever the tray is moved by the guest or by HMP/QMP
>>>>> commands.
>>>>>
>>>>> Signed-off-by: Luiz Capitulino<lcapitulino@redhat.com>
>>>>> ---
>>>>>   QMP/qmp-events.txt |   18 ++++++++++++++++++
>>>>>   block.c            |   24 ++++++++++++++++++++++++
>>>>>   monitor.c          |    3 +++
>>>>>   monitor.h          |    1 +
>>>>>   4 files changed, 46 insertions(+), 0 deletions(-)
>>>>>
>>>>> diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
>>>>> index 06cb404..9286af5 100644
>>>>> --- a/QMP/qmp-events.txt
>>>>> +++ b/QMP/qmp-events.txt
>>>>> @@ -26,6 +26,24 @@ Example:
>>>>>   Note: If action is "stop", a STOP event will eventually follow the
>>>>>   BLOCK_IO_ERROR event.
>>>>>
>>>>> +DEVICE_TRAY_MOVED
>>>>> +-----------------
>>>>> +
>>>>> +It's emitted whenever the tray of a removable device is moved by the guest
>>>>> +or by HMP/QMP commands.
>>>>> +
>>>>> +Data:
>>>>> +
>>>>> +- "device": device name (json-string)
>>>>
>>>> For me, a device name is something related to qdev. 'device' is a
>>>> misnomer consistently used in all QMP commands so far and we can't fix
>>>> it any more, but at least the documentation should clarify what is meant
>>>> (that's for a follow-up patch).
>>>
>>> We can fix it if we really want to: rename, then add the old name as
>>> alias for backward compatibility.  Pick your favourite flavor of cruft.
>>
>> I like it, new events won't have the cruft.
>
> If we reserve "device" for device models, we need sensible names for
> device backends.  One each for block, net and char.  There's some
> precedence for "blockdev", "netdev", "chardev", but they contain "dev",
> so there's still some overloading of the name "device".  Better ideas?

For 1.2 (when QOM is considered stable), this should become pathname.  Given a 
path, a client can determine what the type of the object is (by reading the type 
property).

In fact, I'd like to see all events have a pathname of origin.  This would 
probably become part of the QMP protocol itself.

This gives us a mechanism to subscribe to events from specific objects.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 5/5] qmp: add DEVICE_TRAY_MOVED event

[Luiz Capitulino]

On Fri, 24 Feb 2012 10:01:00 -0600
Anthony Liguori <anthony@codemonkey.ws> wrote:

> On 02/23/2012 08:08 AM, Markus Armbruster wrote:
> > Luiz Capitulino<lcapitulino@redhat.com>  writes:
> >
> >> On Thu, 23 Feb 2012 08:50:08 +0100
> >> Markus Armbruster<armbru@redhat.com>  wrote:
> >>
> >>> Kevin Wolf<kwolf@redhat.com>  writes:
> >>>
> >>>> Am 17.02.2012 20:21, schrieb Luiz Capitulino:
> >>>>> It's emitted whenever the tray is moved by the guest or by HMP/QMP
> >>>>> commands.
> >>>>>
> >>>>> Signed-off-by: Luiz Capitulino<lcapitulino@redhat.com>
> >>>>> ---
> >>>>>   QMP/qmp-events.txt |   18 ++++++++++++++++++
> >>>>>   block.c            |   24 ++++++++++++++++++++++++
> >>>>>   monitor.c          |    3 +++
> >>>>>   monitor.h          |    1 +
> >>>>>   4 files changed, 46 insertions(+), 0 deletions(-)
> >>>>>
> >>>>> diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
> >>>>> index 06cb404..9286af5 100644
> >>>>> --- a/QMP/qmp-events.txt
> >>>>> +++ b/QMP/qmp-events.txt
> >>>>> @@ -26,6 +26,24 @@ Example:
> >>>>>   Note: If action is "stop", a STOP event will eventually follow the
> >>>>>   BLOCK_IO_ERROR event.
> >>>>>
> >>>>> +DEVICE_TRAY_MOVED
> >>>>> +-----------------
> >>>>> +
> >>>>> +It's emitted whenever the tray of a removable device is moved by the guest
> >>>>> +or by HMP/QMP commands.
> >>>>> +
> >>>>> +Data:
> >>>>> +
> >>>>> +- "device": device name (json-string)
> >>>>
> >>>> For me, a device name is something related to qdev. 'device' is a
> >>>> misnomer consistently used in all QMP commands so far and we can't fix
> >>>> it any more, but at least the documentation should clarify what is meant
> >>>> (that's for a follow-up patch).
> >>>
> >>> We can fix it if we really want to: rename, then add the old name as
> >>> alias for backward compatibility.  Pick your favourite flavor of cruft.
> >>
> >> I like it, new events won't have the cruft.
> >
> > If we reserve "device" for device models, we need sensible names for
> > device backends.  One each for block, net and char.  There's some
> > precedence for "blockdev", "netdev", "chardev", but they contain "dev",
> > so there's still some overloading of the name "device".  Better ideas?
> 
> For 1.2 (when QOM is considered stable), this should become pathname.  Given a 
> path, a client can determine what the type of the object is (by reading the type 
> property).
> 
> In fact, I'd like to see all events have a pathname of origin.  This would 
> probably become part of the QMP protocol itself.
> 
> This gives us a mechanism to subscribe to events from specific objects.

This is 1.2 material, right?

I'm asking because the conversion of events to the qapi is not too far away,
but I think that using QOM will somewhat deprecate the code you have in the
glib branch (besides having to wait for 1.2)?

Re: [Qemu-devel] [PATCH 5/5] qmp: add DEVICE_TRAY_MOVED event

[Anthony Liguori]

On 02/24/2012 10:39 AM, Luiz Capitulino wrote:
> On Fri, 24 Feb 2012 10:01:00 -0600
> Anthony Liguori<anthony@codemonkey.ws>  wrote:
>
>> On 02/23/2012 08:08 AM, Markus Armbruster wrote:
>>> Luiz Capitulino<lcapitulino@redhat.com>   writes:
>>>
>>>> On Thu, 23 Feb 2012 08:50:08 +0100
>>>> Markus Armbruster<armbru@redhat.com>   wrote:
>>>>
>>>>> Kevin Wolf<kwolf@redhat.com>   writes:
>>>>>
>>>>>> Am 17.02.2012 20:21, schrieb Luiz Capitulino:
>>>>>>> It's emitted whenever the tray is moved by the guest or by HMP/QMP
>>>>>>> commands.
>>>>>>>
>>>>>>> Signed-off-by: Luiz Capitulino<lcapitulino@redhat.com>
>>>>>>> ---
>>>>>>>    QMP/qmp-events.txt |   18 ++++++++++++++++++
>>>>>>>    block.c            |   24 ++++++++++++++++++++++++
>>>>>>>    monitor.c          |    3 +++
>>>>>>>    monitor.h          |    1 +
>>>>>>>    4 files changed, 46 insertions(+), 0 deletions(-)
>>>>>>>
>>>>>>> diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
>>>>>>> index 06cb404..9286af5 100644
>>>>>>> --- a/QMP/qmp-events.txt
>>>>>>> +++ b/QMP/qmp-events.txt
>>>>>>> @@ -26,6 +26,24 @@ Example:
>>>>>>>    Note: If action is "stop", a STOP event will eventually follow the
>>>>>>>    BLOCK_IO_ERROR event.
>>>>>>>
>>>>>>> +DEVICE_TRAY_MOVED
>>>>>>> +-----------------
>>>>>>> +
>>>>>>> +It's emitted whenever the tray of a removable device is moved by the guest
>>>>>>> +or by HMP/QMP commands.
>>>>>>> +
>>>>>>> +Data:
>>>>>>> +
>>>>>>> +- "device": device name (json-string)
>>>>>>
>>>>>> For me, a device name is something related to qdev. 'device' is a
>>>>>> misnomer consistently used in all QMP commands so far and we can't fix
>>>>>> it any more, but at least the documentation should clarify what is meant
>>>>>> (that's for a follow-up patch).
>>>>>
>>>>> We can fix it if we really want to: rename, then add the old name as
>>>>> alias for backward compatibility.  Pick your favourite flavor of cruft.
>>>>
>>>> I like it, new events won't have the cruft.
>>>
>>> If we reserve "device" for device models, we need sensible names for
>>> device backends.  One each for block, net and char.  There's some
>>> precedence for "blockdev", "netdev", "chardev", but they contain "dev",
>>> so there's still some overloading of the name "device".  Better ideas?
>>
>> For 1.2 (when QOM is considered stable), this should become pathname.  Given a
>> path, a client can determine what the type of the object is (by reading the type
>> property).
>>
>> In fact, I'd like to see all events have a pathname of origin.  This would
>> probably become part of the QMP protocol itself.
>>
>> This gives us a mechanism to subscribe to events from specific objects.
>
> This is 1.2 material, right?

Yes.

> I'm asking because the conversion of events to the qapi is not too far away,
> but I think that using QOM will somewhat deprecate the code you have in the
> glib branch (besides having to wait for 1.2)?

I have some vague ideas about what to do here.  One thought would be to have a 
standard notifier mechanism in Object that was advertised as a property type. 
We could then provide an interface via QMP to [un]subscribe to a notifier property.

I won't get to this until the 1.2 time frame though.  My goals for 1.1 are to 
get qbus conversions merged and refactor IRQs/MemoryRegions to use QOM.  If time 
permits, also refactor the PC to better use QOM.

If someone wants to tackle events in QOM, I'd be happy to provide some 
suggestions on where to start.

Regards,

Anthony Liguori

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 5/5] qmp: add DEVICE_TRAY_MOVED event

[Luiz Capitulino]

On Fri, 24 Feb 2012 10:44:11 -0600
Anthony Liguori <anthony@codemonkey.ws> wrote:

> > I'm asking because the conversion of events to the qapi is not too far away,
> > but I think that using QOM will somewhat deprecate the code you have in the
> > glib branch (besides having to wait for 1.2)?
> 
> I have some vague ideas about what to do here.  One thought would be to have a 
> standard notifier mechanism in Object that was advertised as a property type. 
> We could then provide an interface via QMP to [un]subscribe to a notifier property.

This seems to be a good match with your previous ideas, implemented in the glib
branch. But then subsystems/devices emitting events will have to be converted
to QOM first...

> I won't get to this until the 1.2 time frame though.  My goals for 1.1 are to 
> get qbus conversions merged and refactor IRQs/MemoryRegions to use QOM.  If time 
> permits, also refactor the PC to better use QOM.
> 
> If someone wants to tackle events in QOM, I'd be happy to provide some 
> suggestions on where to start.

I'd like to hear about it, but I'm not sure when I'll start working on it.

[Qemu-devel] New QMP event interface (was Re: [PATCH 5/5] qmp: add DEVICE_TRAY_MOVED event)

[Anthony Liguori]

On 02/24/2012 10:56 AM, Luiz Capitulino wrote:
> On Fri, 24 Feb 2012 10:44:11 -0600
> Anthony Liguori<anthony@codemonkey.ws>  wrote:
>
>>> I'm asking because the conversion of events to the qapi is not too far away,
>>> but I think that using QOM will somewhat deprecate the code you have in the
>>> glib branch (besides having to wait for 1.2)?
>>
>> I have some vague ideas about what to do here.  One thought would be to have a
>> standard notifier mechanism in Object that was advertised as a property type.
>> We could then provide an interface via QMP to [un]subscribe to a notifier property.
>
> This seems to be a good match with your previous ideas, implemented in the glib
> branch. But then subsystems/devices emitting events will have to be converted
> to QOM first...

Well we need to keep the old events for compatibility, so it's really just about 
new events.  I think that by end of 1.2, we would have all non-qdev subsystems 
converted to QOM also so this shouldn't be a problem in practice.

>
>> I won't get to this until the 1.2 time frame though.  My goals for 1.1 are to
>> get qbus conversions merged and refactor IRQs/MemoryRegions to use QOM.  If time
>> permits, also refactor the PC to better use QOM.
>>
>> If someone wants to tackle events in QOM, I'd be happy to provide some
>> suggestions on where to start.
>
> I'd like to hear about it, but I'm not sure when I'll start working on it.

I've only thought about this roughly, but the problems that need to be solved are:

1) Have an object property that corresponds to a NotifierList (easy)

2) Figure out what it means to "get" and "set" a NotifierList.  I think perhaps 
we could somehow turn this into subscribe/unsubscribe...

We could take a plan9-like approach where "get" would return the canonical path 
of a new object that corresponded to a subscription to the event.  This is nice 
because unsubscribing then just becomes a matter of destroying the subscription 
object.

Once you had this subscription object, I think we would want a mechanism to 
"connect" the subscription with either a native function pointer or some 
mechanism that would let us translate that to QMP.  Maybe we only connect with a 
native function pointer and use QAPI generation code to build a native function 
pointer that spits out a marshalled QObject.

3) We would need to think through the QMP interface for all of this.  Given a 
path, we want to be able to subscribe to an event and unsubscribe from an event. 
  We need to unsubscribe all subscribed events when the connection is lost.  It 
would be nice to have convenience interfaces for doing things like subscribing 
to any event on a given type too including yet to be created objects.

Cc'ing libvirt here to see if they have any additional requirements.

Regards,

Anthony Liguori