From 7dfb61238a472acf6fb48d9a444564b9b99a4b56 Mon Sep 17 00:00:00 2001 From: Luiz Capitulino Date: Wed, 7 Apr 2010 14:46:33 -0300 Subject: [PATCH 1/9] QError: New QERR_QMP_BAD_INPUT_OBJECT_MEMBER Signed-off-by: Luiz Capitulino --- qerror.c | 4 ++++ qerror.h | 3 +++ 2 files changed, 7 insertions(+) diff --git a/qerror.c b/qerror.c index 8d885cdce4..b6aaec7a86 100644 --- a/qerror.c +++ b/qerror.c @@ -172,6 +172,10 @@ static const QErrorStringTable qerror_table[] = { .error_fmt = QERR_QMP_BAD_INPUT_OBJECT, .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 bae08c0bb0..c98c61ad11 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': {} }" From 88f7db846264223f6059ec329e7b7a77026ad475 Mon Sep 17 00:00:00 2001 From: Luiz Capitulino Date: Wed, 7 Apr 2010 14:49:37 -0300 Subject: [PATCH 2/9] QMP: Use QERR_QMP_BAD_INPUT_OBJECT_MEMBER 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 --- monitor.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/monitor.c b/monitor.c index c25d551f5c..0611b29d1a 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; } From abaf2f52716625d4b1e29204cab644ed656504cf Mon Sep 17 00:00:00 2001 From: Luiz Capitulino Date: Wed, 7 Apr 2010 14:53:49 -0300 Subject: [PATCH 3/9] QError: Improve QERR_QMP_BAD_INPUT_OBJECT desc Signed-off-by: Luiz Capitulino --- qerror.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qerror.c b/qerror.c index b6aaec7a86..034c7deaad 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, From 04f8c053cca9c329eebb761f3a1ffef3d349b84c Mon Sep 17 00:00:00 2001 From: Luiz Capitulino Date: Tue, 6 Apr 2010 16:39:42 -0300 Subject: [PATCH 4/9] QMP: Check "arguments" member's type Otherwise the following input crashes QEMU: { "execute": "migrate", "arguments": "tcp:0:4446" } Signed-off-by: Luiz Capitulino --- monitor.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/monitor.c b/monitor.c index 0611b29d1a..ef8429861b 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); From 0e8d2b5575938b8876a3c4bb66ee13c5d306fb6d Mon Sep 17 00:00:00 2001 From: Luiz Capitulino 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 --- monitor.c | 3 ++- sysemu.h | 2 ++ vl.c | 18 ++++++++++++++++++ 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/monitor.c b/monitor.c index ef8429861b..0dc24a2f75 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 d0effa031c..fa921df94a 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 a5a0f41729..9ef6f2cbef 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(); } From 140e065d72a2301b0b5f769be664e10ebe223888 Mon Sep 17 00:00:00 2001 From: Jan Kiszka Date: Tue, 6 Apr 2010 16:55:52 +0200 Subject: [PATCH 6/9] monitor: Cleanup ID assignment for compat switch Canonicalize the ID assignment when creating monitor devices via the legacy switch and use less easily colliding names. Signed-off-by: Jan Kiszka Signed-off-by: Luiz Capitulino --- vl.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/vl.c b/vl.c index 9ef6f2cbef..20d24be9d0 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); From 157b9319878e67f5c28b8cf39216d42f4b162586 Mon Sep 17 00:00:00 2001 From: Jan Kiszka Date: Tue, 6 Apr 2010 16:55:53 +0200 Subject: [PATCH 7/9] monitor: Reorder intialization to drop initial mux focus 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 Signed-off-by: Luiz Capitulino --- vl.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/vl.c b/vl.c index 20d24be9d0..a485c58fdd 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); From 97331287ed2c928af6b9f31728d29ef60c3b8cd8 Mon Sep 17 00:00:00 2001 From: Jan Kiszka Date: Tue, 6 Apr 2010 16:55:54 +0200 Subject: [PATCH 8/9] chardev: Document mux option Signed-off-by: Jan Kiszka Signed-off-by: Luiz Capitulino --- qemu-options.hx | 35 ++++++++++++++++++++--------------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/qemu-options.hx b/qemu-options.hx index f4b3bfe90f..83b54c3067 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} From e53f27b9d9df73461308618151fa6e6392aebd85 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 16 Apr 2010 17:25:23 +0200 Subject: [PATCH 9/9] stash away SCM_RIGHTS fd until a getfd command arrives 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 Signed-off-by: Luiz Capitulino --- monitor.c | 9 --------- qemu-char.c | 9 +++------ 2 files changed, 3 insertions(+), 15 deletions(-) diff --git a/monitor.c b/monitor.c index 0dc24a2f75..754bcc5cc0 100644 --- a/monitor.c +++ b/monitor.c @@ -2415,15 +2415,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 05df971412..ac65a1c806 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; - } } }