All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v2 0/4] libqtest: solve QEMU process cleanup problem
@ 2014-07-25 13:16 Stefan Hajnoczi
  2014-07-25 13:16 ` [Qemu-devel] [PATCH v2 1/4] vl: add qemu_system_shutdown_force() Stefan Hajnoczi
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Stefan Hajnoczi @ 2014-07-25 13:16 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Andreas Faerber, Markus Armbruster

v2:
 * Added qemu_system_shutdown_force() API [Peter]

Test cases are supposed to clean up even if they fail.  Historically libqtest
has leaked QEMU processes and files.  This caused annoyances and buildbot
failures so it was gradually fixed.

The solution we have for terminating the QEMU process if the test case fails
was not very satisfactory.  A SIGABRT handler in the test case sends SIGTERM to
QEMU.  This only works if the test case receives SIGABRT, not other ways in
which it could die.  The approach is ugly because it installs a global signal
handler although libqtest is supposed to support multiple simultaneous
instances.

This patch series adds the new -chardev exit-on-eof option.  QEMU will
terminate if the socket/pipe/stdio receives EOF.  That happens when the test
case process terminates for any reason.  By adding this option to -chardev we
can use it with both -qtest and -qmp.

Stefan Hajnoczi (4):
  vl: add qemu_system_shutdown_force()
  libqemustub: add qemu_system_shutdown_force()
  qemu-char: add -chardev exit-on-eof option
  libqtest: use -chardev exit-on-eof to clean up QEMU

 include/sysemu/char.h   |  1 +
 include/sysemu/sysemu.h |  2 +-
 qapi-schema.json        | 23 ++++++++++++++++-------
 qemu-char.c             | 33 +++++++++++++++++++++++++++------
 qemu-options.hx         | 19 +++++++++++++------
 qmp.c                   |  3 +--
 stubs/Makefile.objs     |  1 +
 stubs/shutdown.c        |  5 +++++
 tests/libqtest.c        | 48 +++---------------------------------------------
 ui/sdl.c                |  3 +--
 ui/sdl2.c               |  6 ++----
 vl.c                    | 11 ++++++++---
 12 files changed, 79 insertions(+), 76 deletions(-)
 create mode 100644 stubs/shutdown.c

-- 
1.9.3

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Qemu-devel] [PATCH v2 1/4] vl: add qemu_system_shutdown_force()
  2014-07-25 13:16 [Qemu-devel] [PATCH v2 0/4] libqtest: solve QEMU process cleanup problem Stefan Hajnoczi
@ 2014-07-25 13:16 ` Stefan Hajnoczi
  2014-07-25 13:16 ` [Qemu-devel] [PATCH v2 2/4] libqemustub: " Stefan Hajnoczi
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Stefan Hajnoczi @ 2014-07-25 13:16 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Andreas Faerber, Markus Armbruster

The QEMU --no-shutdown option prevents guests from shutting down.  There
are several cases where shutdown should be forced, even if --no-shutdown
was given.

This patch adds an API for forcing shutdown.  This cleans up the code
and hides the extern int no_shutdown variable which is currently being
touched in various files.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/sysemu/sysemu.h |  2 +-
 qmp.c                   |  3 +--
 ui/sdl.c                |  3 +--
 ui/sdl2.c               |  6 ++----
 vl.c                    | 11 ++++++++---
 5 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index d8539fd..171d7d3 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -58,6 +58,7 @@ void qemu_system_wakeup_request(WakeupReason reason);
 void qemu_system_wakeup_enable(WakeupReason reason, bool enabled);
 void qemu_register_wakeup_notifier(Notifier *notifier);
 void qemu_system_shutdown_request(void);
+void qemu_system_shutdown_force(void);
 void qemu_system_powerdown_request(void);
 void qemu_register_powerdown_notifier(Notifier *notifier);
 void qemu_system_debug_request(void);
@@ -126,7 +127,6 @@ extern int max_cpus;
 extern int cursor_hide;
 extern int graphic_rotate;
 extern int no_quit;
-extern int no_shutdown;
 extern int semihosting_enabled;
 extern int old_param;
 extern int boot_menu;
diff --git a/qmp.c b/qmp.c
index 0d2553a..cd4d6a7 100644
--- a/qmp.c
+++ b/qmp.c
@@ -86,8 +86,7 @@ UuidInfo *qmp_query_uuid(Error **errp)
 
 void qmp_quit(Error **errp)
 {
-    no_shutdown = 0;
-    qemu_system_shutdown_request();
+    qemu_system_shutdown_force();
 }
 
 void qmp_stop(Error **errp)
diff --git a/ui/sdl.c b/ui/sdl.c
index 4e7f920..7c3d91c 100644
--- a/ui/sdl.c
+++ b/ui/sdl.c
@@ -791,8 +791,7 @@ static void sdl_refresh(DisplayChangeListener *dcl)
             break;
         case SDL_QUIT:
             if (!no_quit) {
-                no_shutdown = 0;
-                qemu_system_shutdown_request();
+                qemu_system_shutdown_force();
             }
             break;
         case SDL_MOUSEMOTION:
diff --git a/ui/sdl2.c b/ui/sdl2.c
index fcac87b..f392528 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -711,8 +711,7 @@ static void handle_windowevent(DisplayChangeListener *dcl, SDL_Event *ev)
         break;
     case SDL_WINDOWEVENT_CLOSE:
         if (!no_quit) {
-            no_shutdown = 0;
-            qemu_system_shutdown_request();
+            qemu_system_shutdown_force();
         }
         break;
     }
@@ -743,8 +742,7 @@ static void sdl_refresh(DisplayChangeListener *dcl)
             break;
         case SDL_QUIT:
             if (!no_quit) {
-                no_shutdown = 0;
-                qemu_system_shutdown_request();
+                qemu_system_shutdown_force();
             }
             break;
         case SDL_MOUSEMOTION:
diff --git a/vl.c b/vl.c
index fe451aa..c733e04 100644
--- a/vl.c
+++ b/vl.c
@@ -164,7 +164,7 @@ int acpi_enabled = 1;
 int no_hpet = 0;
 int fd_bootchk = 1;
 static int no_reboot;
-int no_shutdown = 0;
+static int no_shutdown = 0;
 int cursor_hide = 1;
 int graphic_rotate = 0;
 const char *watchdog;
@@ -1915,8 +1915,7 @@ void qemu_system_killed(int signal, pid_t pid)
 {
     shutdown_signal = signal;
     shutdown_pid = pid;
-    no_shutdown = 0;
-    qemu_system_shutdown_request();
+    qemu_system_shutdown_force();
 }
 
 void qemu_system_shutdown_request(void)
@@ -1926,6 +1925,12 @@ void qemu_system_shutdown_request(void)
     qemu_notify_event();
 }
 
+void qemu_system_shutdown_force(void)
+{
+    no_shutdown = 0;
+    qemu_system_shutdown_request();
+}
+
 static void qemu_system_powerdown(void)
 {
     qapi_event_send_powerdown(&error_abort);
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [Qemu-devel] [PATCH v2 2/4] libqemustub: add qemu_system_shutdown_force()
  2014-07-25 13:16 [Qemu-devel] [PATCH v2 0/4] libqtest: solve QEMU process cleanup problem Stefan Hajnoczi
  2014-07-25 13:16 ` [Qemu-devel] [PATCH v2 1/4] vl: add qemu_system_shutdown_force() Stefan Hajnoczi
@ 2014-07-25 13:16 ` Stefan Hajnoczi
  2014-07-25 13:16 ` [Qemu-devel] [PATCH v2 3/4] qemu-char: add -chardev exit-on-eof option Stefan Hajnoczi
  2014-07-25 13:16 ` [Qemu-devel] [PATCH v2 4/4] libqtest: use -chardev exit-on-eof to clean up QEMU Stefan Hajnoczi
  3 siblings, 0 replies; 6+ messages in thread
From: Stefan Hajnoczi @ 2014-07-25 13:16 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Andreas Faerber, Markus Armbruster

These sysemu functions will be needed by qemu-char.c, which is linked
into tests/vhost-user-test without system emulation functionality.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 stubs/Makefile.objs | 1 +
 stubs/shutdown.c    | 5 +++++
 2 files changed, 6 insertions(+)
 create mode 100644 stubs/shutdown.c

diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index 528e161..f17d517 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -39,3 +39,4 @@ stub-obj-$(CONFIG_WIN32) += fd-register.o
 stub-obj-y += cpus.o
 stub-obj-y += kvm.o
 stub-obj-y += qmp_pc_dimm_device_list.o
+stub-obj-y += shutdown.o
diff --git a/stubs/shutdown.c b/stubs/shutdown.c
new file mode 100644
index 0000000..37c96c0
--- /dev/null
+++ b/stubs/shutdown.c
@@ -0,0 +1,5 @@
+#include "sysemu/sysemu.h"
+
+void qemu_system_shutdown_force(void)
+{
+}
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [Qemu-devel] [PATCH v2 3/4] qemu-char: add -chardev exit-on-eof option
  2014-07-25 13:16 [Qemu-devel] [PATCH v2 0/4] libqtest: solve QEMU process cleanup problem Stefan Hajnoczi
  2014-07-25 13:16 ` [Qemu-devel] [PATCH v2 1/4] vl: add qemu_system_shutdown_force() Stefan Hajnoczi
  2014-07-25 13:16 ` [Qemu-devel] [PATCH v2 2/4] libqemustub: " Stefan Hajnoczi
@ 2014-07-25 13:16 ` Stefan Hajnoczi
  2014-07-28 21:21   ` Eric Blake
  2014-07-25 13:16 ` [Qemu-devel] [PATCH v2 4/4] libqtest: use -chardev exit-on-eof to clean up QEMU Stefan Hajnoczi
  3 siblings, 1 reply; 6+ messages in thread
From: Stefan Hajnoczi @ 2014-07-25 13:16 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Andreas Faerber, Markus Armbruster

When QEMU is executed as part of a test case or from a script, it is
usually desirable to exit if the parent process terminates.  This
ensures that "leaked" QEMU processes do not continue consuming resources
after their parent has died.

This patch adds the -chardev exit-on-eof option causing socket and pipe
chardevs to exit QEMU upon close.  This happens when a parent process
deliberately closes its file descriptor but also when the kernel cleans
up a crashed process.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/sysemu/char.h |  1 +
 qapi-schema.json      | 23 ++++++++++++++++-------
 qemu-char.c           | 33 +++++++++++++++++++++++++++------
 qemu-options.hx       | 19 +++++++++++++------
 4 files changed, 57 insertions(+), 19 deletions(-)

diff --git a/include/sysemu/char.h b/include/sysemu/char.h
index 0bbd631..382b320 100644
--- a/include/sysemu/char.h
+++ b/include/sysemu/char.h
@@ -86,6 +86,7 @@ struct CharDriverState {
     guint fd_in_tag;
     QemuOpts *opts;
     QTAILQ_ENTRY(CharDriverState) next;
+    bool exit_on_eof;
 };
 
 /**
diff --git a/qapi-schema.json b/qapi-schema.json
index b11aad2..9b13da1 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2630,10 +2630,13 @@
 # @device: The name of the special file for the device,
 #          i.e. /dev/ttyS0 on Unix or COM1: on Windows
 # @type: What kind of device this is.
+# @exit-on-eof: #optional terminate when other side closes the pipe
+#               (default: false, since: 2.2)
 #
 # Since: 1.4
 ##
-{ 'type': 'ChardevHostdev', 'data': { 'device' : 'str' } }
+{ 'type': 'ChardevHostdev', 'data': { 'device' : 'str',
+                                      '*exit-on-eof' : 'bool' } }
 
 ##
 # @ChardevSocket:
@@ -2648,14 +2651,17 @@
 # @nodelay: #optional set TCP_NODELAY socket option (default: false)
 # @telnet: #optional enable telnet protocol on server
 #          sockets (default: false)
+# @exit-on-eof: #optional terminate when other side closes socket
+#               (default: false, since: 2.2)
 #
 # Since: 1.4
 ##
-{ 'type': 'ChardevSocket', 'data': { 'addr'     : 'SocketAddress',
-                                     '*server'  : 'bool',
-                                     '*wait'    : 'bool',
-                                     '*nodelay' : 'bool',
-                                     '*telnet'  : 'bool' } }
+{ 'type': 'ChardevSocket', 'data': { 'addr'         : 'SocketAddress',
+                                     '*server'      : 'bool',
+                                     '*wait'        : 'bool',
+                                     '*nodelay'     : 'bool',
+                                     '*telnet'      : 'bool',
+                                     '*exit-on-eof' : 'bool' } }
 
 ##
 # @ChardevUdp:
@@ -2689,10 +2695,13 @@
 # @signal: #optional Allow signals (such as SIGINT triggered by ^C)
 #          be delivered to qemu.  Default: true in -nographic mode,
 #          false otherwise.
+# @exit-on-eof: #optional terminate when other side sends EOF
+#               (default: false, since: 2.2)
 #
 # Since: 1.5
 ##
-{ 'type': 'ChardevStdio', 'data': { '*signal' : 'bool' } }
+{ 'type': 'ChardevStdio', 'data': { '*signal' : 'bool',
+                                    '*exit-on-eof' : 'bool' } }
 
 ##
 # @ChardevSpiceChannel:
diff --git a/qemu-char.c b/qemu-char.c
index 7acc03f..1974b8f 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -110,9 +110,15 @@ void qemu_chr_be_event(CharDriverState *s, int event)
             break;
     }
 
-    if (!s->chr_event)
-        return;
-    s->chr_event(s->handler_opaque, event);
+    if (s->chr_event) {
+        s->chr_event(s->handler_opaque, event);
+    }
+
+    if (s->exit_on_eof && event == CHR_EVENT_CLOSED) {
+        fprintf(stderr, "qemu: terminating due to eof on chardev '%s'\n",
+                s->label);
+        qemu_system_shutdown_force();
+    }
 }
 
 void qemu_chr_be_generic_open(CharDriverState *s)
@@ -991,6 +997,7 @@ static CharDriverState *qemu_chr_open_pipe(ChardevHostdev *opts)
     int fd_in, fd_out;
     char filename_in[256], filename_out[256];
     const char *filename = opts->device;
+    CharDriverState *chr;
 
     if (filename == NULL) {
         fprintf(stderr, "chardev: pipe: no filename given\n");
@@ -1011,7 +1018,9 @@ static CharDriverState *qemu_chr_open_pipe(ChardevHostdev *opts)
             return NULL;
         }
     }
-    return qemu_chr_open_fd(fd_in, fd_out);
+    chr = qemu_chr_open_fd(fd_in, fd_out);
+    chr->exit_on_eof = opts->has_exit_on_eof && opts->exit_on_eof;
+    return chr;
 }
 
 /* init terminal so that we can grab keys */
@@ -2893,6 +2902,7 @@ static void tcp_chr_close(CharDriverState *chr)
 static CharDriverState *qemu_chr_open_socket_fd(int fd, bool do_nodelay,
                                                 bool is_listen, bool is_telnet,
                                                 bool is_waitconnect,
+                                                bool is_exit_on_eof,
                                                 Error **errp)
 {
     CharDriverState *chr = NULL;
@@ -2955,6 +2965,7 @@ static CharDriverState *qemu_chr_open_socket_fd(int fd, bool do_nodelay,
     chr->chr_update_read_handler = tcp_chr_update_read_handler;
     /* be isn't opened until we get a connection */
     chr->explicit_be_open = true;
+    chr->exit_on_eof = is_exit_on_eof;
 
     if (is_listen) {
         s->listen_fd = fd;
@@ -2991,6 +3002,7 @@ static CharDriverState *qemu_chr_open_socket(QemuOpts *opts)
     bool is_telnet      = qemu_opt_get_bool(opts, "telnet", false);
     bool do_nodelay     = !qemu_opt_get_bool(opts, "delay", true);
     bool is_unix        = qemu_opt_get(opts, "path") != NULL;
+    bool is_exit_on_eof = qemu_opt_get_bool(opts, "exit-on-eof", false);
 
     if (is_unix) {
         if (is_listen) {
@@ -3013,7 +3025,7 @@ static CharDriverState *qemu_chr_open_socket(QemuOpts *opts)
         qemu_set_nonblock(fd);
 
     chr = qemu_chr_open_socket_fd(fd, do_nodelay, is_listen, is_telnet,
-                                  is_waitconnect, &local_err);
+                                  is_waitconnect, is_exit_on_eof, &local_err);
     if (local_err) {
         goto fail;
     }
@@ -3376,6 +3388,8 @@ static void qemu_chr_parse_stdio(QemuOpts *opts, ChardevBackend *backend,
     backend->stdio = g_new0(ChardevStdio, 1);
     backend->stdio->has_signal = true;
     backend->stdio->signal = qemu_opt_get_bool(opts, "signal", true);
+    backend->stdio->has_exit_on_eof = true;
+    backend->stdio->exit_on_eof = qemu_opt_get_bool(opts, "exit-on-eof", false);
 }
 
 static void qemu_chr_parse_serial(QemuOpts *opts, ChardevBackend *backend,
@@ -3415,6 +3429,8 @@ static void qemu_chr_parse_pipe(QemuOpts *opts, ChardevBackend *backend,
     }
     backend->pipe = g_new0(ChardevHostdev, 1);
     backend->pipe->device = g_strdup(device);
+    backend->pipe->has_exit_on_eof = true;
+    backend->pipe->exit_on_eof = qemu_opt_get_bool(opts, "exit-on-eof", false);
 }
 
 static void qemu_chr_parse_ringbuf(QemuOpts *opts, ChardevBackend *backend,
@@ -3848,6 +3864,9 @@ QemuOptsList qemu_chardev_opts = {
         },{
             .name = "chardev",
             .type = QEMU_OPT_STRING,
+        },{
+            .name = "exit-on-eof",
+            .type = QEMU_OPT_BOOL,
         },
         { /* end of list */ }
     },
@@ -3967,6 +3986,7 @@ static CharDriverState *qmp_chardev_open_socket(ChardevSocket *sock,
     bool is_listen      = sock->has_server  ? sock->server  : true;
     bool is_telnet      = sock->has_telnet  ? sock->telnet  : false;
     bool is_waitconnect = sock->has_wait    ? sock->wait    : false;
+    bool is_exit_on_eof = sock->has_exit_on_eof ? sock->exit_on_eof : false;
     int fd;
 
     if (is_listen) {
@@ -3978,7 +3998,8 @@ static CharDriverState *qmp_chardev_open_socket(ChardevSocket *sock,
         return NULL;
     }
     return qemu_chr_open_socket_fd(fd, do_nodelay, is_listen,
-                                   is_telnet, is_waitconnect, errp);
+                                   is_telnet, is_waitconnect,
+                                   is_exit_on_eof, errp);
 }
 
 static CharDriverState *qmp_chardev_open_udp(ChardevUdp *udp,
diff --git a/qemu-options.hx b/qemu-options.hx
index 9e54686..4b4da4f 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1927,8 +1927,9 @@ ETEXI
 DEF("chardev", HAS_ARG, QEMU_OPTION_chardev,
     "-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][,mux=on|off] (tcp)\n"
-    "-chardev socket,id=id,path=path[,server][,nowait][,telnet],[mux=on|off] (unix)\n"
+    "         [,server][,nowait][,telnet][,mux=on|off][,exit-on-eof] (tcp)\n"
+    "-chardev socket,id=id,path=path[,server][,nowait][,telnet],[mux=on|off]\n"
+    "         [,exit-on-eof] (unix)\n"
     "-chardev udp,id=id[,host=host],port=port[,localaddr=localaddr]\n"
     "         [,localport=localport][,ipv4][,ipv6][,mux=on|off]\n"
     "-chardev msmouse,id=id[,mux=on|off]\n"
@@ -1936,13 +1937,13 @@ DEF("chardev", HAS_ARG, QEMU_OPTION_chardev,
     "         [,mux=on|off]\n"
     "-chardev ringbuf,id=id[,size=size]\n"
     "-chardev file,id=id,path=path[,mux=on|off]\n"
-    "-chardev pipe,id=id,path=path[,mux=on|off]\n"
+    "-chardev pipe,id=id,path=path[,mux=on|off][,exit-on-eof]\n"
 #ifdef _WIN32
     "-chardev console,id=id[,mux=on|off]\n"
     "-chardev serial,id=id,path=path[,mux=on|off]\n"
 #else
     "-chardev pty,id=id[,mux=on|off]\n"
-    "-chardev stdio,id=id[,mux=on|off][,signal=on|off]\n"
+    "-chardev stdio,id=id[,mux=on|off][,signal=on|off][,exit-on-eof]\n"
 #endif
 #ifdef CONFIG_BRLAPI
     "-chardev braille,id=id[,mux=on|off]\n"
@@ -2000,7 +2001,7 @@ Options to each backend are described below.
 A void device. This device will not emit any data, and will drop any data it
 receives. The null backend does not take any options.
 
-@item -chardev socket ,id=@var{id} [@var{TCP options} or @var{unix options}] [,server] [,nowait] [,telnet]
+@item -chardev socket ,id=@var{id} [@var{TCP options} or @var{unix options}] [,server] [,nowait] [,telnet] [,exit-on-eof]
 
 Create a two-way stream socket, which can be either a TCP or a unix socket. A
 unix socket will be created if @option{path} is specified. Behaviour is
@@ -2014,6 +2015,8 @@ connect to a listening socket.
 @option{telnet} specifies that traffic on the socket should interpret telnet
 escape sequences.
 
+@option{exit-on-eof} specifies that QEMU should terminate upon disconnect
+
 TCP and unix socket options are given below:
 
 @table @option
@@ -2094,7 +2097,7 @@ Log all traffic received from the guest to a file.
 created if it does not already exist, and overwritten if it does. @option{path}
 is required.
 
-@item -chardev pipe ,id=@var{id} ,path=@var{path}
+@item -chardev pipe ,id=@var{id} ,path=@var{path} [,exit-on-eof]
 
 Create a two-way connection to the guest. The behaviour differs slightly between
 Windows hosts and other hosts:
@@ -2111,6 +2114,8 @@ be present.
 @option{path} forms part of the pipe path as described above. @option{path} is
 required.
 
+@option{exit-on-eof} specifies that QEMU should terminate upon disconnect
+
 @item -chardev console ,id=@var{id}
 
 Send traffic from the guest to QEMU's standard output. @option{console} does not
@@ -2141,6 +2146,8 @@ Connect to standard input and standard output of the QEMU process.
 exiting QEMU with the key sequence @key{Control-c}. This option is enabled by
 default, use @option{signal=off} to disable it.
 
+@option{exit-on-eof} specifies that QEMU should terminate upon EOF
+
 @option{stdio} is not available on Windows hosts.
 
 @item -chardev braille ,id=@var{id}
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [Qemu-devel] [PATCH v2 4/4] libqtest: use -chardev exit-on-eof to clean up QEMU
  2014-07-25 13:16 [Qemu-devel] [PATCH v2 0/4] libqtest: solve QEMU process cleanup problem Stefan Hajnoczi
                   ` (2 preceding siblings ...)
  2014-07-25 13:16 ` [Qemu-devel] [PATCH v2 3/4] qemu-char: add -chardev exit-on-eof option Stefan Hajnoczi
@ 2014-07-25 13:16 ` Stefan Hajnoczi
  3 siblings, 0 replies; 6+ messages in thread
From: Stefan Hajnoczi @ 2014-07-25 13:16 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Andreas Faerber, Markus Armbruster

When the test case aborts it is important to terminate the QEMU process
so it does not leak.  This was implemented using a SIGABRT handler
function in libqtest that sent SIGTERM to QEMU.

The SIGABRT approach is messy because it requires a global signal
handler but libqtest should support multiple simultaneous instances.

Simplify the code using the new -chardev exit-on-eof option.  QEMU will
automatically exit when our qtest socket closes.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/libqtest.c | 48 +++---------------------------------------------
 1 file changed, 3 insertions(+), 45 deletions(-)

diff --git a/tests/libqtest.c b/tests/libqtest.c
index 98e8f4b..6c3dd27 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -46,12 +46,8 @@ struct QTestState
     bool irq_level[MAX_IRQ];
     GString *rx;
     pid_t qemu_pid;  /* our child QEMU process */
-    struct sigaction sigact_old; /* restored on exit */
 };
 
-static GList *qtest_instances;
-static struct sigaction sigact_old;
-
 #define g_assert_no_errno(ret) do { \
     g_assert_cmpint(ret, !=, -1); \
 } while (0)
@@ -110,32 +106,6 @@ static void kill_qemu(QTestState *s)
     }
 }
 
-static void sigabrt_handler(int signo)
-{
-    GList *elem;
-    for (elem = qtest_instances; elem; elem = elem->next) {
-        kill_qemu(elem->data);
-    }
-}
-
-static void setup_sigabrt_handler(void)
-{
-    struct sigaction sigact;
-
-    /* Catch SIGABRT to clean up on g_assert() failure */
-    sigact = (struct sigaction){
-        .sa_handler = sigabrt_handler,
-        .sa_flags = SA_RESETHAND,
-    };
-    sigemptyset(&sigact.sa_mask);
-    sigaction(SIGABRT, &sigact, &sigact_old);
-}
-
-static void cleanup_sigabrt_handler(void)
-{
-    sigaction(SIGABRT, &sigact_old, NULL);
-}
-
 QTestState *qtest_init(const char *extra_args)
 {
     QTestState *s;
@@ -156,17 +126,12 @@ QTestState *qtest_init(const char *extra_args)
     sock = init_socket(socket_path);
     qmpsock = init_socket(qmp_socket_path);
 
-    /* Only install SIGABRT handler once */
-    if (!qtest_instances) {
-        setup_sigabrt_handler();
-    }
-
-    qtest_instances = g_list_prepend(qtest_instances, s);
-
     s->qemu_pid = fork();
     if (s->qemu_pid == 0) {
         command = g_strdup_printf("exec %s "
-                                  "-qtest unix:%s,nowait "
+                                  "-chardev socket,id=qtestdev,path=%s,nowait,"
+                                  "exit-on-eof "
+                                  "-qtest chardev:qtestdev "
                                   "-qtest-log /dev/null "
                                   "-qmp unix:%s,nowait "
                                   "-machine accel=qtest "
@@ -207,13 +172,6 @@ QTestState *qtest_init(const char *extra_args)
 
 void qtest_quit(QTestState *s)
 {
-    /* Uninstall SIGABRT handler on last instance */
-    if (qtest_instances && !qtest_instances->next) {
-        cleanup_sigabrt_handler();
-    }
-
-    qtest_instances = g_list_remove(qtest_instances, s);
-
     kill_qemu(s);
     close(s->fd);
     close(s->qmp_fd);
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH v2 3/4] qemu-char: add -chardev exit-on-eof option
  2014-07-25 13:16 ` [Qemu-devel] [PATCH v2 3/4] qemu-char: add -chardev exit-on-eof option Stefan Hajnoczi
@ 2014-07-28 21:21   ` Eric Blake
  0 siblings, 0 replies; 6+ messages in thread
From: Eric Blake @ 2014-07-28 21:21 UTC (permalink / raw)
  To: Stefan Hajnoczi, qemu-devel
  Cc: Peter Maydell, Andreas Faerber, Markus Armbruster

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

On 07/25/2014 07:16 AM, Stefan Hajnoczi wrote:
> When QEMU is executed as part of a test case or from a script, it is
> usually desirable to exit if the parent process terminates.  This
> ensures that "leaked" QEMU processes do not continue consuming resources
> after their parent has died.
> 
> This patch adds the -chardev exit-on-eof option causing socket and pipe
> chardevs to exit QEMU upon close.  This happens when a parent process
> deliberately closes its file descriptor but also when the kernel cleans
> up a crashed process.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  include/sysemu/char.h |  1 +
>  qapi-schema.json      | 23 ++++++++++++++++-------
>  qemu-char.c           | 33 +++++++++++++++++++++++++++------
>  qemu-options.hx       | 19 +++++++++++++------
>  4 files changed, 57 insertions(+), 19 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2014-07-28 21:21 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-25 13:16 [Qemu-devel] [PATCH v2 0/4] libqtest: solve QEMU process cleanup problem Stefan Hajnoczi
2014-07-25 13:16 ` [Qemu-devel] [PATCH v2 1/4] vl: add qemu_system_shutdown_force() Stefan Hajnoczi
2014-07-25 13:16 ` [Qemu-devel] [PATCH v2 2/4] libqemustub: " Stefan Hajnoczi
2014-07-25 13:16 ` [Qemu-devel] [PATCH v2 3/4] qemu-char: add -chardev exit-on-eof option Stefan Hajnoczi
2014-07-28 21:21   ` Eric Blake
2014-07-25 13:16 ` [Qemu-devel] [PATCH v2 4/4] libqtest: use -chardev exit-on-eof to clean up QEMU Stefan Hajnoczi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.