[Qemu-devel] [PATCH 0/9][PULL]: QMP/Monitor queue

[Qemu-devel] [PATCH 0/9][PULL]: QMP/Monitor queue

[Luiz Capitulino]

 Hi Anthony,

 The following QMP/Monitor patches have been sent to the list and look good
to me. I have also tested most of them.

 The changes (since a303f9e37b87ced34e966dc2c0b7f86bc5e74035) are available in
the following repository:

    git://repo.or.cz/qemu/qmp-unstable.git for-anthony

Jan Kiszka (3):
      monitor: Cleanup ID assignment for compat switch
      monitor: Reorder intialization to drop initial mux focus
      chardev: Document mux option

Luiz Capitulino (5):
      QError: New QERR_QMP_BAD_INPUT_OBJECT_MEMBER
      QMP: Use QERR_QMP_BAD_INPUT_OBJECT_MEMBER
      QError: Improve QERR_QMP_BAD_INPUT_OBJECT desc
      QMP: Check "arguments" member's type
      Monitor: Return before exiting with 'quit'

Paolo Bonzini (1):
      stash away SCM_RIGHTS fd until a getfd command arrives

 monitor.c       |   16 +++++-----------
 qemu-char.c     |    9 +++------
 qemu-options.hx |   35 ++++++++++++++++++++---------------
 qerror.c        |    6 +++++-
 qerror.h        |    3 +++
 sysemu.h        |    2 ++
 vl.c            |   33 +++++++++++++++++++++++++--------
 7 files changed, 63 insertions(+), 41 deletions(-)

[Qemu-devel] [PATCH 1/9] QError: New QERR_QMP_BAD_INPUT_OBJECT_MEMBER

[Luiz Capitulino]

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 qerror.c |    4 ++++
 qerror.h |    3 +++
 2 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/qerror.c b/qerror.c
index 8d885cd..b6aaec7 100644
--- a/qerror.c
+++ b/qerror.c
@@ -173,6 +173,10 @@ static const QErrorStringTable qerror_table[] = {
         .desc      = "Bad QMP input object",
     },
     {
+        .error_fmt = QERR_QMP_BAD_INPUT_OBJECT_MEMBER,
+        .desc      = "QMP input object member '%(member)' expects '%(expected)'",
+    },
+    {
         .error_fmt = QERR_SET_PASSWD_FAILED,
         .desc      = "Could not set password",
     },
diff --git a/qerror.h b/qerror.h
index bae08c0..c98c61a 100644
--- a/qerror.h
+++ b/qerror.h
@@ -145,6 +145,9 @@ QError *qobject_to_qerror(const QObject *obj);
 #define QERR_QMP_BAD_INPUT_OBJECT \
     "{ 'class': 'QMPBadInputObject', 'data': { 'expected': %s } }"
 
+#define QERR_QMP_BAD_INPUT_OBJECT_MEMBER \
+    "{ 'class': 'QMPBadInputObjectMember', 'data': { 'member': %s, 'expected': %s } }"
+
 #define QERR_SET_PASSWD_FAILED \
     "{ 'class': 'SetPasswdFailed', 'data': {} }"
 
-- 
1.7.1.rc1.12.ga6018

[Qemu-devel] [PATCH 2/9] QMP: Use QERR_QMP_BAD_INPUT_OBJECT_MEMBER

[Luiz Capitulino]

The QERR_QMP_BAD_INPUT_OBJECT error is going to be used only
for two problems: the input is not an object or the "execute"
key is missing.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 monitor.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/monitor.c b/monitor.c
index c25d551..0611b29 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4404,7 +4404,7 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
         qerror_report(QERR_QMP_BAD_INPUT_OBJECT, "execute");
         goto err_input;
     } else if (qobject_type(obj) != QTYPE_QSTRING) {
-        qerror_report(QERR_QMP_BAD_INPUT_OBJECT, "string");
+        qerror_report(QERR_QMP_BAD_INPUT_OBJECT_MEMBER, "execute", "string");
         goto err_input;
     }
 
-- 
1.7.1.rc1.12.ga6018

[Qemu-devel] [PATCH 3/9] QError: Improve QERR_QMP_BAD_INPUT_OBJECT desc

[Luiz Capitulino]

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 qerror.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/qerror.c b/qerror.c
index b6aaec7..034c7de 100644
--- a/qerror.c
+++ b/qerror.c
@@ -170,7 +170,7 @@ static const QErrorStringTable qerror_table[] = {
     },
     {
         .error_fmt = QERR_QMP_BAD_INPUT_OBJECT,
-        .desc      = "Bad QMP input object",
+        .desc      = "Expected '%(expected)' in QMP input",
     },
     {
         .error_fmt = QERR_QMP_BAD_INPUT_OBJECT_MEMBER,
-- 
1.7.1.rc1.12.ga6018

[Qemu-devel] [PATCH 4/9] QMP: Check "arguments" member's type

[Luiz Capitulino]

Otherwise the following input crashes QEMU:

{ "execute": "migrate", "arguments": "tcp:0:4446" }

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 monitor.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/monitor.c b/monitor.c
index 0611b29..ef84298 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4437,6 +4437,9 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
     obj = qdict_get(input, "arguments");
     if (!obj) {
         args = qdict_new();
+    } else if (qobject_type(obj) != QTYPE_QDICT) {
+        qerror_report(QERR_QMP_BAD_INPUT_OBJECT_MEMBER, "arguments", "object");
+        goto err_input;
     } else {
         args = qobject_to_qdict(obj);
         QINCREF(args);
-- 
1.7.1.rc1.12.ga6018

[Qemu-devel] [PATCH 5/9] Monitor: Return before exiting with 'quit'

[Luiz Capitulino]

The 'quit' Monitor command (implemented by do_quit()) calls
exit() directly, this is problematic under QMP because QEMU
exits before having a chance to send the ok response.

Clients don't know if QEMU exited because of a problem or
because the 'quit' command has been executed.

This commit fixes that by moving the exit() call to the main
loop, so that do_quit() requests the system to quit, instead
of calling exit() directly.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 monitor.c |    2 +-
 sysemu.h  |    2 ++
 vl.c      |   18 ++++++++++++++++++
 3 files changed, 21 insertions(+), 1 deletions(-)

diff --git a/monitor.c b/monitor.c
index ef84298..611cbe9 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1017,7 +1017,7 @@ static void do_info_cpu_stats(Monitor *mon)
  */
 static int do_quit(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
-    exit(0);
+    qemu_system_exit_request();
     return 0;
 }
 
diff --git a/sysemu.h b/sysemu.h
index d0effa0..fa921df 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -45,9 +45,11 @@ void cpu_disable_ticks(void);
 void qemu_system_reset_request(void);
 void qemu_system_shutdown_request(void);
 void qemu_system_powerdown_request(void);
+void qemu_system_exit_request(void);
 int qemu_shutdown_requested(void);
 int qemu_reset_requested(void);
 int qemu_powerdown_requested(void);
+int qemu_exit_requested(void);
 extern qemu_irq qemu_system_powerdown;
 void qemu_system_reset(void);
 
diff --git a/vl.c b/vl.c
index a5a0f41..9ef6f2c 100644
--- a/vl.c
+++ b/vl.c
@@ -1697,6 +1697,7 @@ static int shutdown_requested;
 static int powerdown_requested;
 int debug_requested;
 int vmstop_requested;
+static int exit_requested;
 
 int qemu_shutdown_requested(void)
 {
@@ -1719,6 +1720,12 @@ int qemu_powerdown_requested(void)
     return r;
 }
 
+int qemu_exit_requested(void)
+{
+    /* just return it, we'll exit() anyway */
+    return exit_requested;
+}
+
 static int qemu_debug_requested(void)
 {
     int r = debug_requested;
@@ -1789,6 +1796,12 @@ void qemu_system_powerdown_request(void)
     qemu_notify_event();
 }
 
+void qemu_system_exit_request(void)
+{
+    exit_requested = 1;
+    qemu_notify_event();
+}
+
 #ifdef _WIN32
 static void host_main_loop_wait(int *timeout)
 {
@@ -1925,6 +1938,8 @@ static int vm_can_run(void)
         return 0;
     if (debug_requested)
         return 0;
+    if (exit_requested)
+        return 0;
     return 1;
 }
 
@@ -1977,6 +1992,9 @@ static void main_loop(void)
         if ((r = qemu_vmstop_requested())) {
             vm_stop(r);
         }
+        if (qemu_exit_requested()) {
+            exit(0);
+        }
     }
     pause_all_vcpus();
 }
-- 
1.7.1.rc1.12.ga6018

[Qemu-devel] [PATCH 6/9] monitor: Cleanup ID assignment for compat switch

[Luiz Capitulino]

From: Jan Kiszka <jan.kiszka@siemens.com>

Canonicalize the ID assignment when creating monitor devices via the
legacy switch and use less easily colliding names.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 vl.c |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/vl.c b/vl.c
index 9ef6f2c..20d24be 100644
--- a/vl.c
+++ b/vl.c
@@ -2348,11 +2348,9 @@ static void monitor_parse(const char *optarg, const char *mode)
     if (strstart(optarg, "chardev:", &p)) {
         snprintf(label, sizeof(label), "%s", p);
     } else {
-        if (monitor_device_index) {
-            snprintf(label, sizeof(label), "monitor%d",
-                     monitor_device_index);
-        } else {
-            snprintf(label, sizeof(label), "monitor");
+        snprintf(label, sizeof(label), "compat_monitor%d",
+                 monitor_device_index);
+        if (monitor_device_index == 0) {
             def = 1;
         }
         opts = qemu_chr_parse_compat(label, optarg);
-- 
1.7.1.rc1.12.ga6018

[Qemu-devel] [PATCH 7/9] monitor: Reorder intialization to drop initial mux focus

[Luiz Capitulino]

From: Jan Kiszka <jan.kiszka@siemens.com>

So far a multiplexed monitor started disabled. Restore this property for
the new way of configuring by moving the monitor initialization before
all devices (the last one to attach to a char-mux will gain the focus).

Once we have a real use case for that, we may also consider assigning
the initial focus explicitly.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 vl.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/vl.c b/vl.c
index 20d24be..a485c58 100644
--- a/vl.c
+++ b/vl.c
@@ -3618,6 +3618,10 @@ int main(int argc, char **argv, char **envp)
         }
     }
 
+    if (qemu_opts_foreach(&qemu_mon_opts, mon_init_func, NULL, 1) != 0) {
+        exit(1);
+    }
+
     if (foreach_device_config(DEV_SERIAL, serial_parse) < 0)
         exit(1);
     if (foreach_device_config(DEV_PARALLEL, parallel_parse) < 0)
@@ -3730,9 +3734,6 @@ int main(int argc, char **argv, char **envp)
 
     text_consoles_set_display(ds);
 
-    if (qemu_opts_foreach(&qemu_mon_opts, mon_init_func, NULL, 1) != 0)
-        exit(1);
-
     if (gdbstub_dev && gdbserver_start(gdbstub_dev) < 0) {
         fprintf(stderr, "qemu: could not open gdbserver on device '%s'\n",
                 gdbstub_dev);
-- 
1.7.1.rc1.12.ga6018

[Qemu-devel] [PATCH 8/9] chardev: Document mux option

[Luiz Capitulino]

From: Jan Kiszka <jan.kiszka@siemens.com>

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 qemu-options.hx |   35 ++++++++++++++++++++---------------
 1 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index f4b3bfe..83b54c3 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1175,32 +1175,33 @@ DEFHEADING()
 DEFHEADING(Character device options:)
 
 DEF("chardev", HAS_ARG, QEMU_OPTION_chardev,
-    "-chardev null,id=id\n"
+    "-chardev null,id=id[,mux=on|off]\n"
     "-chardev socket,id=id[,host=host],port=host[,to=to][,ipv4][,ipv6][,nodelay]\n"
-    "         [,server][,nowait][,telnet] (tcp)\n"
-    "-chardev socket,id=id,path=path[,server][,nowait][,telnet] (unix)\n"
+    "         [,server][,nowait][,telnet][,mux=on|off] (tcp)\n"
+    "-chardev socket,id=id,path=path[,server][,nowait][,telnet],[mux=on|off] (unix)\n"
     "-chardev udp,id=id[,host=host],port=port[,localaddr=localaddr]\n"
-    "         [,localport=localport][,ipv4][,ipv6]\n"
-    "-chardev msmouse,id=id\n"
+    "         [,localport=localport][,ipv4][,ipv6][,mux=on|off]\n"
+    "-chardev msmouse,id=id[,mux=on|off]\n"
     "-chardev vc,id=id[[,width=width][,height=height]][[,cols=cols][,rows=rows]]\n"
-    "-chardev file,id=id,path=path\n"
-    "-chardev pipe,id=id,path=path\n"
+    "         [,mux=on|off]\n"
+    "-chardev file,id=id,path=path[,mux=on|off]\n"
+    "-chardev pipe,id=id,path=path[,mux=on|off]\n"
 #ifdef _WIN32
-    "-chardev console,id=id\n"
-    "-chardev serial,id=id,path=path\n"
+    "-chardev console,id=id[,mux=on|off]\n"
+    "-chardev serial,id=id,path=path[,mux=on|off]\n"
 #else
-    "-chardev pty,id=id\n"
-    "-chardev stdio,id=id\n"
+    "-chardev pty,id=id[,mux=on|off]\n"
+    "-chardev stdio,id=id[,mux=on|off]\n"
 #endif
 #ifdef CONFIG_BRLAPI
-    "-chardev braille,id=id\n"
+    "-chardev braille,id=id[,mux=on|off]\n"
 #endif
 #if defined(__linux__) || defined(__sun__) || defined(__FreeBSD__) \
         || defined(__NetBSD__) || defined(__OpenBSD__) || defined(__DragonFly__)
-    "-chardev tty,id=id,path=path\n"
+    "-chardev tty,id=id,path=path[,mux=on|off]\n"
 #endif
 #if defined(__linux__) || defined(__FreeBSD__) || defined(__DragonFly__)
-    "-chardev parport,id=id,path=path\n"
+    "-chardev parport,id=id,path=path[,mux=on|off]\n"
 #endif
     , QEMU_ARCH_ALL
 )
@@ -1210,7 +1211,7 @@ STEXI
 The general form of a character device option is:
 @table @option
 
-@item -chardev @var{backend} ,id=@var{id} [,@var{options}]
+@item -chardev @var{backend} ,id=@var{id} [,mux=on|off] [,@var{options}]
 @findex -chardev
 Backend is one of:
 @option{null},
@@ -1232,6 +1233,10 @@ The specific backend will determine the applicable options.
 All devices must have an id, which can be any string up to 127 characters long.
 It is used to uniquely identify this device in other command line directives.
 
+A character device may be used in multiplexing mode by multiple front-ends.
+The key sequence of @key{Control-a} and @key{c} will rotate the input focus
+between attached front-ends. Specify @option{mux=on} to enable this mode.
+
 Options to each backend are described below.
 
 @item -chardev null ,id=@var{id}
-- 
1.7.1.rc1.12.ga6018

[Qemu-devel] [PATCH 9/9] stash away SCM_RIGHTS fd until a getfd command arrives

[Luiz Capitulino]

From: Paolo Bonzini <pbonzini@redhat.com>

If there is already a fd in s->msgfd before recvmsg it is
closed by parts that this patch does not touch.  So, only
one descriptor can be "leaked" by attaching it to a command
other than getfd.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 monitor.c   |    9 ---------
 qemu-char.c |    9 +++------
 2 files changed, 3 insertions(+), 15 deletions(-)

diff --git a/monitor.c b/monitor.c
index 611cbe9..814b7dc 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2414,15 +2414,6 @@ static int do_getfd(Monitor *mon, const QDict *qdict, QObject **ret_data)
         return -1;
     }
 
-    fd = dup(fd);
-    if (fd == -1) {
-        if (errno == EMFILE)
-            qerror_report(QERR_TOO_MANY_FILES);
-        else
-            qerror_report(QERR_UNDEFINED_ERROR);
-        return -1;
-    }
-
     QLIST_FOREACH(monfd, &mon->fds, next) {
         if (strcmp(monfd->name, fdname) != 0) {
             continue;
diff --git a/qemu-char.c b/qemu-char.c
index 05df971..ac65a1c 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2000,8 +2000,9 @@ static void tcp_chr_process_IAC_bytes(CharDriverState *chr,
 static int tcp_get_msgfd(CharDriverState *chr)
 {
     TCPCharDriver *s = chr->opaque;
-
-    return s->msgfd;
+    int fd = s->msgfd;
+    s->msgfd = -1;
+    return fd;
 }
 
 #ifndef _WIN32
@@ -2089,10 +2090,6 @@ static void tcp_chr_read(void *opaque)
             tcp_chr_process_IAC_bytes(chr, s, buf, &size);
         if (size > 0)
             qemu_chr_read(chr, buf, size);
-        if (s->msgfd != -1) {
-            close(s->msgfd);
-            s->msgfd = -1;
-        }
     }
 }
 
-- 
1.7.1.rc1.12.ga6018

Re: [Qemu-devel] [PATCH 5/9] Monitor: Return before exiting with 'quit'

[Anthony Liguori]

On 04/26/2010 10:47 AM, Luiz Capitulino wrote:
> The 'quit' Monitor command (implemented by do_quit()) calls
> exit() directly, this is problematic under QMP because QEMU
> exits before having a chance to send the ok response.
>
> Clients don't know if QEMU exited because of a problem or
> because the 'quit' command has been executed.
>
> This commit fixes that by moving the exit() call to the main
> loop, so that do_quit() requests the system to quit, instead
> of calling exit() directly.
>    

Does this also have the effect of printing out a (qemu) prompt after 
quit before an EOF appears on that socket?

Regards,

Anthony Liguori

> Signed-off-by: Luiz Capitulino<lcapitulino@redhat.com>
> ---
>   monitor.c |    2 +-
>   sysemu.h  |    2 ++
>   vl.c      |   18 ++++++++++++++++++
>   3 files changed, 21 insertions(+), 1 deletions(-)
>
> diff --git a/monitor.c b/monitor.c
> index ef84298..611cbe9 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -1017,7 +1017,7 @@ static void do_info_cpu_stats(Monitor *mon)
>    */
>   static int do_quit(Monitor *mon, const QDict *qdict, QObject **ret_data)
>   {
> -    exit(0);
> +    qemu_system_exit_request();
>       return 0;
>   }
>
> diff --git a/sysemu.h b/sysemu.h
> index d0effa0..fa921df 100644
> --- a/sysemu.h
> +++ b/sysemu.h
> @@ -45,9 +45,11 @@ void cpu_disable_ticks(void);
>   void qemu_system_reset_request(void);
>   void qemu_system_shutdown_request(void);
>   void qemu_system_powerdown_request(void);
> +void qemu_system_exit_request(void);
>   int qemu_shutdown_requested(void);
>   int qemu_reset_requested(void);
>   int qemu_powerdown_requested(void);
> +int qemu_exit_requested(void);
>   extern qemu_irq qemu_system_powerdown;
>   void qemu_system_reset(void);
>
> diff --git a/vl.c b/vl.c
> index a5a0f41..9ef6f2c 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1697,6 +1697,7 @@ static int shutdown_requested;
>   static int powerdown_requested;
>   int debug_requested;
>   int vmstop_requested;
> +static int exit_requested;
>
>   int qemu_shutdown_requested(void)
>   {
> @@ -1719,6 +1720,12 @@ int qemu_powerdown_requested(void)
>       return r;
>   }
>
> +int qemu_exit_requested(void)
> +{
> +    /* just return it, we'll exit() anyway */
> +    return exit_requested;
> +}
> +
>   static int qemu_debug_requested(void)
>   {
>       int r = debug_requested;
> @@ -1789,6 +1796,12 @@ void qemu_system_powerdown_request(void)
>       qemu_notify_event();
>   }
>
> +void qemu_system_exit_request(void)
> +{
> +    exit_requested = 1;
> +    qemu_notify_event();
> +}
> +
>   #ifdef _WIN32
>   static void host_main_loop_wait(int *timeout)
>   {
> @@ -1925,6 +1938,8 @@ static int vm_can_run(void)
>           return 0;
>       if (debug_requested)
>           return 0;
> +    if (exit_requested)
> +        return 0;
>       return 1;
>   }
>
> @@ -1977,6 +1992,9 @@ static void main_loop(void)
>           if ((r = qemu_vmstop_requested())) {
>               vm_stop(r);
>           }
> +        if (qemu_exit_requested()) {
> +            exit(0);
> +        }
>       }
>       pause_all_vcpus();
>   }
>    

Re: [Qemu-devel] [PATCH 5/9] Monitor: Return before exiting with 'quit'

[Luiz Capitulino]

On Mon, 26 Apr 2010 12:49:40 -0500
Anthony Liguori <anthony@codemonkey.ws> wrote:

> On 04/26/2010 10:47 AM, Luiz Capitulino wrote:
> > The 'quit' Monitor command (implemented by do_quit()) calls
> > exit() directly, this is problematic under QMP because QEMU
> > exits before having a chance to send the ok response.
> >
> > Clients don't know if QEMU exited because of a problem or
> > because the 'quit' command has been executed.
> >
> > This commit fixes that by moving the exit() call to the main
> > loop, so that do_quit() requests the system to quit, instead
> > of calling exit() directly.
> >    
> 
> Does this also have the effect of printing out a (qemu) prompt after 
> quit before an EOF appears on that socket?

 Ah, right..

 So, the easiest way to fix this is:

if (user monitor) {
   exit(0);
} else {
   go through main;
}

 And, wrt to the pull, assuming you like the other patches, what's the best for you?

 Should I just drop this patch and ask you to pull again or can I do the fix,
rebase, send it in this thread, and ping you?

Re: [Qemu-devel] [PATCH 5/9] Monitor: Return before exiting with 'quit'

[Anthony Liguori]

On 04/26/2010 01:22 PM, Luiz Capitulino wrote:
> On Mon, 26 Apr 2010 12:49:40 -0500
> Anthony Liguori<anthony@codemonkey.ws>  wrote:
>
>    
>> On 04/26/2010 10:47 AM, Luiz Capitulino wrote:
>>      
>>> The 'quit' Monitor command (implemented by do_quit()) calls
>>> exit() directly, this is problematic under QMP because QEMU
>>> exits before having a chance to send the ok response.
>>>
>>> Clients don't know if QEMU exited because of a problem or
>>> because the 'quit' command has been executed.
>>>
>>> This commit fixes that by moving the exit() call to the main
>>> loop, so that do_quit() requests the system to quit, instead
>>> of calling exit() directly.
>>>
>>>        
>> Does this also have the effect of printing out a (qemu) prompt after
>> quit before an EOF appears on that socket?
>>      
>   Ah, right..
>    

It's not necessarily a bad thing if it does.  I just wanted to raise 
that because it's possible that someone depends on the behavior.

I'm not sure it matters to me if we change this behavior though.

>   So, the easiest way to fix this is:
>
> if (user monitor) {
>     exit(0);
> } else {
>     go through main;
> }
>
>   And, wrt to the pull, assuming you like the other patches, what's the best for you?
>    

I'm happy to pull it, just wanted to see fi this issue was considered 
before I did.

Regards,

Anthony Liguori

>   Should I just drop this patch and ask you to pull again or can I do the fix,
> rebase, send it in this thread, and ping you?
>    

Re: [Qemu-devel] [PATCH 5/9] Monitor: Return before exiting with 'quit'

[Luiz Capitulino]

On Mon, 26 Apr 2010 13:25:38 -0500
Anthony Liguori <anthony@codemonkey.ws> wrote:

> On 04/26/2010 01:22 PM, Luiz Capitulino wrote:
> > On Mon, 26 Apr 2010 12:49:40 -0500
> > Anthony Liguori<anthony@codemonkey.ws>  wrote:
> >
> >    
> >> On 04/26/2010 10:47 AM, Luiz Capitulino wrote:
> >>      
> >>> The 'quit' Monitor command (implemented by do_quit()) calls
> >>> exit() directly, this is problematic under QMP because QEMU
> >>> exits before having a chance to send the ok response.
> >>>
> >>> Clients don't know if QEMU exited because of a problem or
> >>> because the 'quit' command has been executed.
> >>>
> >>> This commit fixes that by moving the exit() call to the main
> >>> loop, so that do_quit() requests the system to quit, instead
> >>> of calling exit() directly.
> >>>
> >>>        
> >> Does this also have the effect of printing out a (qemu) prompt after
> >> quit before an EOF appears on that socket?
> >>      
> >   Ah, right..
> >    
> 
> It's not necessarily a bad thing if it does.  I just wanted to raise 
> that because it's possible that someone depends on the behavior.

 Yes, and it's also a bit ugly if you do '-monitor stdio':

~/stuff/virt/ ./qemu-qmp -monitor stdio
QEMU 0.12.50 monitor - type 'help' for more information
(qemu) quit
(qemu) ~/stuff/virt/

 So, I'd like to fix it.

> I'm not sure it matters to me if we change this behavior though.

 It's very easy to fix, the following patch does it.

 I have (tested and) rebased the for-anthony branch with it.

From 14ecef47d1989f9b646fc0fe7a4bf42c091d1432 Mon Sep 17 00:00:00 2001
From: Luiz Capitulino <lcapitulino@redhat.com>
Date: Tue, 6 Apr 2010 18:55:54 -0300
Subject: [PATCH 5/9] Monitor: Return before exiting with 'quit'

The 'quit' Monitor command (implemented by do_quit()) calls
exit() directly, this is problematic under QMP because QEMU
exits before having a chance to send the ok response.

Clients don't know if QEMU exited because of a problem or
because the 'quit' command has been executed.

This commit fixes that by moving the exit() call to the main
loop, so that do_quit() requests the system to quit, instead
of calling exit() directly.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 monitor.c |    7 ++++++-
 sysemu.h  |    2 ++
 vl.c      |   18 ++++++++++++++++++
 3 files changed, 26 insertions(+), 1 deletions(-)

diff --git a/monitor.c b/monitor.c
index ef84298..5b0258f 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1017,7 +1017,12 @@ static void do_info_cpu_stats(Monitor *mon)
  */
 static int do_quit(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
-    exit(0);
+    if (monitor_ctrl_mode(mon)) {
+        qemu_system_exit_request();
+    } else {
+        exit(0);
+    }
+
     return 0;
 }
 
diff --git a/sysemu.h b/sysemu.h
index d0effa0..fa921df 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -45,9 +45,11 @@ void cpu_disable_ticks(void);
 void qemu_system_reset_request(void);
 void qemu_system_shutdown_request(void);
 void qemu_system_powerdown_request(void);
+void qemu_system_exit_request(void);
 int qemu_shutdown_requested(void);
 int qemu_reset_requested(void);
 int qemu_powerdown_requested(void);
+int qemu_exit_requested(void);
 extern qemu_irq qemu_system_powerdown;
 void qemu_system_reset(void);
 
diff --git a/vl.c b/vl.c
index a5a0f41..9ef6f2c 100644
--- a/vl.c
+++ b/vl.c
@@ -1697,6 +1697,7 @@ static int shutdown_requested;
 static int powerdown_requested;
 int debug_requested;
 int vmstop_requested;
+static int exit_requested;
 
 int qemu_shutdown_requested(void)
 {
@@ -1719,6 +1720,12 @@ int qemu_powerdown_requested(void)
     return r;
 }
 
+int qemu_exit_requested(void)
+{
+    /* just return it, we'll exit() anyway */
+    return exit_requested;
+}
+
 static int qemu_debug_requested(void)
 {
     int r = debug_requested;
@@ -1789,6 +1796,12 @@ void qemu_system_powerdown_request(void)
     qemu_notify_event();
 }
 
+void qemu_system_exit_request(void)
+{
+    exit_requested = 1;
+    qemu_notify_event();
+}
+
 #ifdef _WIN32
 static void host_main_loop_wait(int *timeout)
 {
@@ -1925,6 +1938,8 @@ static int vm_can_run(void)
         return 0;
     if (debug_requested)
         return 0;
+    if (exit_requested)
+        return 0;
     return 1;
 }
 
@@ -1977,6 +1992,9 @@ static void main_loop(void)
         if ((r = qemu_vmstop_requested())) {
             vm_stop(r);
         }
+        if (qemu_exit_requested()) {
+            exit(0);
+        }
     }
     pause_all_vcpus();
 }
-- 
1.7.1.rc1.12.ga6018

Re: [Qemu-devel] [PATCH 5/9] Monitor: Return before exiting with 'quit'

[Anthony Liguori]

On 04/26/2010 01:53 PM, Luiz Capitulino wrote:
> On Mon, 26 Apr 2010 13:25:38 -0500
> Anthony Liguori<anthony@codemonkey.ws>  wrote:
>
>    
>> On 04/26/2010 01:22 PM, Luiz Capitulino wrote:
>>      
>>> On Mon, 26 Apr 2010 12:49:40 -0500
>>> Anthony Liguori<anthony@codemonkey.ws>   wrote:
>>>
>>>
>>>        
>>>> On 04/26/2010 10:47 AM, Luiz Capitulino wrote:
>>>>
>>>>          
>>>>> The 'quit' Monitor command (implemented by do_quit()) calls
>>>>> exit() directly, this is problematic under QMP because QEMU
>>>>> exits before having a chance to send the ok response.
>>>>>
>>>>> Clients don't know if QEMU exited because of a problem or
>>>>> because the 'quit' command has been executed.
>>>>>
>>>>> This commit fixes that by moving the exit() call to the main
>>>>> loop, so that do_quit() requests the system to quit, instead
>>>>> of calling exit() directly.
>>>>>
>>>>>
>>>>>            
>>>> Does this also have the effect of printing out a (qemu) prompt after
>>>> quit before an EOF appears on that socket?
>>>>
>>>>          
>>>    Ah, right..
>>>
>>>        
>> It's not necessarily a bad thing if it does.  I just wanted to raise
>> that because it's possible that someone depends on the behavior.
>>      
>   Yes, and it's also a bit ugly if you do '-monitor stdio':
>
> ~/stuff/virt/ ./qemu-qmp -monitor stdio
> QEMU 0.12.50 monitor - type 'help' for more information
> (qemu) quit
> (qemu) ~/stuff/virt/
>
>   So, I'd like to fix it.
>
>    
>> I'm not sure it matters to me if we change this behavior though.
>>      
>   It's very easy to fix, the following patch does it.
>
>   I have (tested and) rebased the for-anthony branch with it.
>
>  From 14ecef47d1989f9b646fc0fe7a4bf42c091d1432 Mon Sep 17 00:00:00 2001
> From: Luiz Capitulino<lcapitulino@redhat.com>
> Date: Tue, 6 Apr 2010 18:55:54 -0300
> Subject: [PATCH 5/9] Monitor: Return before exiting with 'quit'
>
> The 'quit' Monitor command (implemented by do_quit()) calls
> exit() directly, this is problematic under QMP because QEMU
> exits before having a chance to send the ok response.
>
> Clients don't know if QEMU exited because of a problem or
> because the 'quit' command has been executed.
>
> This commit fixes that by moving the exit() call to the main
> loop, so that do_quit() requests the system to quit, instead
> of calling exit() directly.
>
> Signed-off-by: Luiz Capitulino<lcapitulino@redhat.com>
> ---
>   monitor.c |    7 ++++++-
>   sysemu.h  |    2 ++
>   vl.c      |   18 ++++++++++++++++++
>   3 files changed, 26 insertions(+), 1 deletions(-)
>
> diff --git a/monitor.c b/monitor.c
> index ef84298..5b0258f 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -1017,7 +1017,12 @@ static void do_info_cpu_stats(Monitor *mon)
>    */
>   static int do_quit(Monitor *mon, const QDict *qdict, QObject **ret_data)
>   {
> -    exit(0);
> +    if (monitor_ctrl_mode(mon)) {
> +        qemu_system_exit_request();
> +    } else {
> +        exit(0);
> +    }
> +
>    

Now they have different behaviors though because 
qemu_system_exit_request() actually exits gracefully.

For instance, now in QMP mode, an ifdown script will be executed when 
the 'quit' command is invoked whereas it won't be executed when done 
through the human monitor.

I honestly don't mind the weird print outs with -monitor stdio.  I think 
exiting gracefully in both cases is better.

Regards,

Anthony Liguori

>       return 0;
>   }
>
> diff --git a/sysemu.h b/sysemu.h
> index d0effa0..fa921df 100644
> --- a/sysemu.h
> +++ b/sysemu.h
> @@ -45,9 +45,11 @@ void cpu_disable_ticks(void);
>   void qemu_system_reset_request(void);
>   void qemu_system_shutdown_request(void);
>   void qemu_system_powerdown_request(void);
> +void qemu_system_exit_request(void);
>   int qemu_shutdown_requested(void);
>   int qemu_reset_requested(void);
>   int qemu_powerdown_requested(void);
> +int qemu_exit_requested(void);
>   extern qemu_irq qemu_system_powerdown;
>   void qemu_system_reset(void);
>
> diff --git a/vl.c b/vl.c
> index a5a0f41..9ef6f2c 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1697,6 +1697,7 @@ static int shutdown_requested;
>   static int powerdown_requested;
>   int debug_requested;
>   int vmstop_requested;
> +static int exit_requested;
>
>   int qemu_shutdown_requested(void)
>   {
> @@ -1719,6 +1720,12 @@ int qemu_powerdown_requested(void)
>       return r;
>   }
>
> +int qemu_exit_requested(void)
> +{
> +    /* just return it, we'll exit() anyway */
> +    return exit_requested;
> +}
> +
>   static int qemu_debug_requested(void)
>   {
>       int r = debug_requested;
> @@ -1789,6 +1796,12 @@ void qemu_system_powerdown_request(void)
>       qemu_notify_event();
>   }
>
> +void qemu_system_exit_request(void)
> +{
> +    exit_requested = 1;
> +    qemu_notify_event();
> +}
> +
>   #ifdef _WIN32
>   static void host_main_loop_wait(int *timeout)
>   {
> @@ -1925,6 +1938,8 @@ static int vm_can_run(void)
>           return 0;
>       if (debug_requested)
>           return 0;
> +    if (exit_requested)
> +        return 0;
>       return 1;
>   }
>
> @@ -1977,6 +1992,9 @@ static void main_loop(void)
>           if ((r = qemu_vmstop_requested())) {
>               vm_stop(r);
>           }
> +        if (qemu_exit_requested()) {
> +            exit(0);
> +        }
>       }
>       pause_all_vcpus();
>   }
>    

[Qemu-devel] Re: [PATCH 5/9] Monitor: Return before exiting with 'quit'

[Jan Kiszka]

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

Anthony Liguori wrote:
> On 04/26/2010 01:53 PM, Luiz Capitulino wrote:
>> On Mon, 26 Apr 2010 13:25:38 -0500
>> Anthony Liguori<anthony@codemonkey.ws>  wrote:
>>
>>   
>>> On 04/26/2010 01:22 PM, Luiz Capitulino wrote:
>>>     
>>>> On Mon, 26 Apr 2010 12:49:40 -0500
>>>> Anthony Liguori<anthony@codemonkey.ws>   wrote:
>>>>
>>>>
>>>>       
>>>>> On 04/26/2010 10:47 AM, Luiz Capitulino wrote:
>>>>>
>>>>>         
>>>>>> The 'quit' Monitor command (implemented by do_quit()) calls
>>>>>> exit() directly, this is problematic under QMP because QEMU
>>>>>> exits before having a chance to send the ok response.
>>>>>>
>>>>>> Clients don't know if QEMU exited because of a problem or
>>>>>> because the 'quit' command has been executed.
>>>>>>
>>>>>> This commit fixes that by moving the exit() call to the main
>>>>>> loop, so that do_quit() requests the system to quit, instead
>>>>>> of calling exit() directly.
>>>>>>
>>>>>>
>>>>>>            
>>>>> Does this also have the effect of printing out a (qemu) prompt after
>>>>> quit before an EOF appears on that socket?
>>>>>
>>>>>          
>>>>    Ah, right..
>>>>
>>>>        
>>> It's not necessarily a bad thing if it does.  I just wanted to raise
>>> that because it's possible that someone depends on the behavior.
>>>      
>>   Yes, and it's also a bit ugly if you do '-monitor stdio':
>>
>> ~/stuff/virt/ ./qemu-qmp -monitor stdio
>> QEMU 0.12.50 monitor - type 'help' for more information
>> (qemu) quit
>> (qemu) ~/stuff/virt/
>>
>>   So, I'd like to fix it.
>>
>>   
>>> I'm not sure it matters to me if we change this behavior though.
>>>      
>>   It's very easy to fix, the following patch does it.
>>
>>   I have (tested and) rebased the for-anthony branch with it.
>>
>>  From 14ecef47d1989f9b646fc0fe7a4bf42c091d1432 Mon Sep 17 00:00:00 2001
>> From: Luiz Capitulino<lcapitulino@redhat.com>
>> Date: Tue, 6 Apr 2010 18:55:54 -0300
>> Subject: [PATCH 5/9] Monitor: Return before exiting with 'quit'
>>
>> The 'quit' Monitor command (implemented by do_quit()) calls
>> exit() directly, this is problematic under QMP because QEMU
>> exits before having a chance to send the ok response.
>>
>> Clients don't know if QEMU exited because of a problem or
>> because the 'quit' command has been executed.
>>
>> This commit fixes that by moving the exit() call to the main
>> loop, so that do_quit() requests the system to quit, instead
>> of calling exit() directly.
>>
>> Signed-off-by: Luiz Capitulino<lcapitulino@redhat.com>
>> ---
>>   monitor.c |    7 ++++++-
>>   sysemu.h  |    2 ++
>>   vl.c      |   18 ++++++++++++++++++
>>   3 files changed, 26 insertions(+), 1 deletions(-)
>>
>> diff --git a/monitor.c b/monitor.c
>> index ef84298..5b0258f 100644
>> --- a/monitor.c
>> +++ b/monitor.c
>> @@ -1017,7 +1017,12 @@ static void do_info_cpu_stats(Monitor *mon)
>>    */
>>   static int do_quit(Monitor *mon, const QDict *qdict, QObject
>> **ret_data)
>>   {
>> -    exit(0);
>> +    if (monitor_ctrl_mode(mon)) {
>> +        qemu_system_exit_request();
>> +    } else {
>> +        exit(0);
>> +    }
>> +
>>    
> 
> Now they have different behaviors though because
> qemu_system_exit_request() actually exits gracefully.
> 
> For instance, now in QMP mode, an ifdown script will be executed when
> the 'quit' command is invoked whereas it won't be executed when done
> through the human monitor.
> 
> I honestly don't mind the weird print outs with -monitor stdio.  I think
> exiting gracefully in both cases is better.

Just suspend the monitor before leaving for termination. That should
have both the proper visual and functional effect.

Jan



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

[Qemu-devel] Re: [PATCH 5/9] Monitor: Return before exiting with 'quit'

[Anthony Liguori]

On 04/26/2010 02:10 PM, Jan Kiszka wrote:
>
> Just suspend the monitor before leaving for termination. That should
> have both the proper visual and functional effect.
>    

Very good idea.

Regards,

Anthony Liguori

> Jan
>
>
>    

[Qemu-devel] Re: [PATCH 5/9] Monitor: Return before exiting with 'quit'

[Luiz Capitulino]

On Mon, 26 Apr 2010 14:13:40 -0500
Anthony Liguori <anthony@codemonkey.ws> wrote:

> On 04/26/2010 02:10 PM, Jan Kiszka wrote:
> >
> > Just suspend the monitor before leaving for termination. That should
> > have both the proper visual and functional effect.
> >    
> 
> Very good idea.

 Yeah, works great.

 Final version of the patch below, for-anthony branch updated.

From 0e8d2b5575938b8876a3c4bb66ee13c5d306fb6d Mon Sep 17 00:00:00 2001
From: Luiz Capitulino <lcapitulino@redhat.com>
Date: Tue, 6 Apr 2010 18:55:54 -0300
Subject: [PATCH 5/9] Monitor: Return before exiting with 'quit'

The 'quit' Monitor command (implemented by do_quit()) calls
exit() directly, this is problematic under QMP because QEMU
exits before having a chance to send the ok response.

Clients don't know if QEMU exited because of a problem or
because the 'quit' command has been executed.

This commit fixes that by moving the exit() call to the main
loop, so that do_quit() requests the system to quit, instead
of calling exit() directly.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 monitor.c |    3 ++-
 sysemu.h  |    2 ++
 vl.c      |   18 ++++++++++++++++++
 3 files changed, 22 insertions(+), 1 deletions(-)

diff --git a/monitor.c b/monitor.c
index ef84298..0dc24a2 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1017,7 +1017,8 @@ static void do_info_cpu_stats(Monitor *mon)
  */
 static int do_quit(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
-    exit(0);
+    monitor_suspend(mon);
+    qemu_system_exit_request();
     return 0;
 }
 
diff --git a/sysemu.h b/sysemu.h
index d0effa0..fa921df 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -45,9 +45,11 @@ void cpu_disable_ticks(void);
 void qemu_system_reset_request(void);
 void qemu_system_shutdown_request(void);
 void qemu_system_powerdown_request(void);
+void qemu_system_exit_request(void);
 int qemu_shutdown_requested(void);
 int qemu_reset_requested(void);
 int qemu_powerdown_requested(void);
+int qemu_exit_requested(void);
 extern qemu_irq qemu_system_powerdown;
 void qemu_system_reset(void);
 
diff --git a/vl.c b/vl.c
index a5a0f41..9ef6f2c 100644
--- a/vl.c
+++ b/vl.c
@@ -1697,6 +1697,7 @@ static int shutdown_requested;
 static int powerdown_requested;
 int debug_requested;
 int vmstop_requested;
+static int exit_requested;
 
 int qemu_shutdown_requested(void)
 {
@@ -1719,6 +1720,12 @@ int qemu_powerdown_requested(void)
     return r;
 }
 
+int qemu_exit_requested(void)
+{
+    /* just return it, we'll exit() anyway */
+    return exit_requested;
+}
+
 static int qemu_debug_requested(void)
 {
     int r = debug_requested;
@@ -1789,6 +1796,12 @@ void qemu_system_powerdown_request(void)
     qemu_notify_event();
 }
 
+void qemu_system_exit_request(void)
+{
+    exit_requested = 1;
+    qemu_notify_event();
+}
+
 #ifdef _WIN32
 static void host_main_loop_wait(int *timeout)
 {
@@ -1925,6 +1938,8 @@ static int vm_can_run(void)
         return 0;
     if (debug_requested)
         return 0;
+    if (exit_requested)
+        return 0;
     return 1;
 }
 
@@ -1977,6 +1992,9 @@ static void main_loop(void)
         if ((r = qemu_vmstop_requested())) {
             vm_stop(r);
         }
+        if (qemu_exit_requested()) {
+            exit(0);
+        }
     }
     pause_all_vcpus();
 }
-- 
1.7.1.rc1.12.ga6018

Re: [Qemu-devel] [PATCH 0/9][PULL]: QMP/Monitor queue

[Anthony Liguori]

On 04/26/2010 10:47 AM, Luiz Capitulino wrote:
>   Hi Anthony,
>
>   The following QMP/Monitor patches have been sent to the list and look good
> to me. I have also tested most of them.
>    

Applied.  Thanks.

Regards,

Anthony Liguori

>   The changes (since a303f9e37b87ced34e966dc2c0b7f86bc5e74035) are available in
> the following repository:
>
>      git://repo.or.cz/qemu/qmp-unstable.git for-anthony
>
> Jan Kiszka (3):
>        monitor: Cleanup ID assignment for compat switch
>        monitor: Reorder intialization to drop initial mux focus
>        chardev: Document mux option
>
> Luiz Capitulino (5):
>        QError: New QERR_QMP_BAD_INPUT_OBJECT_MEMBER
>        QMP: Use QERR_QMP_BAD_INPUT_OBJECT_MEMBER
>        QError: Improve QERR_QMP_BAD_INPUT_OBJECT desc
>        QMP: Check "arguments" member's type
>        Monitor: Return before exiting with 'quit'
>
> Paolo Bonzini (1):
>        stash away SCM_RIGHTS fd until a getfd command arrives
>
>   monitor.c       |   16 +++++-----------
>   qemu-char.c     |    9 +++------
>   qemu-options.hx |   35 ++++++++++++++++++++---------------
>   qerror.c        |    6 +++++-
>   qerror.h        |    3 +++
>   sysemu.h        |    2 ++
>   vl.c            |   33 +++++++++++++++++++++++++--------
>   7 files changed, 63 insertions(+), 41 deletions(-)
>
>
>
>    

[Qemu-devel] Re: [PATCH 5/9] Monitor: Return before exiting with 'quit'

[Paolo Bonzini]

On 04/26/2010 09:44 PM, Luiz Capitulino wrote:
> +    qemu_system_exit_request();

Untested suggestion: why add qemu_system_exit_request, exit_requested, 
and a hook in the main loop?  You can do instead

    no_shutdown = 0;
    qemu_system_shutdown_request();

which will actually call quit_timers() and net_cleanup() properly unlike 
a blind exit(0).

Alternatively, just give an error when "quit"-ting from QMP and keep the 
current behavior for non-QMP.  This way you do not provide two ways to 
do the same thing.  People will have to avoid -no-shutdown (I don't see 
how it is useful from QMP) and they will be able to use the "shutdown" 
monitor command.

Paolo

[Qemu-devel] Re: [PATCH 5/9] Monitor: Return before exiting with 'quit'

[Luiz Capitulino]

On Tue, 27 Apr 2010 13:52:29 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 04/26/2010 09:44 PM, Luiz Capitulino wrote:
> > +    qemu_system_exit_request();
> 
> Untested suggestion: why add qemu_system_exit_request, exit_requested, 
> and a hook in the main loop?  You can do instead
> 
>     no_shutdown = 0;
>     qemu_system_shutdown_request();
> 
> which will actually call quit_timers() and net_cleanup() properly unlike 
> a blind exit(0).

 Hm, this looks good. It has the side effect of emitting the SHUTDOWN
event, but maybe this is even desirable.

 I will send a patch if there are no objections.

> Alternatively, just give an error when "quit"-ting from QMP and keep the 
> current behavior for non-QMP.  This way you do not provide two ways to 
> do the same thing.  People will have to avoid -no-shutdown (I don't see 
> how it is useful from QMP) and they will be able to use the "shutdown" 
> monitor command.

 Not sure if I got you here, why should we return an error?

[Qemu-devel] Re: [PATCH 5/9] Monitor: Return before exiting with 'quit'

[Paolo Bonzini]

On 04/27/2010 03:20 PM, Luiz Capitulino wrote:
> On Tue, 27 Apr 2010 13:52:29 +0200 Paolo Bonzini  wrote:
>> On 04/26/2010 09:44 PM, Luiz Capitulino wrote:
>>> +    qemu_system_exit_request();
>>
>> Untested suggestion: why add qemu_system_exit_request, exit_requested,
>> and a hook in the main loop?  You can do instead
>>
>>      no_shutdown = 0;
>>      qemu_system_shutdown_request();
>>
>> which will actually call quit_timers() and net_cleanup() properly unlike
>> a blind exit(0).
>
>   Hm, this looks good. It has the side effect of emitting the SHUTDOWN
> event, but maybe this is even desirable.

Exactly.

>> Alternatively, just give an error when "quit"-ting from QMP and keep the
>> current behavior for non-QMP.  This way you do not provide two ways to
>> do the same thing.  People will have to avoid -no-shutdown (I don't see
>> how it is useful from QMP) and they will be able to use the "shutdown"
>> monitor command.
>
>   Not sure if I got you here, why should we return an error?

Because quit looks like a useless duplicate of shutdown in QMP 
scenarios.  (As long as you do not pass -no-shutdown; but I don't see 
why a management app should).

Paolo