[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5] qemu-nbd: Implement socket activation.
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v5] qemu-nbd: Implement socket activation. |
Date: |
Mon, 6 Feb 2017 13:04:52 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0 |
On 02/04/2017 04:03 AM, Richard W.M. Jones wrote:
> Socket activation (sometimes known as systemd socket activation)
> allows an Internet superserver to pass a pre-opened listening socket
> to the process, instead of having qemu-nbd open a socket itself. This
> is done via the LISTEN_FDS and LISTEN_PID environment variables, and a
> standard file descriptor range.
>
> This change partially implements socket activation for qemu-nbd. If
> the environment variables are set correctly, then socket activation
> will happen automatically, otherwise everything works as before. The
> limitation is that LISTEN_FDS must be 1.
>
> Signed-off-by: Richard W.M. Jones.
> ---
> +
> + /* So the file descriptors don't leak into child processes. */
> + for (i = 0; i < nr_fds; ++i) {
> + fd = FIRST_SOCKET_ACTIVATION_FD + i;
> + if (fcntl(fd, F_SETFD, FD_CLOEXEC) == -1) {
This is not the right way to do things (it risks clearing any other FD_
flags currently set on the fd, even though there aren't any such FD_
flags in common use). You HAVE to do F_GETFD before F_SETFD
(read-modify-write) for correct usage - and the best way to do that is
to use qemu_set_cloexec() from util/oslib-posix.c.
Except that qemu_set_cloexec() doesn't pay attention to errors...
> + /* If we cannot set FD_CLOEXEC then it probably means the file
> + * descriptor is invalid, so socket activation has gone wrong
> + * and we should exit.
> + */
> + error_report("Socket activation failed: "
> + "invalid file descriptor fd = %d: %m",
> + fd);
...while you use it for argument validation. Hmm.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature