[Qemu-devel] [PATCH] Add 'query-events' command to QMP to query async events

[Qemu-devel] [PATCH] Add 'query-events' command to QMP to query async events

[Daniel P. Berrange]

From: "Daniel P. Berrange" <berrange@redhat.com>

Sometimes it is neccessary for an application to determine
whether a particular QMP event is available, so they can
decide whether to use compatibility code instead. This
introduces a new 'query-events' command to QMP todo just
that

 { "execute": "query-events" }
 {"return": [{"name": "WAKEUP"},
             {"name": "SUSPEND"},
             {"name": "DEVICE_TRAY_MOVED"},
             {"name": "BLOCK_JOB_CANCELLED"},
             {"name": "BLOCK_JOB_COMPLETED"},
             ...snip...
             {"name": "SHUTDOWN"}]}

* monitor.c: Split out MonitorEvent -> string conversion
  into monitor_protocol_event_name() API. Add impl of
  qmp_query_events monitor command handler
* qapi-schema.json, qmp-commands.hx: Define contract of
  query-events command

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 monitor.c        |   93 ++++++++++++++++++++++++++++++++++-------------------
 qapi-schema.json |   22 +++++++++++++
 qmp-commands.hx  |   37 +++++++++++++++++++++
 3 files changed, 119 insertions(+), 33 deletions(-)

diff --git a/monitor.c b/monitor.c
index ef59cd9..73ecf21 100644
--- a/monitor.c
+++ b/monitor.c
@@ -422,84 +422,93 @@ static void timestamp_put(QDict *qdict)
     qdict_put_obj(qdict, "timestamp", obj);
 }
 
-/**
- * monitor_protocol_event(): Generate a Monitor event
- *
- * Event-specific data can be emitted through the (optional) 'data' parameter.
- */
-void monitor_protocol_event(MonitorEvent event, QObject *data)
-{
-    QDict *qmp;
-    const char *event_name;
-    Monitor *mon;
-
-    assert(event < QEVENT_MAX);
 
+static const char *monitor_protocol_event_name(MonitorEvent event)
+{
     switch (event) {
         case QEVENT_SHUTDOWN:
-            event_name = "SHUTDOWN";
+            return "SHUTDOWN";
             break;
         case QEVENT_RESET:
-            event_name = "RESET";
+            return "RESET";
             break;
         case QEVENT_POWERDOWN:
-            event_name = "POWERDOWN";
+            return "POWERDOWN";
             break;
         case QEVENT_STOP:
-            event_name = "STOP";
+            return "STOP";
             break;
         case QEVENT_RESUME:
-            event_name = "RESUME";
+            return "RESUME";
             break;
         case QEVENT_VNC_CONNECTED:
-            event_name = "VNC_CONNECTED";
+            return "VNC_CONNECTED";
             break;
         case QEVENT_VNC_INITIALIZED:
-            event_name = "VNC_INITIALIZED";
+            return "VNC_INITIALIZED";
             break;
         case QEVENT_VNC_DISCONNECTED:
-            event_name = "VNC_DISCONNECTED";
+            return "VNC_DISCONNECTED";
             break;
         case QEVENT_BLOCK_IO_ERROR:
-            event_name = "BLOCK_IO_ERROR";
+            return "BLOCK_IO_ERROR";
             break;
         case QEVENT_RTC_CHANGE:
-            event_name = "RTC_CHANGE";
+            return "RTC_CHANGE";
             break;
         case QEVENT_WATCHDOG:
-            event_name = "WATCHDOG";
+            return "WATCHDOG";
             break;
         case QEVENT_SPICE_CONNECTED:
-            event_name = "SPICE_CONNECTED";
+            return "SPICE_CONNECTED";
             break;
         case QEVENT_SPICE_INITIALIZED:
-            event_name = "SPICE_INITIALIZED";
+            return "SPICE_INITIALIZED";
             break;
         case QEVENT_SPICE_DISCONNECTED:
-            event_name = "SPICE_DISCONNECTED";
+            return "SPICE_DISCONNECTED";
             break;
         case QEVENT_BLOCK_JOB_COMPLETED:
-            event_name = "BLOCK_JOB_COMPLETED";
+            return "BLOCK_JOB_COMPLETED";
             break;
         case QEVENT_BLOCK_JOB_CANCELLED:
-            event_name = "BLOCK_JOB_CANCELLED";
+            return "BLOCK_JOB_CANCELLED";
             break;
         case QEVENT_DEVICE_TRAY_MOVED:
-             event_name = "DEVICE_TRAY_MOVED";
+             return "DEVICE_TRAY_MOVED";
             break;
         case QEVENT_SUSPEND:
-            event_name = "SUSPEND";
+            return "SUSPEND";
             break;
         case QEVENT_WAKEUP:
-            event_name = "WAKEUP";
+            return "WAKEUP";
             break;
         case QEVENT_BALLOON_CHANGE:
-            event_name = "BALLOON_CHANGE";
+            return "BALLOON_CHANGE";
             break;
         default:
-            abort();
+            return NULL;
             break;
     }
+}
+
+/**
+ * monitor_protocol_event(): Generate a Monitor event
+ *
+ * Event-specific data can be emitted through the (optional) 'data' parameter.
+ */
+void monitor_protocol_event(MonitorEvent event, QObject *data)
+{
+    QDict *qmp;
+    const char *event_name;
+    Monitor *mon;
+
+    assert(event < QEVENT_MAX);
+
+    event_name = monitor_protocol_event_name(event);
+    if (event_name == NULL) {
+        abort();
+    }
 
     qmp = qdict_new();
     timestamp_put(qmp);
@@ -741,6 +750,24 @@ CommandInfoList *qmp_query_commands(Error **errp)
     return cmd_list;
 }
 
+EventInfoList *qmp_query_events(Error **errp)
+{
+    EventInfoList *info, *ev_list = NULL;
+    MonitorEvent e;
+
+    for (e = 0 ; e < QEVENT_MAX ; e++) {
+        const char *evname = monitor_protocol_event_name(e);
+        info = g_malloc0(sizeof(*info));
+        info->value = g_malloc0(sizeof(*info->value));
+        info->value->name = g_strdup(evname);
+
+        info->next = ev_list;
+        ev_list = info;
+    }
+
+    return ev_list;
+}
+
 /* set the current CPU defined by the user */
 int monitor_set_cpu(int cpu_index)
 {
diff --git a/qapi-schema.json b/qapi-schema.json
index 2ca7195..94aa0ae 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -228,6 +228,28 @@
 { 'command': 'query-commands', 'returns': ['CommandInfo'] }
 
 ##
+# @EventInfo:
+#
+# Information about a QMP event
+#
+# @name: The event name
+#
+# Since: 1.2.0
+##
+{ 'type': 'EventInfo', 'data': {'name': 'str'} }
+
+##
+# @query-events:
+#
+# Return a list of supported QMP events by this server
+#
+# Returns: A list of @EventInfo for all supported events
+#
+# Since: 1.2.0
+##
+{ 'command': 'query-events', 'returns': ['EventInfo'] }
+
+##
 # @MigrationStats
 #
 # Detailed migration status.
diff --git a/qmp-commands.hx b/qmp-commands.hx
index db980fa..604ebfa 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1179,6 +1179,43 @@ EQMP
     },
 
 SQMP
+query-events
+--------------
+
+List QMP available events.
+
+Each event is represented by a json-object, the returned value is a json-array
+of all events.
+
+Each json-object contain:
+
+- "name": event's name (json-string)
+
+Example:
+
+-> { "execute": "query-events" }
+<- {
+      "return":[
+         {
+            "name":"SHUTDOWN"
+         },
+         {
+            "name":"RESET"
+         }
+      ]
+   }
+
+Note: This example has been shortened as the real response is too long.
+
+EQMP
+
+    {
+        .name       = "query-events",
+        .args_type  = "",
+        .mhandler.cmd_new = qmp_marshal_input_query_events,
+    },
+
+SQMP
 query-chardev
 -------------
 
-- 
1.7.7.6

Re: [Qemu-devel] [PATCH] Add 'query-events' command to QMP to query async events

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1662 bytes --]

On 05/16/2012 06:55 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange@redhat.com>
> 
> Sometimes it is neccessary for an application to determine
> whether a particular QMP event is available, so they can
> decide whether to use compatibility code instead. This
> introduces a new 'query-events' command to QMP todo just

s/todo/to do/

> that
> 
>  { "execute": "query-events" }
>  {"return": [{"name": "WAKEUP"},
>              {"name": "SUSPEND"},
>              {"name": "DEVICE_TRAY_MOVED"},
>              {"name": "BLOCK_JOB_CANCELLED"},
>              {"name": "BLOCK_JOB_COMPLETED"},
>              ...snip...
>              {"name": "SHUTDOWN"}]}
> 
> * monitor.c: Split out MonitorEvent -> string conversion
>   into monitor_protocol_event_name() API. Add impl of
>   qmp_query_events monitor command handler
> * qapi-schema.json, qmp-commands.hx: Define contract of
>   query-events command

Definitely useful for libvirt.


> +static const char *monitor_protocol_event_name(MonitorEvent event)
> +{
>      switch (event) {
>          case QEVENT_SHUTDOWN:
> -            event_name = "SHUTDOWN";
> +            return "SHUTDOWN";
>              break;

These 'break' statements are now unreachable; does this matter in qemu
coding style?


> +query-events
> +--------------
> +
> +List QMP available events.
> +
> +Each event is represented by a json-object, the returned value is a json-array
> +of all events.
> +
> +Each json-object contain:

s/contain/contains/

-- 
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: 620 bytes --]

Re: [Qemu-devel] [PATCH] Add 'query-events' command to QMP to query async events

[Anthony Liguori]

On 05/16/2012 11:04 AM, Eric Blake wrote:
> On 05/16/2012 06:55 AM, Daniel P. Berrange wrote:
>> From: "Daniel P. Berrange"<berrange@redhat.com>
>>
>> Sometimes it is neccessary for an application to determine
>> whether a particular QMP event is available, so they can
>> decide whether to use compatibility code instead. This
>> introduces a new 'query-events' command to QMP todo just
>
> s/todo/to do/
>
>> that
>>
>>   { "execute": "query-events" }
>>   {"return": [{"name": "WAKEUP"},
>>               {"name": "SUSPEND"},
>>               {"name": "DEVICE_TRAY_MOVED"},
>>               {"name": "BLOCK_JOB_CANCELLED"},
>>               {"name": "BLOCK_JOB_COMPLETED"},
>>               ...snip...
>>               {"name": "SHUTDOWN"}]}
>>
>> * monitor.c: Split out MonitorEvent ->  string conversion
>>    into monitor_protocol_event_name() API. Add impl of
>>    qmp_query_events monitor command handler
>> * qapi-schema.json, qmp-commands.hx: Define contract of
>>    query-events command
>
> Definitely useful for libvirt.
>
>
>> +static const char *monitor_protocol_event_name(MonitorEvent event)
>> +{
>>       switch (event) {
>>           case QEVENT_SHUTDOWN:
>> -            event_name = "SHUTDOWN";
>> +            return "SHUTDOWN";
>>               break;
>
> These 'break' statements are now unreachable; does this matter in qemu
> coding style?

Seems like we should just take the opportunity to kill off the function entirely 
and replace it with a table:

static const char *monitor_event_names[] = {
  [QEVENT_SHUTDOWN] = "SHUTDOWN",
  ...
};

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH] Add 'query-events' command to QMP to query async events

[Daniel P. Berrange]

On Wed, May 16, 2012 at 11:18:22AM -0500, Anthony Liguori wrote:
> On 05/16/2012 11:04 AM, Eric Blake wrote:
> >On 05/16/2012 06:55 AM, Daniel P. Berrange wrote:
> >>From: "Daniel P. Berrange"<berrange@redhat.com>
> >>
> >>Sometimes it is neccessary for an application to determine
> >>whether a particular QMP event is available, so they can
> >>decide whether to use compatibility code instead. This
> >>introduces a new 'query-events' command to QMP todo just
> >
> >s/todo/to do/
> >
> >>that
> >>
> >>  { "execute": "query-events" }
> >>  {"return": [{"name": "WAKEUP"},
> >>              {"name": "SUSPEND"},
> >>              {"name": "DEVICE_TRAY_MOVED"},
> >>              {"name": "BLOCK_JOB_CANCELLED"},
> >>              {"name": "BLOCK_JOB_COMPLETED"},
> >>              ...snip...
> >>              {"name": "SHUTDOWN"}]}
> >>
> >>* monitor.c: Split out MonitorEvent ->  string conversion
> >>   into monitor_protocol_event_name() API. Add impl of
> >>   qmp_query_events monitor command handler
> >>* qapi-schema.json, qmp-commands.hx: Define contract of
> >>   query-events command
> >
> >Definitely useful for libvirt.
> >
> >
> >>+static const char *monitor_protocol_event_name(MonitorEvent event)
> >>+{
> >>      switch (event) {
> >>          case QEVENT_SHUTDOWN:
> >>-            event_name = "SHUTDOWN";
> >>+            return "SHUTDOWN";
> >>              break;
> >
> >These 'break' statements are now unreachable; does this matter in qemu
> >coding style?
> 
> Seems like we should just take the opportunity to kill off the
> function entirely and replace it with a table:
> 
> static const char *monitor_event_names[] = {
>  [QEVENT_SHUTDOWN] = "SHUTDOWN",
>  ...
> };

That sounds good to me, I'll update the patch accordingly.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH] Add 'query-events' command to QMP to query async events

[Luiz Capitulino]

On Wed, 16 May 2012 11:18:22 -0500
Anthony Liguori <anthony@codemonkey.ws> wrote:

> On 05/16/2012 11:04 AM, Eric Blake wrote:
> > On 05/16/2012 06:55 AM, Daniel P. Berrange wrote:
> >> From: "Daniel P. Berrange"<berrange@redhat.com>
> >>
> >> Sometimes it is neccessary for an application to determine
> >> whether a particular QMP event is available, so they can
> >> decide whether to use compatibility code instead. This
> >> introduces a new 'query-events' command to QMP todo just
> >
> > s/todo/to do/
> >
> >> that
> >>
> >>   { "execute": "query-events" }
> >>   {"return": [{"name": "WAKEUP"},
> >>               {"name": "SUSPEND"},
> >>               {"name": "DEVICE_TRAY_MOVED"},
> >>               {"name": "BLOCK_JOB_CANCELLED"},
> >>               {"name": "BLOCK_JOB_COMPLETED"},
> >>               ...snip...
> >>               {"name": "SHUTDOWN"}]}
> >>
> >> * monitor.c: Split out MonitorEvent ->  string conversion
> >>    into monitor_protocol_event_name() API. Add impl of
> >>    qmp_query_events monitor command handler
> >> * qapi-schema.json, qmp-commands.hx: Define contract of
> >>    query-events command
> >
> > Definitely useful for libvirt.
> >
> >
> >> +static const char *monitor_protocol_event_name(MonitorEvent event)
> >> +{
> >>       switch (event) {
> >>           case QEVENT_SHUTDOWN:
> >> -            event_name = "SHUTDOWN";
> >> +            return "SHUTDOWN";
> >>               break;
> >
> > These 'break' statements are now unreachable; does this matter in qemu
> > coding style?
> 
> Seems like we should just take the opportunity to kill off the function entirely 
> and replace it with a table:
> 
> static const char *monitor_event_names[] = {
>   [QEVENT_SHUTDOWN] = "SHUTDOWN",
>   ...
> };

Yes, that's really better.

Btw, the ideal way of having this would be to add the event type to the
qapi and then add schema introspection.

As I'm not sure I can guarantee that for 1.2, I'm ok with this command.

Anthony, I'm assuming you're ok too.