* [Qemu-devel] [PATCH] qemu-nbd: Implement socket activation.
@ 2017-02-02 14:51 Richard W.M. Jones
2017-02-02 14:51 ` Richard W.M. Jones
0 siblings, 1 reply; 3+ messages in thread
From: Richard W.M. Jones @ 2017-02-02 14:51 UTC (permalink / raw)
To: pbonzini; +Cc: qemu-devel, den, rkagan, dplotnikov, berrange, stefanha
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 patch partially implements socket activation.
The limitation of this implementation is that qemu-nbd can only listen
on a single file descriptor, and so if LISTEN_FDS > 1 (eg. for
listening on multiple interfaces or ports) socket activation will
fail. However for the simple case of listening on a single port, and
either all interfaces with IPv4+IPv6, or just a loopback interface,
the current implementation works fine. Fixing this properly would
require considerable changes throughout qemu, since qemu's currently
handling of getaddrinfo is plainly wrong.
To use qemu-nbd from systemd, you create
/etc/systemd/system/nbd.socket:
[Unit]
Description=QEMU Network Block Device server
[Socket]
ListenStream=10809
[Install]
WantedBy=sockets.target
and /etc/systemd/system/nbd.service:
[Service]
ExecStart=/usr/sbin/qemu-nbd -v -t /path/to/file
and enable the socket service (only):
systemctl enable nbd.socket
systemctl start nbd.socket
and then connecting to port 10809 will start qemu-nbd and service the
file, with systemd opening the listening socket.
In the ExecStart line, the qemu-nbd -v option is only needed if you
want enhanced debugging. The -t option is required unless you want to
fiddle with systemd settings for rate-limiting.
If you try to use the -p and similar options with socket activation
then qemu-nbd will give an error.
(I wasn't sure where to document this -- there is no obvious
documentation for qemu-nbd beyond the simple list of command line
arguments)
This is based on the implementations in
libvirt (src/util/virutil.c:virGetListenFDs) and
nbdkit (src/main.c:get_socket_activation), and also on Denis Plotnikov's
implementation of --server-sock-fd
(https://lists.gnu.org/archive/html/qemu-devel/2016-09/msg07781.html).
Rich.
^ permalink raw reply [flat|nested] 3+ messages in thread
* [Qemu-devel] [PATCH] qemu-nbd: Implement socket activation.
2017-02-02 14:51 [Qemu-devel] [PATCH] qemu-nbd: Implement socket activation Richard W.M. Jones
@ 2017-02-02 14:51 ` Richard W.M. Jones
2017-02-02 15:04 ` Daniel P. Berrange
0 siblings, 1 reply; 3+ messages in thread
From: Richard W.M. Jones @ 2017-02-02 14:51 UTC (permalink / raw)
To: pbonzini; +Cc: qemu-devel, den, rkagan, dplotnikov, berrange, stefanha
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.
---
qemu-nbd.c | 174 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 162 insertions(+), 12 deletions(-)
diff --git a/qemu-nbd.c b/qemu-nbd.c
index c734f62..bfa52c3 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -386,9 +386,9 @@ static void nbd_update_server_watch(void)
}
-static SocketAddress *nbd_build_socket_address(const char *sockpath,
- const char *bindto,
- const char *port)
+static SocketAddress *nbd_build_socket_fd(const char *sockpath,
+ const char *bindto,
+ const char *port)
{
SocketAddress *saddr;
@@ -463,6 +463,131 @@ static QCryptoTLSCreds *nbd_get_tls_creds(const char *id, Error **errp)
return creds;
}
+static void setup_address_and_port(const char **address, const char **port)
+{
+ if (*address == NULL) {
+ *address = "0.0.0.0";
+ }
+
+ if (*port == NULL) {
+ *port = g_strdup_printf("%d", NBD_DEFAULT_PORT);;
+ }
+}
+
+#define FIRST_SOCKET_ACTIVATION_FD 3 /* defined by systemd ABI */
+
+#ifndef _WIN32
+/*
+ * Check if socket activation was requested via use of the
+ * LISTEN_FDS and LISTEN_PID environment variables.
+ *
+ * Returns 0 if no socket activation, or the number of FDs.
+ */
+static unsigned int check_socket_activation(void)
+{
+ const char *s;
+ unsigned int pid;
+ unsigned int nr_fds;
+ unsigned int i;
+ int fd;
+
+ s = getenv("LISTEN_PID");
+ if (s == NULL) {
+ return 0;
+ }
+ if (sscanf(s, "%u", &pid) != 1) {
+ if (verbose) {
+ fprintf(stderr, "malformed %s environment variable (ignored)\n",
+ "LISTEN_PID");
+ }
+ return 0;
+ }
+ if (pid != getpid()) {
+ if (verbose) {
+ fprintf(stderr, "%s was not for us (ignored)\n",
+ "LISTEN_PID");
+ }
+ return 0;
+ }
+
+ s = getenv("LISTEN_FDS");
+ if (s == NULL) {
+ return 0;
+ }
+ if (sscanf(s, "%u", &nr_fds) != 1) {
+ if (verbose) {
+ fprintf(stderr, "malformed %s environment variable (ignored)\n",
+ "LISTEN_FDS");
+ }
+ return 0;
+ }
+
+ /* A limitation of current qemu-nbd is that it can only listen on
+ * a single socket. When that limitation is lifted, we can change
+ * this function to allow LISTEN_FDS > 1, and remove the assertion
+ * in the main function below.
+ */
+ if (nr_fds > 1) {
+ error_report("qemu-nbd does not support socket activation with %s > 1",
+ "LISTEN_FDS");
+ exit(EXIT_FAILURE);
+ }
+
+ /* So these are not passed to any child processes we might start. */
+ unsetenv("LISTEN_FDS");
+ unsetenv("LISTEN_PID");
+
+ /* 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) {
+ /* 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);
+ exit(EXIT_FAILURE);
+ }
+ }
+
+ return nr_fds;
+}
+
+#else /* !_WIN32 */
+static unsigned int check_socket_activation(void)
+{
+ return 0;
+}
+#endif
+
+/*
+ * Check socket parameters compatibility when socket activation is used.
+ */
+static const char *socket_activation_validate_opts(const char *device,
+ const char *sockpath,
+ const char *address,
+ const char *port)
+{
+ if (device != NULL) {
+ return "NBD device can't be set when using socket activation";
+ }
+
+ if (sockpath != NULL) {
+ return "Unix socket can't be set when using socket activation";
+ }
+
+ if (address != NULL) {
+ return "The interface can't be set when using socket activation";
+ }
+
+ if (port != NULL) {
+ return "TCP port number can't be set when using socket activation";
+ }
+
+ return NULL;
+}
int main(int argc, char **argv)
{
@@ -471,7 +596,7 @@ int main(int argc, char **argv)
off_t dev_offset = 0;
uint16_t nbdflags = 0;
bool disconnect = false;
- const char *bindto = "0.0.0.0";
+ const char *bindto = NULL;
const char *port = NULL;
char *sockpath = NULL;
char *device = NULL;
@@ -533,6 +658,7 @@ int main(int argc, char **argv)
char *trace_file = NULL;
bool fork_process = false;
int old_stderr = -1;
+ unsigned socket_activation;
/* The client thread uses SIGTERM to interrupt the server. A signal
* handler ensures that "qemu-nbd -v -c" exits with a nice status code.
@@ -751,6 +877,19 @@ int main(int argc, char **argv)
trace_init_file(trace_file);
qemu_set_log(LOG_TRACE);
+ socket_activation = check_socket_activation();
+ if (socket_activation == 0) {
+ setup_address_and_port(&bindto, &port);
+ } else {
+ /* Using socket activation - check user didn't use -p etc. */
+ const char *err_msg = socket_activation_validate_opts(device, sockpath,
+ bindto, port);
+ if (err_msg != NULL) {
+ error_report("%s", err_msg);
+ exit(EXIT_FAILURE);
+ }
+ }
+
if (tlscredsid) {
if (sockpath) {
error_report("TLS is only supported with IPv4/IPv6");
@@ -855,7 +994,25 @@ int main(int argc, char **argv)
snprintf(sockpath, 128, SOCKET_PATH, basename(device));
}
- saddr = nbd_build_socket_address(sockpath, bindto, port);
+ if (socket_activation == 0) {
+ server_ioc = qio_channel_socket_new();
+ saddr = nbd_build_socket_fd(sockpath, bindto, port);
+ if (qio_channel_socket_listen_sync(server_ioc, saddr, &local_err) < 0) {
+ object_unref(OBJECT(server_ioc));
+ error_report_err(local_err);
+ return 1;
+ }
+ } else {
+ /* See comment in check_socket_activation above. */
+ assert(socket_activation == 1);
+ server_ioc = qio_channel_socket_new_fd(FIRST_SOCKET_ACTIVATION_FD,
+ &local_err);
+ if (server_ioc == NULL) {
+ error_report("Failed to use socket activation: %s",
+ error_get_pretty(local_err));
+ exit(EXIT_FAILURE);
+ }
+ }
if (qemu_init_main_loop(&local_err)) {
error_report_err(local_err);
@@ -950,13 +1107,6 @@ int main(int argc, char **argv)
exit(EXIT_FAILURE);
}
- server_ioc = qio_channel_socket_new();
- if (qio_channel_socket_listen_sync(server_ioc, saddr, &local_err) < 0) {
- object_unref(OBJECT(server_ioc));
- error_report_err(local_err);
- return 1;
- }
-
if (device) {
int ret;
--
2.10.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] qemu-nbd: Implement socket activation.
2017-02-02 14:51 ` Richard W.M. Jones
@ 2017-02-02 15:04 ` Daniel P. Berrange
0 siblings, 0 replies; 3+ messages in thread
From: Daniel P. Berrange @ 2017-02-02 15:04 UTC (permalink / raw)
To: Richard W.M. Jones
Cc: pbonzini, qemu-devel, den, rkagan, dplotnikov, stefanha
On Thu, Feb 02, 2017 at 02:51:59PM +0000, 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.
> ---
> qemu-nbd.c | 174 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 162 insertions(+), 12 deletions(-)
>
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index c734f62..bfa52c3 100644
> --- a/qemu-nbd.c
> +++ b/qemu-nbd.c
> @@ -386,9 +386,9 @@ static void nbd_update_server_watch(void)
> }
>
>
> -static SocketAddress *nbd_build_socket_address(const char *sockpath,
> - const char *bindto,
> - const char *port)
> +static SocketAddress *nbd_build_socket_fd(const char *sockpath,
> + const char *bindto,
> + const char *port)
I don't think this needs renaming - its still returning an address
rather than an FD
> {
> SocketAddress *saddr;
>
> @@ -463,6 +463,131 @@ static QCryptoTLSCreds *nbd_get_tls_creds(const char *id, Error **errp)
> return creds;
> }
>
> +static void setup_address_and_port(const char **address, const char **port)
> +{
> + if (*address == NULL) {
> + *address = "0.0.0.0";
> + }
> +
> + if (*port == NULL) {
> + *port = g_strdup_printf("%d", NBD_DEFAULT_PORT);;
> + }
> +}
> +
> +#define FIRST_SOCKET_ACTIVATION_FD 3 /* defined by systemd ABI */
> +
> +#ifndef _WIN32
> +/*
> + * Check if socket activation was requested via use of the
> + * LISTEN_FDS and LISTEN_PID environment variables.
> + *
> + * Returns 0 if no socket activation, or the number of FDs.
> + */
> +static unsigned int check_socket_activation(void)
> +{
> + const char *s;
> + unsigned int pid;
> + unsigned int nr_fds;
> + unsigned int i;
> + int fd;
> +
> + s = getenv("LISTEN_PID");
> + if (s == NULL) {
> + return 0;
> + }
> + if (sscanf(s, "%u", &pid) != 1) {
IIRC qemu_strtoul would be preferred for this.
> + if (verbose) {
> + fprintf(stderr, "malformed %s environment variable (ignored)\n",
> + "LISTEN_PID");
> + }
> + return 0;
> + }
> + if (pid != getpid()) {
> + if (verbose) {
> + fprintf(stderr, "%s was not for us (ignored)\n",
> + "LISTEN_PID");
> + }
> + return 0;
> + }
> +
> + s = getenv("LISTEN_FDS");
> + if (s == NULL) {
> + return 0;
> + }
> + if (sscanf(s, "%u", &nr_fds) != 1) {
And this.
> + if (verbose) {
> + fprintf(stderr, "malformed %s environment variable (ignored)\n",
> + "LISTEN_FDS");
> + }
> + return 0;
> + }
> +
> + /* A limitation of current qemu-nbd is that it can only listen on
> + * a single socket. When that limitation is lifted, we can change
> + * this function to allow LISTEN_FDS > 1, and remove the assertion
> + * in the main function below.
> + */
> + if (nr_fds > 1) {
> + error_report("qemu-nbd does not support socket activation with %s > 1",
> + "LISTEN_FDS");
> + exit(EXIT_FAILURE);
> + }
> +
> + /* So these are not passed to any child processes we might start. */
> + unsetenv("LISTEN_FDS");
> + unsetenv("LISTEN_PID");
> +
> + /* 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) {
> + /* 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);
> + exit(EXIT_FAILURE);
> + }
> + }
> +
> + return nr_fds;
> +}
> +
> +#else /* !_WIN32 */
> +static unsigned int check_socket_activation(void)
> +{
> + return 0;
> +}
> +#endif
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-02-02 15:04 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-02-02 14:51 [Qemu-devel] [PATCH] qemu-nbd: Implement socket activation Richard W.M. Jones
2017-02-02 14:51 ` Richard W.M. Jones
2017-02-02 15:04 ` Daniel P. Berrange
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).