Re: [Qemu-devel] [PATCH v2 2/2] char: allow passing pre-opened socket file descriptor at startup

[Eric Blake <eblake@redhat.com>]

On 12/21/2017 09:59 AM, Daniel P. Berrange wrote:
> When starting QEMU management apps will usually setup a monitor socket, and
> then open it immediately after startup. If not using QEMU's own -daemonize
> arg, this process can be troublesome to handle correctly. The mgmt app will
> need to repeatedly call connect() until it succeeds, because it does not
> know when QEMU has created the listener socket. If can't retry connect()
> forever though, because an error might have caused QEMU to exit before it
> even creates the monitor.
> 
> The obvious way to fix this kind of problem is to just pass in a pre-opened
> socket file descriptor for the QEMU monitor to listen on. The management app
> can now immediately call connect() just once. If connect() fails it knows
> that QEMU has exited with an error.
> 

> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>   chardev/char-socket.c |  32 ++++++++--
>   chardev/char.c        |   3 +
>   tests/test-char.c     | 173 +++++++++++++++++++++++++++++++++++++++++++++++++-
>   util/qemu-sockets.c   |  42 +++++++++++-
>   4 files changed, 238 insertions(+), 12 deletions(-)
> 

> @@ -983,25 +984,36 @@ static void qemu_chr_parse_socket(QemuOpts *opts, ChardevBackend *backend,
>       const char *path = qemu_opt_get(opts, "path");
>       const char *host = qemu_opt_get(opts, "host");
>       const char *port = qemu_opt_get(opts, "port");
> +    const char *fd = qemu_opt_get(opts, "fd");
>       const char *tls_creds = qemu_opt_get(opts, "tls-creds");
>       SocketAddressLegacy *addr;
>       ChardevSocket *sock;
>   
> +    if ((!!path + !!fd + !!host) != 1) {

I see you liked my suggestion for a compact rendering; the resulting 
line has more symbols than it does alphanumerics ;)


> +/* Syms in libqemustub.a are discarded at .o file granularity.
> + * To replace monitor_get_fd() we must ensure everything in
> + * stubs/monitor.c is defined, to make sure monitor.o is discarded
> + * otherwise we get duplicate syms at link time.
> + */
> +Monitor *cur_mon = NULL;

Patchew is correct that you can omit the ' = NULL'.

With that tweaked,
Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org