[Qemu-devel] [PATCH 1/2] Print asynchronous notifications on request

[Qemu-devel] [PATCH 1/2] Print asynchronous notifications on request

[Amit Shah]

This patch adds the ability to selectively enable asynchronous notifications
from individual qemu subsystems by a new 'notify' monitor command.

This is helpful for programs currently parsing monitor output.

A sample invocation will look like this:

(qemu)
    <vnc connection closed>
(qemu) notify vnc on
    <vnc connection closed>
(qemu) # VNC: Closing down connection 127.0.0.1:1

Notice that the output is prefixed by '#'. Also, it will appear on the
line that has '(qemu) ' already output on it.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 qemu/console.h |    4 ++++
 qemu/monitor.c |   34 ++++++++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 0 deletions(-)

diff --git a/qemu/console.h b/qemu/console.h
index 383ea1a..0cf575c 100644
--- a/qemu/console.h
+++ b/qemu/console.h
@@ -292,6 +292,8 @@ void curses_display_init(DisplayState *ds, int full_screen);
 /* x_keymap.c */
 extern uint8_t _translate_keycode(const int key);
 
+#define MAX_ASYNC_EVENTS  0
+
 /* FIXME: term_printf et al should probably go elsewhere so everything
    does not need to include console.h  */
 /* monitor.c */
@@ -299,6 +301,8 @@ void monitor_init(CharDriverState *hd, int show_banner);
 void term_puts(const char *str);
 void term_vprintf(const char *fmt, va_list ap);
 void term_printf(const char *fmt, ...) __attribute__ ((__format__ (__printf__, 1, 2)));
+void term_printf_async(const int event, const char *fmt, ...)
+  __attribute__ ((__format__ (__printf__, 2, 3)));
 void term_print_filename(const char *filename);
 void term_flush(void);
 void term_print_help(void);
diff --git a/qemu/monitor.c b/qemu/monitor.c
index 7ff9890..ef871d0 100644
--- a/qemu/monitor.c
+++ b/qemu/monitor.c
@@ -65,6 +65,8 @@ typedef struct term_cmd_t {
     const char *help;
 } term_cmd_t;
 
+int async_printable_events[MAX_ASYNC_EVENTS];
+
 #define MAX_MON 4
 static CharDriverState *monitor_hd[MAX_MON];
 static int hide_banner;
@@ -122,6 +124,24 @@ void term_printf(const char *fmt, ...)
     va_end(ap);
 }
 
+void term_printf_async(const int event, const char *fmt, ...)
+{
+    va_list ap;
+    va_start(ap, fmt);
+
+    if (event > MAX_ASYNC_EVENTS)
+        goto cleanup;
+    if (!async_printable_events[event])
+        goto cleanup;
+
+    term_printf("# ");
+    term_vprintf(fmt, ap);
+
+cleanup:
+    va_end(ap);
+    return;
+}
+
 void term_print_filename(const char *filename)
 {
     int i;
@@ -210,6 +230,18 @@ static void do_help(const char *name)
     help_cmd(name);
 }
 
+static void do_notify_async_events(char *event_str, char *enable)
+{
+    int event;
+
+    return;
+
+    if (!strcmp(enable, "on"))
+        async_printable_events[event] = 1;
+    else
+        async_printable_events[event] = 0;
+}
+
 static void do_commit(const char *device)
 {
     int i, all_devices;
@@ -1517,6 +1549,8 @@ static const term_cmd_t term_cmds[] = {
     { "balloon", "i", do_balloon,
       "target", "request VM to change it's memory allocation (in MB)" },
     { "set_link", "ss", do_set_link, "name [up|down]" },
+    { "notify", "ss", do_notify_async_events,
+      "NULL on|off", "enable / disable printing of notifications for the specified event" },
     { NULL, NULL, },
 };
 
-- 
1.6.0.6

[Qemu-devel] [PATCH 2/2] vnc: Use async notifications for closing down messages

[Amit Shah]

If "notify vnc on" is issued on the monitor, VNC close events will be
shown on the monitor, prefixed with the '#' character.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 qemu/console.h |    3 ++-
 qemu/monitor.c |    7 +++++--
 qemu/vnc.c     |    4 ++++
 3 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/qemu/console.h b/qemu/console.h
index 0cf575c..9b04b51 100644
--- a/qemu/console.h
+++ b/qemu/console.h
@@ -292,7 +292,8 @@ void curses_display_init(DisplayState *ds, int full_screen);
 /* x_keymap.c */
 extern uint8_t _translate_keycode(const int key);
 
-#define MAX_ASYNC_EVENTS  0
+#define VNC_ASYNC_EVENT   1
+#define MAX_ASYNC_EVENTS  VNC_ASYNC_EVENT
 
 /* FIXME: term_printf et al should probably go elsewhere so everything
    does not need to include console.h  */
diff --git a/qemu/monitor.c b/qemu/monitor.c
index ef871d0..a581e52 100644
--- a/qemu/monitor.c
+++ b/qemu/monitor.c
@@ -234,7 +234,10 @@ static void do_notify_async_events(char *event_str, char *enable)
 {
     int event;
 
-    return;
+    if (!strcmp(event_str, "vnc"))
+        event = VNC_ASYNC_EVENT;
+    else
+        return;
 
     if (!strcmp(enable, "on"))
         async_printable_events[event] = 1;
@@ -1550,7 +1553,7 @@ static const term_cmd_t term_cmds[] = {
       "target", "request VM to change it's memory allocation (in MB)" },
     { "set_link", "ss", do_set_link, "name [up|down]" },
     { "notify", "ss", do_notify_async_events,
-      "NULL on|off", "enable / disable printing of notifications for the specified event" },
+      "vnc on|off", "enable / disable printing of notifications for the specified event" },
     { NULL, NULL, },
 };
 
diff --git a/qemu/vnc.c b/qemu/vnc.c
index 17ea9a2..ad52d74 100644
--- a/qemu/vnc.c
+++ b/qemu/vnc.c
@@ -746,6 +746,8 @@ static int vnc_client_io_error(VncState *vs, int ret, int last_errno)
 	vs->wiremode = VNC_WIREMODE_CLEAR;
 #endif /* CONFIG_VNC_TLS */
         audio_del(vs);
+        if (vs->display)
+            term_printf_async(VNC_ASYNC_EVENT, "VNC: Closing down connection %s\n", vs->display);
 	return 0;
     }
     return ret;
@@ -2261,6 +2263,8 @@ void vnc_display_close(DisplayState *ds)
     vs->x509verify = 0;
 #endif
     audio_del(vs);
+    if (vs->display)
+        term_printf_async(VNC_ASYNC_EVENT, "VNC: Closing down connection %s\n", vs->display);
 }
 
 int vnc_display_password(DisplayState *ds, const char *password)
-- 
1.6.0.6

[Qemu-devel] Re: [PATCH 1/2] Print asynchronous notifications on request

[Jan Kiszka]

Amit Shah wrote:
> This patch adds the ability to selectively enable asynchronous notifications
> from individual qemu subsystems by a new 'notify' monitor command.
> 
> This is helpful for programs currently parsing monitor output.
> 
> A sample invocation will look like this:
> 
> (qemu)
>     <vnc connection closed>
> (qemu) notify vnc on
>     <vnc connection closed>
> (qemu) # VNC: Closing down connection 127.0.0.1:1
> 
> Notice that the output is prefixed by '#'. Also, it will appear on the
> line that has '(qemu) ' already output on it.

I understand the need, but the result looks a bit ugly, at least to
humans forced to parse it. If you get this while in the middle of type a
command... Moreover, is it impossible that such an async notification is
issued while some other subsystem is already dumping a multi-line
message to the monitor (using multiple prints)? That would be really
problematic.

But if this approach is merged, then I would really say we need
separately configurable monitor terminals, and notify should then only
affect the issuing one. However, my feeling is that a real
machine-dedicated and easily processable channel would be better suited
for this use case.

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] Re: [PATCH 1/2] Print asynchronous notifications on request

[Avi Kivity]

Jan Kiszka wrote:
> I understand the need, but the result looks a bit ugly, at least to
> humans forced to parse it. If you get this while in the middle of type a
> command... Moreover, is it impossible that such an async notification is
> issued while some other subsystem is already dumping a multi-line
> message to the monitor (using multiple prints)? That would be really
> problematic.
>   

No, unless that subsystem schedules between prints.

> But if this approach is merged, then I would really say we need
> separately configurable monitor terminals, 

No no please no

> and notify should then only
> affect the issuing one. However, my feeling is that a real
> machine-dedicated and easily processable channel would be better suited
> for this use case.
>   

Definitely.  Have a machine protocol with a strong emphasis on 
compatibility and parseability, and a human protocol with emphasis on 
friendliness (prompts, completions, fix typos).

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] Re: [PATCH 1/2] Print asynchronous notifications on request

[Amit Shah]

On (Tue) Jan 20 2009 [12:35:23], Jan Kiszka wrote:
> Amit Shah wrote:
> > This patch adds the ability to selectively enable asynchronous notifications
> > from individual qemu subsystems by a new 'notify' monitor command.
> > 
> > This is helpful for programs currently parsing monitor output.
> > 
> > A sample invocation will look like this:
> > 
> > (qemu)
> >     <vnc connection closed>
> > (qemu) notify vnc on
> >     <vnc connection closed>
> > (qemu) # VNC: Closing down connection 127.0.0.1:1
> > 
> > Notice that the output is prefixed by '#'. Also, it will appear on the
> > line that has '(qemu) ' already output on it.
> 
> I understand the need, but the result looks a bit ugly, at least to
> humans forced to parse it. If you get this while in the middle of type a
> command... Moreover, is it impossible that such an async notification is
> issued while some other subsystem is already dumping a multi-line
> message to the monitor (using multiple prints)? That would be really
> problematic.

Agreed. However, note that the messages sent via term_printf_async()
will only be emitted if 'notify <foo> on' is issued. Humans wouldn't
normally do that.

[Qemu-devel] Re: [PATCH 1/2] Print asynchronous notifications on request

[Jan Kiszka]

Amit Shah wrote:
> On (Tue) Jan 20 2009 [12:35:23], Jan Kiszka wrote:
>> Amit Shah wrote:
>>> This patch adds the ability to selectively enable asynchronous notifications
>>> from individual qemu subsystems by a new 'notify' monitor command.
>>>
>>> This is helpful for programs currently parsing monitor output.
>>>
>>> A sample invocation will look like this:
>>>
>>> (qemu)
>>>     <vnc connection closed>
>>> (qemu) notify vnc on
>>>     <vnc connection closed>
>>> (qemu) # VNC: Closing down connection 127.0.0.1:1
>>>
>>> Notice that the output is prefixed by '#'. Also, it will appear on the
>>> line that has '(qemu) ' already output on it.
>> I understand the need, but the result looks a bit ugly, at least to
>> humans forced to parse it. If you get this while in the middle of type a
>> command... Moreover, is it impossible that such an async notification is
>> issued while some other subsystem is already dumping a multi-line
>> message to the monitor (using multiple prints)? That would be really
>> problematic.
> 
> Agreed. However, note that the messages sent via term_printf_async()
> will only be emitted if 'notify <foo> on' is issued. Humans wouldn't
> normally do that.
> 

The scenario I have in mind is a qemu instance managed by libvirt etc. +
a monitor console or some forwarded monitor accedd to a gdb frontend
used to analyse/debug a guest. On the latter interface, the user may not
want to get disturbed by notifications the management tools likes to
receive.

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux

[Qemu-devel] Re: [PATCH 1/2] Print asynchronous notifications on request

[Anthony Liguori]

Amit Shah wrote:
> This patch adds the ability to selectively enable asynchronous notifications
> from individual qemu subsystems by a new 'notify' monitor command.
>
> This is helpful for programs currently parsing monitor output.
>   

I'd rather see two monitors be supported and the second monitor issue an 
explicit "listen" command to wait for async notifications

This mechanism scares me a fair bit since you're printing over the user 
prompt.

> A sample invocation will look like this:
>
> (qemu)
>     <vnc connection closed>
> (qemu) notify vnc on
>     <vnc connection closed>
> (qemu) # VNC: Closing down connection 127.0.0.1:1
>
> Notice that the output is prefixed by '#'. Also, it will appear on the
> line that has '(qemu) ' already output on it.
>
> Signed-off-by: Amit Shah <amit.shah@redhat.com>
> ---
>  qemu/console.h |    4 ++++
>  qemu/monitor.c |   34 ++++++++++++++++++++++++++++++++++
>  2 files changed, 38 insertions(+), 0 deletions(-)
>
> diff --git a/qemu/console.h b/qemu/console.h
> index 383ea1a..0cf575c 100644
> --- a/qemu/console.h
> +++ b/qemu/console.h
> @@ -292,6 +292,8 @@ void curses_display_init(DisplayState *ds, int full_screen);
>  /* x_keymap.c */
>  extern uint8_t _translate_keycode(const int key);
>
> +#define MAX_ASYNC_EVENTS  0
> +
>  /* FIXME: term_printf et al should probably go elsewhere so everything
>     does not need to include console.h  */
>  /* monitor.c */
> @@ -299,6 +301,8 @@ void monitor_init(CharDriverState *hd, int show_banner);
>  void term_puts(const char *str);
>  void term_vprintf(const char *fmt, va_list ap);
>  void term_printf(const char *fmt, ...) __attribute__ ((__format__ (__printf__, 1, 2)));
> +void term_printf_async(const int event, const char *fmt, ...)
> +  __attribute__ ((__format__ (__printf__, 2, 3)));
>  void term_print_filename(const char *filename);
>  void term_flush(void);
>  void term_print_help(void);
> diff --git a/qemu/monitor.c b/qemu/monitor.c
> index 7ff9890..ef871d0 100644
> --- a/qemu/monitor.c
> +++ b/qemu/monitor.c
> @@ -65,6 +65,8 @@ typedef struct term_cmd_t {
>      const char *help;
>  } term_cmd_t;
>
> +int async_printable_events[MAX_ASYNC_EVENTS];
> +
>  #define MAX_MON 4
>  static CharDriverState *monitor_hd[MAX_MON];
>  static int hide_banner;
> @@ -122,6 +124,24 @@ void term_printf(const char *fmt, ...)
>      va_end(ap);
>  }
>
> +void term_printf_async(const int event, const char *fmt, ...)
> +{
> +    va_list ap;
> +    va_start(ap, fmt);
> +
> +    if (event > MAX_ASYNC_EVENTS)
> +        goto cleanup;
> +    if (!async_printable_events[event])
> +        goto cleanup;
> +
> +    term_printf("# ");
> +    term_vprintf(fmt, ap);
> +
> +cleanup:
> +    va_end(ap);
> +    return;
> +}
> +
>  void term_print_filename(const char *filename)
>  {
>      int i;
> @@ -210,6 +230,18 @@ static void do_help(const char *name)
>      help_cmd(name);
>  }
>
> +static void do_notify_async_events(char *event_str, char *enable)
> +{
> +    int event;
> +
> +    return;
> +
> +    if (!strcmp(enable, "on"))
> +        async_printable_events[event] = 1;
> +    else
> +        async_printable_events[event] = 0;
> +}
> +
>  static void do_commit(const char *device)
>  {
>      int i, all_devices;
> @@ -1517,6 +1549,8 @@ static const term_cmd_t term_cmds[] = {
>      { "balloon", "i", do_balloon,
>        "target", "request VM to change it's memory allocation (in MB)" },
>      { "set_link", "ss", do_set_link, "name [up|down]" },
> +    { "notify", "ss", do_notify_async_events,
> +      "NULL on|off", "enable / disable printing of notifications for the specified event" },
>      { NULL, NULL, },
>  };
>
>