| Message ID | 1406219965-13214-3-git-send-email-stefanha@redhat.com |
|---|---|
| State | New |
| Headers | show |
Stefan Hajnoczi <stefanha@redhat.com> writes: > 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 | 34 ++++++++++++++++++++++++++++------ > qemu-options.hx | 19 +++++++++++++------ > 4 files changed, 58 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: Any use cases beyond libqtest? If no, should this be x-exit-on-eof? Hmm, looks like there's no precedence for x- in QAPI.
On Fri, Jul 25, 2014 at 9:12 AM, Markus Armbruster <armbru@redhat.com> wrote: > Stefan Hajnoczi <stefanha@redhat.com> writes: > >> 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 | 34 ++++++++++++++++++++++++++++------ >> qemu-options.hx | 19 +++++++++++++------ >> 4 files changed, 58 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: > > Any use cases beyond libqtest? qemu-iotests should use it too. Basically any script that drives QEMU directly will need to clean up the QEMU process. Stefan
On 07/25/2014 02:12 AM, Markus Armbruster wrote: > Stefan Hajnoczi <stefanha@redhat.com> writes: > >> 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. >> ## >> -{ 'type': 'ChardevHostdev', 'data': { 'device' : 'str' } } >> +{ 'type': 'ChardevHostdev', 'data': { 'device' : 'str', >> + '*exit-on-eof' : 'bool' } } >> >> ## >> # @ChardevSocket: > > Any use cases beyond libqtest? Libvirt probably won't use it for normal guests (we don't want to kill qemu just because the monitor disconnects), but does have the notion of an autodestroy guest where it might be useful (we WANT the guest to go away if libvirtd dies early). In fact, autodestroy guests are used during migration - we want to kill qemu on the destination side if libvirtd dies before the source side finishes sending the migration stream. But in that scenario, once migration succeeds, libvirt has to be able to convert an autodestroy guest back into a normal guest that no longer disappears when libvirtd does; would this be something that QMP can toggle the state of this attribute on the fly? Perhaps through qom-set? > > If no, should this be x-exit-on-eof? > > Hmm, looks like there's no precedence for x- in QAPI. Ah, but we do. For example, x-rdma-pin-all in MigrationCapability in qemu 1.7; which has later been removed. However, we also have precedence of actions in QAPI that are very unlikely to be used outside of qtest, but which are not marked experimental; for example, the 'Abort' action in 'transaction' will probably never be used by libvirt (but as I type that, the thought in the back of my mind is that I could possibly do some sort of feature probing by setting up a transaction that will abort, and distinguish between whether the transaction aborted or whether it errored out because the feature I'm probing for isn't present). I'm fine leaving this as plain 'exit-on-eof'.
Eric Blake <eblake@redhat.com> writes: > On 07/25/2014 02:12 AM, Markus Armbruster wrote: >> Stefan Hajnoczi <stefanha@redhat.com> writes: >> >>> 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. > >>> ## >>> -{ 'type': 'ChardevHostdev', 'data': { 'device' : 'str' } } >>> +{ 'type': 'ChardevHostdev', 'data': { 'device' : 'str', >>> + '*exit-on-eof' : 'bool' } } >>> >>> ## >>> # @ChardevSocket: >> >> Any use cases beyond libqtest? > > Libvirt probably won't use it for normal guests (we don't want to kill > qemu just because the monitor disconnects), but does have the notion of > an autodestroy guest where it might be useful (we WANT the guest to go > away if libvirtd dies early). In fact, autodestroy guests are used > during migration - we want to kill qemu on the destination side if > libvirtd dies before the source side finishes sending the migration > stream. But in that scenario, once migration succeeds, libvirt has to > be able to convert an autodestroy guest back into a normal guest that no > longer disappears when libvirtd does; would this be something that QMP > can toggle the state of this attribute on the fly? Perhaps through qom-set? After migration completes, execution moves from source to target. Wouldn't you want to switch off target auto-destruct together with that move, atomically? >> If no, should this be x-exit-on-eof? >> >> Hmm, looks like there's no precedence for x- in QAPI. > > Ah, but we do. For example, x-rdma-pin-all in MigrationCapability in > qemu 1.7; which has later been removed. > > However, we also have precedence of actions in QAPI that are very > unlikely to be used outside of qtest, but which are not marked > experimental; for example, the 'Abort' action in 'transaction' will > probably never be used by libvirt Arguably not a conscious decision to make it ABI forever, more a case of nobody thought about *not* making it ABI :) > (but as I type that, the thought in > the back of my mind is that I could possibly do some sort of feature > probing by setting up a transaction that will abort, and distinguish > between whether the transaction aborted or whether it errored out > because the feature I'm probing for isn't present). > > I'm fine leaving this as plain 'exit-on-eof'. I don't really mind this instance, but I'm a bit concerned about rank ABI growth.
On 07/29/2014 12:12 AM, Markus Armbruster wrote: >> Libvirt probably won't use it for normal guests (we don't want to kill >> qemu just because the monitor disconnects), but does have the notion of >> an autodestroy guest where it might be useful (we WANT the guest to go >> away if libvirtd dies early). In fact, autodestroy guests are used >> during migration - we want to kill qemu on the destination side if >> libvirtd dies before the source side finishes sending the migration >> stream. But in that scenario, once migration succeeds, libvirt has to >> be able to convert an autodestroy guest back into a normal guest that no >> longer disappears when libvirtd does; would this be something that QMP >> can toggle the state of this attribute on the fly? Perhaps through qom-set? > > After migration completes, execution moves from source to target. > Wouldn't you want to switch off target auto-destruct together with that > move, atomically? Libvirt starts the destination with -S, and migration can't complete until libvirt resumes the destination CPUs with 'cont'. Libvirt's current timing of releasing auto-destruct is based on handshaking between source and destination; it occurs after source claims migration is done but before resuming CPUs on the destination, which satisfies the atomicity that you correctly observed to be necessary. >> However, we also have precedence of actions in QAPI that are very >> unlikely to be used outside of qtest, but which are not marked >> experimental; for example, the 'Abort' action in 'transaction' will >> probably never be used by libvirt > > Arguably not a conscious decision to make it ABI forever, more a case of > nobody thought about *not* making it ABI :) Added in June 2013; and we *did* have a discussion on whether to hide the transaction name at the time... https://lists.gnu.org/archive/html/qemu-devel/2013-06/msg03281.html > I don't really mind this instance, but I'm a bit concerned about rank > ABI growth. And that's a good position to maintain - it's always good to justify new knobs, especially since once they are ABI, it's harder to refactor around them.
Eric Blake <eblake@redhat.com> writes: > On 07/29/2014 12:12 AM, Markus Armbruster wrote: > >>> Libvirt probably won't use it for normal guests (we don't want to kill >>> qemu just because the monitor disconnects), but does have the notion of >>> an autodestroy guest where it might be useful (we WANT the guest to go >>> away if libvirtd dies early). In fact, autodestroy guests are used >>> during migration - we want to kill qemu on the destination side if >>> libvirtd dies before the source side finishes sending the migration >>> stream. But in that scenario, once migration succeeds, libvirt has to >>> be able to convert an autodestroy guest back into a normal guest that no >>> longer disappears when libvirtd does; would this be something that QMP >>> can toggle the state of this attribute on the fly? Perhaps through qom-set? >> >> After migration completes, execution moves from source to target. >> Wouldn't you want to switch off target auto-destruct together with that >> move, atomically? > > Libvirt starts the destination with -S, and migration can't complete > until libvirt resumes the destination CPUs with 'cont'. Libvirt's > current timing of releasing auto-destruct is based on handshaking > between source and destination; it occurs after source claims migration > is done but before resuming CPUs on the destination, which satisfies the > atomicity that you correctly observed to be necessary. Makes sense, thanks. >>> However, we also have precedence of actions in QAPI that are very >>> unlikely to be used outside of qtest, but which are not marked >>> experimental; for example, the 'Abort' action in 'transaction' will >>> probably never be used by libvirt >> >> Arguably not a conscious decision to make it ABI forever, more a case of >> nobody thought about *not* making it ABI :) > > Added in June 2013; and we *did* have a discussion on whether to hide > the transaction name at the time... > https://lists.gnu.org/archive/html/qemu-devel/2013-06/msg03281.html I stand corrected. >> I don't really mind this instance, but I'm a bit concerned about rank >> ABI growth. > > And that's a good position to maintain - it's always good to justify new > knobs, especially since once they are ABI, it's harder to refactor > around them. Exactly.
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..9015bc9 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -110,9 +110,16 @@ 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); + no_shutdown = 0; + qemu_system_shutdown_request(); + } } void qemu_chr_be_generic_open(CharDriverState *s) @@ -991,6 +998,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 +1019,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 +2903,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 +2966,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 +3003,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 +3026,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 +3389,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 +3430,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 +3865,9 @@ QemuOptsList qemu_chardev_opts = { },{ .name = "chardev", .type = QEMU_OPT_STRING, + },{ + .name = "exit-on-eof", + .type = QEMU_OPT_BOOL, }, { /* end of list */ } }, @@ -3967,6 +3987,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 +3999,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}
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 | 34 ++++++++++++++++++++++++++++------ qemu-options.hx | 19 +++++++++++++------ 4 files changed, 58 insertions(+), 19 deletions(-)