Re: [Qemu-devel] [PATCH 4/4] qmp: add BLOCK_MEDIUM_EJECT event

[Markus Armbruster]

Luiz Capitulino <address@hidden> writes:

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

I like the simplicity of this patch.  A few remarks inline.

> Signed-off-by: Luiz Capitulino <address@hidden>
> ---
>  QMP/qmp-events.txt |   17 +++++++++++++++++
>  block.c            |   24 ++++++++++++++++++++++++
>  monitor.c          |    3 +++
>  monitor.h          |    1 +
>  4 files changed, 45 insertions(+), 0 deletions(-)
>
> diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
> index 06cb404..c52e7fe 100644
> --- a/QMP/qmp-events.txt
> +++ b/QMP/qmp-events.txt
> @@ -26,6 +26,23 @@ Example:
>  Note: If action is "stop", a STOP event will eventually follow the
>  BLOCK_IO_ERROR event.
>  
> +BLOCK_MEDIUM_EJECT
> +------------------
> +
> +It's emitted whenever the tray is moved by the guest or by HMP/QMP
> +commands.
> +
> +Data:
> +
> +- "device": device name (json-string)
> +- "ejected": true if the tray has been opened or false if it has been closed

I'd make this "load", because I find "true to load, false to eject" more
pleasing, but that's strictly a matter of taste.

> +
> +{ "event": "BLOCK_MEDIUM_EJECT",
> +  "data": { "device": "ide1-cd0",
> +            "ejected": true
> +  },
> +  "timestamp": { "seconds": 1265044230, "microseconds": 450486 } }
> +
>  RESET
>  -----
>  
> diff --git a/block.c b/block.c
> index 47f1823..e5e2a5f 100644
> --- a/block.c
> +++ b/block.c
> @@ -970,10 +970,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, 'ejected': %i }",
> +                              bdrv_get_device_name(bs), ejected);
> +    monitor_protocol_event(QEVENT_BLOCK_MEDIUM_EJECT, 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);

Emits event even when tray stays closed (tray_was_closed && !load),
doesn't it?

> +        }
> +        if (load) {
> +            /* tray close */
> +            bdrv_emit_qmp_eject_event(bs, false);

Likewise (!tray_was_closed && load)?

> +        }
>      }
>  }
>  
> @@ -3567,6 +3587,10 @@ void bdrv_eject(BlockDriverState *bs, bool eject_flag, 
> bool tray_changed)
>      if (drv && drv->bdrv_eject) {
>          drv->bdrv_eject(bs, eject_flag);
>      }
> +
> +    if (tray_changed) {
> +        bdrv_emit_qmp_eject_event(bs, eject_flag);
> +    }

I wonder why we bother to call bdrv_eject() with false tray_changed at
all.  If there's no good reason for that, we could replace PATCH 3/4 by
one that simply skips the call when the virtual tray stays put.

>  }
>  
>  /**
> diff --git a/monitor.c b/monitor.c
> index aadbdcb..1758f03 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_BLOCK_MEDIUM_EJECT:
> +             event_name = "BLOCK_MEDIUM_EJECT";
> +            break;
>          default:
>              abort();
>              break;
> diff --git a/monitor.h b/monitor.h
> index b72ea07..a2555e5 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_BLOCK_MEDIUM_EJECT,
>      QEVENT_MAX,
>  } MonitorEvent;