* [Qemu-devel] [PULL 1/4] qemu-ga: obey LISTEN_PID when using systemd socket activation
2017-03-20 16:04 [Qemu-devel] [PULL 0/4] Misc fixes for 2.9.0-rc1 Paolo Bonzini
@ 2017-03-20 16:04 ` Paolo Bonzini
2017-03-20 16:04 ` [Qemu-devel] [PULL 2/4] xen: do not build backends for targets that do not support xen Paolo Bonzini
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Paolo Bonzini @ 2017-03-20 16:04 UTC (permalink / raw)
To: qemu-devel; +Cc: Richard W.M. Jones, Stefan Hajnoczi
qemu-ga's socket activation support was not obeying the LISTEN_PID
environment variable, which avoids that a process uses a socket-activation
file descriptor meant for its parent.
Mess can for example ensue if a process forks a children before consuming
the socket-activation file descriptor and therefore setting O_CLOEXEC
on it.
Luckily, qemu-nbd also got socket activation code, and its copy does
support LISTEN_PID. Some extra fixups are needed to ensure that the
code can be used for both, but that's what this patch does. The
main change is to replace get_listen_fds's "consume" argument with
the FIRST_SOCKET_ACTIVATION_FD macro from the qemu-nbd code.
Cc: "Richard W.M. Jones" <rjones@redhat.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/qemu/systemd.h | 26 +++++++++++++
qemu-nbd.c | 100 ++++---------------------------------------------
qga/main.c | 51 +++++++------------------
util/Makefile.objs | 1 +
util/systemd.c | 77 +++++++++++++++++++++++++++++++++++++
5 files changed, 125 insertions(+), 130 deletions(-)
create mode 100644 include/qemu/systemd.h
create mode 100644 util/systemd.c
diff --git a/include/qemu/systemd.h b/include/qemu/systemd.h
new file mode 100644
index 0000000..e6a877e
--- /dev/null
+++ b/include/qemu/systemd.h
@@ -0,0 +1,26 @@
+/*
+ * systemd socket activation support
+ *
+ * Copyright 2017 Red Hat, Inc. and/or its affiliates
+ *
+ * Authors:
+ * Richard W.M. Jones <rjones@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifndef QEMU_SYSTEMD_H
+#define QEMU_SYSTEMD_H 1
+
+#define FIRST_SOCKET_ACTIVATION_FD 3 /* defined by systemd ABI */
+
+/*
+ * 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.
+ */
+unsigned int check_socket_activation(void);
+
+#endif
diff --git a/qemu-nbd.c b/qemu-nbd.c
index e4fede6..e080fb7 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -28,6 +28,7 @@
#include "qemu/config-file.h"
#include "qemu/bswap.h"
#include "qemu/log.h"
+#include "qemu/systemd.h"
#include "block/snapshot.h"
#include "qapi/util.h"
#include "qapi/qmp/qstring.h"
@@ -474,98 +475,6 @@ static void setup_address_and_port(const char **address, const char **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 long pid;
- unsigned long nr_fds;
- unsigned int i;
- int fd;
- int err;
-
- s = getenv("LISTEN_PID");
- if (s == NULL) {
- return 0;
- }
- err = qemu_strtoul(s, NULL, 10, &pid);
- if (err) {
- 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;
- }
- err = qemu_strtoul(s, NULL, 10, &nr_fds);
- if (err) {
- if (verbose) {
- fprintf(stderr, "malformed %s environment variable (ignored)\n",
- "LISTEN_FDS");
- }
- return 0;
- }
- assert(nr_fds <= UINT_MAX);
-
- /* 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 (unsigned int) nr_fds;
-}
-
-#else /* !_WIN32 */
-static unsigned int check_socket_activation(void)
-{
- return 0;
-}
-#endif
-
/*
* Check socket parameters compatibility when socket activation is used.
*/
@@ -892,6 +801,13 @@ int main(int argc, char **argv)
error_report("%s", err_msg);
exit(EXIT_FAILURE);
}
+
+ /* qemu-nbd can only listen on a single socket. */
+ if (socket_activation > 1) {
+ error_report("qemu-nbd does not support socket activation with %s > 1",
+ "LISTEN_FDS");
+ exit(EXIT_FAILURE);
+ }
}
if (tlscredsid) {
diff --git a/qga/main.c b/qga/main.c
index 92658bc..07c2953 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -29,6 +29,7 @@
#include "qemu/bswap.h"
#include "qemu/help_option.h"
#include "qemu/sockets.h"
+#include "qemu/systemd.h"
#ifdef _WIN32
#include "qga/service-win32.h"
#include "qga/vss-win32.h"
@@ -186,37 +187,6 @@ void reopen_fd_to_null(int fd)
}
#endif
-/**
- * get_listen_fd:
- * @consume: true to prevent future calls from succeeding
- *
- * Fetch a listen file descriptor that was passed via systemd socket
- * activation. Use @consume to prevent child processes from thinking a file
- * descriptor was passed.
- *
- * Returns: file descriptor or -1 if no fd was passed
- */
-static int get_listen_fd(bool consume)
-{
-#ifdef _WIN32
- return -1; /* no fd passing expected, unsetenv(3) not available */
-#else
- const char *listen_fds = getenv("LISTEN_FDS");
- int fd = STDERR_FILENO + 1;
-
- if (!listen_fds || strcmp(listen_fds, "1") != 0) {
- return -1;
- }
-
- if (consume) {
- unsetenv("LISTEN_FDS");
- }
-
- qemu_set_cloexec(fd);
- return fd;
-#endif /* !_WIN32 */
-}
-
static void usage(const char *cmd)
{
printf(
@@ -1251,7 +1221,7 @@ static bool check_is_frozen(GAState *s)
return false;
}
-static int run_agent(GAState *s, GAConfig *config)
+static int run_agent(GAState *s, GAConfig *config, int socket_activation)
{
ga_state = s;
@@ -1333,7 +1303,7 @@ static int run_agent(GAState *s, GAConfig *config)
s->main_loop = g_main_loop_new(NULL, false);
if (!channel_init(ga_state, config->method, config->channel_path,
- get_listen_fd(true))) {
+ socket_activation ? FIRST_SOCKET_ACTIVATION_FD : -1)) {
g_critical("failed to initialize guest agent channel");
return EXIT_FAILURE;
}
@@ -1357,7 +1327,7 @@ int main(int argc, char **argv)
int ret = EXIT_SUCCESS;
GAState *s = g_new0(GAState, 1);
GAConfig *config = g_new0(GAConfig, 1);
- int listen_fd;
+ int socket_activation;
config->log_level = G_LOG_LEVEL_ERROR | G_LOG_LEVEL_CRITICAL;
@@ -1379,8 +1349,13 @@ int main(int argc, char **argv)
config->method = g_strdup("virtio-serial");
}
- listen_fd = get_listen_fd(false);
- if (listen_fd >= 0) {
+ socket_activation = check_socket_activation();
+ if (socket_activation > 1) {
+ g_critical("qemu-ga only supports listening on one socket");
+ ret = EXIT_FAILURE;
+ goto end;
+ }
+ if (socket_activation) {
SocketAddress *addr;
g_free(config->method);
@@ -1388,7 +1363,7 @@ int main(int argc, char **argv)
config->method = NULL;
config->channel_path = NULL;
- addr = socket_local_address(listen_fd, NULL);
+ addr = socket_local_address(FIRST_SOCKET_ACTIVATION_FD, NULL);
if (addr) {
if (addr->type == SOCKET_ADDRESS_KIND_UNIX) {
config->method = g_strdup("unix-listen");
@@ -1433,7 +1408,7 @@ int main(int argc, char **argv)
goto end;
}
- ret = run_agent(s, config);
+ ret = run_agent(s, config, socket_activation);
end:
if (s->command_state) {
diff --git a/util/Makefile.objs b/util/Makefile.objs
index 06366b5..c6205eb 100644
--- a/util/Makefile.objs
+++ b/util/Makefile.objs
@@ -42,3 +42,4 @@ util-obj-y += log.o
util-obj-y += qdist.o
util-obj-y += qht.o
util-obj-y += range.o
+util-obj-y += systemd.o
diff --git a/util/systemd.c b/util/systemd.c
new file mode 100644
index 0000000..d22e86c
--- /dev/null
+++ b/util/systemd.c
@@ -0,0 +1,77 @@
+/*
+ * systemd socket activation support
+ *
+ * Copyright 2017 Red Hat, Inc. and/or its affiliates
+ *
+ * Authors:
+ * Richard W.M. Jones <rjones@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/systemd.h"
+#include "qemu/cutils.h"
+#include "qemu/error-report.h"
+
+#ifndef _WIN32
+unsigned int check_socket_activation(void)
+{
+ const char *s;
+ unsigned long pid;
+ unsigned long nr_fds;
+ unsigned int i;
+ int fd;
+ int err;
+
+ s = getenv("LISTEN_PID");
+ if (s == NULL) {
+ return 0;
+ }
+ err = qemu_strtoul(s, NULL, 10, &pid);
+ if (err) {
+ return 0;
+ }
+ if (pid != getpid()) {
+ return 0;
+ }
+
+ s = getenv("LISTEN_FDS");
+ if (s == NULL) {
+ return 0;
+ }
+ err = qemu_strtoul(s, NULL, 10, &nr_fds);
+ if (err) {
+ return 0;
+ }
+ assert(nr_fds <= UINT_MAX);
+
+ /* 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 (unsigned int) nr_fds;
+}
+
+#else /* !_WIN32 */
+unsigned int check_socket_activation(void)
+{
+ return 0;
+}
+#endif
--
2.9.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* [Qemu-devel] [PULL 2/4] xen: do not build backends for targets that do not support xen
2017-03-20 16:04 [Qemu-devel] [PULL 0/4] Misc fixes for 2.9.0-rc1 Paolo Bonzini
2017-03-20 16:04 ` [Qemu-devel] [PULL 1/4] qemu-ga: obey LISTEN_PID when using systemd socket activation Paolo Bonzini
@ 2017-03-20 16:04 ` Paolo Bonzini
2017-03-20 16:04 ` [Qemu-devel] [PULL 3/4] configure: remove Cygwin Paolo Bonzini
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Paolo Bonzini @ 2017-03-20 16:04 UTC (permalink / raw)
To: qemu-devel
Cc: Stefano Stabellini, Stefano Stabellini, pbonzini, peter.maydell,
rth, stefanha
From: Stefano Stabellini <sstabellini@kernel.org>
Change Makefile.objs to use CONFIG_XEN instead of CONFIG_XEN_BACKEND, so
that the Xen backends are only built for targets that support Xen.
Set CONFIG_XEN in the toplevel Makefile to ensure that files that are
built only once pick up Xen support properly.
Signed-off-by: Stefano Stabellini <stefano@aporeto.com>
Tested-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Greg Kurz <groug@kaod.org>
CC: pbonzini@redhat.com
CC: peter.maydell@linaro.org
CC: rth@twiddle.net
CC: stefanha@redhat.com
Message-Id: <1489694518-16978-1-git-send-email-sstabellini@kernel.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
Makefile | 1 +
hw/block/Makefile.objs | 2 +-
hw/char/Makefile.objs | 2 +-
hw/display/Makefile.objs | 2 +-
hw/net/Makefile.objs | 2 +-
hw/usb/Makefile.objs | 2 +-
hw/xen/Makefile.objs | 2 +-
7 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/Makefile b/Makefile
index 73e0c12..f4f90df 100644
--- a/Makefile
+++ b/Makefile
@@ -26,6 +26,7 @@ endif
CONFIG_SOFTMMU := $(if $(filter %-softmmu,$(TARGET_DIRS)),y)
CONFIG_USER_ONLY := $(if $(filter %-user,$(TARGET_DIRS)),y)
+CONFIG_XEN := $(CONFIG_XEN_BACKEND)
CONFIG_ALL=y
-include config-all-devices.mak
-include config-all-disas.mak
diff --git a/hw/block/Makefile.objs b/hw/block/Makefile.objs
index d4c3ab7..e0ed980 100644
--- a/hw/block/Makefile.objs
+++ b/hw/block/Makefile.objs
@@ -4,7 +4,7 @@ common-obj-$(CONFIG_SSI_M25P80) += m25p80.o
common-obj-$(CONFIG_NAND) += nand.o
common-obj-$(CONFIG_PFLASH_CFI01) += pflash_cfi01.o
common-obj-$(CONFIG_PFLASH_CFI02) += pflash_cfi02.o
-common-obj-$(CONFIG_XEN_BACKEND) += xen_disk.o
+common-obj-$(CONFIG_XEN) += xen_disk.o
common-obj-$(CONFIG_ECC) += ecc.o
common-obj-$(CONFIG_ONENAND) += onenand.o
common-obj-$(CONFIG_NVME_PCI) += nvme.o
diff --git a/hw/char/Makefile.objs b/hw/char/Makefile.objs
index 6ea76fe..725fdc4 100644
--- a/hw/char/Makefile.objs
+++ b/hw/char/Makefile.objs
@@ -7,7 +7,7 @@ common-obj-$(CONFIG_SERIAL_ISA) += serial-isa.o
common-obj-$(CONFIG_SERIAL_PCI) += serial-pci.o
common-obj-$(CONFIG_VIRTIO) += virtio-console.o
common-obj-$(CONFIG_XILINX) += xilinx_uartlite.o
-common-obj-$(CONFIG_XEN_BACKEND) += xen_console.o
+common-obj-$(CONFIG_XEN) += xen_console.o
common-obj-$(CONFIG_CADENCE) += cadence_uart.o
obj-$(CONFIG_EXYNOS4) += exynos4210_uart.o
diff --git a/hw/display/Makefile.objs b/hw/display/Makefile.objs
index 063889b..3d02e8b 100644
--- a/hw/display/Makefile.objs
+++ b/hw/display/Makefile.objs
@@ -5,7 +5,7 @@ common-obj-$(CONFIG_JAZZ_LED) += jazz_led.o
common-obj-$(CONFIG_PL110) += pl110.o
common-obj-$(CONFIG_SSD0303) += ssd0303.o
common-obj-$(CONFIG_SSD0323) += ssd0323.o
-common-obj-$(CONFIG_XEN_BACKEND) += xenfb.o
+common-obj-$(CONFIG_XEN) += xenfb.o
common-obj-$(CONFIG_VGA_PCI) += vga-pci.o
common-obj-$(CONFIG_VGA_ISA) += vga-isa.o
diff --git a/hw/net/Makefile.objs b/hw/net/Makefile.objs
index 610ed3e..6a95d92 100644
--- a/hw/net/Makefile.objs
+++ b/hw/net/Makefile.objs
@@ -1,5 +1,5 @@
common-obj-$(CONFIG_DP8393X) += dp8393x.o
-common-obj-$(CONFIG_XEN_BACKEND) += xen_nic.o
+common-obj-$(CONFIG_XEN) += xen_nic.o
# PCI network cards
common-obj-$(CONFIG_NE2000_PCI) += ne2000.o
diff --git a/hw/usb/Makefile.objs b/hw/usb/Makefile.objs
index 98b5c9d..5958be8 100644
--- a/hw/usb/Makefile.objs
+++ b/hw/usb/Makefile.objs
@@ -40,5 +40,5 @@ common-obj-$(CONFIG_USB_REDIR) += redirect.o quirks.o
common-obj-y += $(patsubst %,host-%.o,$(HOST_USB))
ifeq ($(CONFIG_USB_LIBUSB),y)
-common-obj-$(CONFIG_XEN_BACKEND) += xen-usb.o
+common-obj-$(CONFIG_XEN) += xen-usb.o
endif
diff --git a/hw/xen/Makefile.objs b/hw/xen/Makefile.objs
index 591cdc2..4be3ec9 100644
--- a/hw/xen/Makefile.objs
+++ b/hw/xen/Makefile.objs
@@ -1,5 +1,5 @@
# xen backend driver support
-common-obj-$(CONFIG_XEN_BACKEND) += xen_backend.o xen_devconfig.o xen_pvdev.o
+common-obj-$(CONFIG_XEN) += xen_backend.o xen_devconfig.o xen_pvdev.o
obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen-host-pci-device.o
obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen_pt.o xen_pt_config_init.o xen_pt_graphics.o xen_pt_msi.o
--
2.9.3
^ permalink raw reply related [flat|nested] 6+ messages in thread