From 53fabd4b86e15869e13fb762686d674c64294385 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 16 Mar 2017 16:29:45 +0100 Subject: [PATCH 1/4] qemu-ga: obey LISTEN_PID when using systemd socket activation 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" Cc: Stefan Hajnoczi Reviewed-by: Daniel P. Berrange Signed-off-by: Paolo Bonzini --- 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 0000000000..e6a877e5c6 --- /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 + * + * 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 e4fede641e..e080fb7c75 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 92658bc0e2..07c295376f 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 06366b5828..c6205ebf86 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 0000000000..d22e86c707 --- /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 + * + * 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 From 6b827cca9a781c486d42724b9462b65176588680 Mon Sep 17 00:00:00 2001 From: Stefano Stabellini Date: Thu, 16 Mar 2017 13:01:50 -0700 Subject: [PATCH 2/4] xen: do not build backends for targets that do not support xen 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 Tested-by: Greg Kurz Reviewed-by: Greg Kurz 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 --- 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 73e0c121c8..f4f90dfad6 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 d4c3ab758d..e0ed980c90 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 6ea76feb12..725fdc46f4 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 063889beaf..3d02e8bfc5 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 610ed3e7ae..6a95d92d37 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 98b5c9d273..5958be8ce3 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 591cdc229d..4be3ec9c77 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 From 732a802076635e9a5d56a9e37332e5c1836d43f2 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 17 Mar 2017 16:32:47 +0100 Subject: [PATCH 3/4] configure: remove Cygwin The Cygwin target is really compiling for native Win32 with -mno-cygwin. Except, GCC 4.7.0 has finally removed the long deprecated -mno-cygwin option, and that happened about five years ago. Let it rest in peace. Signed-off-by: Paolo Bonzini --- bsd-user/mmap.c | 5 ----- configure | 6 ------ 2 files changed, 11 deletions(-) diff --git a/bsd-user/mmap.c b/bsd-user/mmap.c index ee5907330f..1ad018a127 100644 --- a/bsd-user/mmap.c +++ b/bsd-user/mmap.c @@ -199,12 +199,7 @@ static int mmap_frag(abi_ulong real_start, return 0; } -#if defined(__CYGWIN__) -/* Cygwin doesn't have a whole lot of address space. */ -static abi_ulong mmap_next_start = 0x18000000; -#else static abi_ulong mmap_next_start = 0x40000000; -#endif unsigned long last_brk; diff --git a/configure b/configure index 99d8bece70..b9a30cfd7b 100755 --- a/configure +++ b/configure @@ -553,12 +553,6 @@ fi HOST_VARIANT_DIR="" case $targetos in -CYGWIN*) - mingw32="yes" - QEMU_CFLAGS="-mno-cygwin $QEMU_CFLAGS" - audio_possible_drivers="sdl" - audio_drv_list="sdl" -;; MINGW32*) mingw32="yes" hax="yes" From b3d3a426da9b067366bc132d53a6fa6b72675b55 Mon Sep 17 00:00:00 2001 From: Vincent Palatin Date: Mon, 20 Mar 2017 11:15:49 +0100 Subject: [PATCH 4/4] hax: fix breakage in locking use qemu_mutex_lock_iothread consistently in qemu_hax_cpu_thread_fn() as done in other _thread_fn functions, instead of grabbing directly the BQL. This way we ensure that iothread_locked is properly set. On v2.9.0-rc0, QEMU was dying in an assertion in the mutex code when running with '--enable-hax' either on OSX or Windows. This bug was triggered since the code modification for multithreading added new usages of qemu_mutex_iothread_locked. This fixes the breakage on both platforms, I can now run again a full Chromium OS image with HAX kernel acceleration. Signed-off-by: Vincent Palatin Message-Id: <20170320101549.150076-1-vpalatin@chromium.org> Signed-off-by: Paolo Bonzini --- cpus.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cpus.c b/cpus.c index b84a392dda..167d9615e1 100644 --- a/cpus.c +++ b/cpus.c @@ -1344,8 +1344,9 @@ static void *qemu_hax_cpu_thread_fn(void *arg) { CPUState *cpu = arg; int r; + + qemu_mutex_lock_iothread(); qemu_thread_get_self(cpu->thread); - qemu_mutex_lock(&qemu_global_mutex); cpu->thread_id = qemu_get_thread_id(); cpu->created = true;