From a5ebc0ebae1d2283cfec4170b1a04317f2faee74 Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Fri, 26 Sep 2014 16:46:02 -0300 Subject: [PATCH 01/23] tests: Add missing include to test-bitops.c The test code needs osdep.h for the ARRAY_SIZE macro. Signed-off-by: Eduardo Habkost Signed-off-by: Michael Tokarev --- tests/test-bitops.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test-bitops.c b/tests/test-bitops.c index 8238eb5f6b..47b5d3ed9a 100644 --- a/tests/test-bitops.c +++ b/tests/test-bitops.c @@ -8,6 +8,7 @@ #include #include +#include "qemu/osdep.h" #include "qemu/bitops.h" typedef struct { From afeeead1bcd1080a621500240ea74ec433b21885 Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Fri, 26 Sep 2014 16:46:03 -0300 Subject: [PATCH 02/23] bitops.h: Don't include qemu-common.h This removes the following circular dependency: bitops.h -> qemu-common.h -> target-i386/cpu.h -> target-i386/cpu-qom.h -> qom/cpu.h -> qdev-core.h -> bitmap.h -> bitops.h. Signed-off-by: Eduardo Habkost Signed-off-by: Michael Tokarev --- include/qemu/bitops.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/include/qemu/bitops.h b/include/qemu/bitops.h index 7e2d5c996e..181bd46063 100644 --- a/include/qemu/bitops.h +++ b/include/qemu/bitops.h @@ -12,7 +12,9 @@ #ifndef BITOPS_H #define BITOPS_H -#include "qemu-common.h" +#include +#include + #include "host-utils.h" #define BITS_PER_BYTE CHAR_BIT From d6aaddfe977417e3df4fc8a0fc0b4cc015909b84 Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Fri, 26 Sep 2014 16:46:04 -0300 Subject: [PATCH 03/23] bitmap.h: Don't include qemu-common.h This will avoid unexpected circular header dependencies in the future. Signed-off-by: Eduardo Habkost Signed-off-by: Michael Tokarev --- include/qemu/bitmap.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/include/qemu/bitmap.h b/include/qemu/bitmap.h index edf4f17d9c..f0273c965f 100644 --- a/include/qemu/bitmap.h +++ b/include/qemu/bitmap.h @@ -12,7 +12,11 @@ #ifndef BITMAP_H #define BITMAP_H -#include "qemu-common.h" +#include +#include +#include + +#include "qemu/osdep.h" #include "qemu/bitops.h" /* From 11693a6cf0c3fac2e0ad64f06978095f85f0ec8a Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Sun, 14 Sep 2014 20:36:33 +0100 Subject: [PATCH 04/23] target-xtensa: mark XtensaConfig structs as unused The XtensaConfig structs will be defined but not used if they are for the opposite endianness from that of the binary being built; keep the compiler from complaining about this by marking them with the 'unused' attribute. Signed-off-by: Peter Maydell Acked-by: Max Filippov Signed-off-by: Michael Tokarev --- target-xtensa/core-dc232b.c | 2 +- target-xtensa/core-dc233c.c | 2 +- target-xtensa/core-fsf.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/target-xtensa/core-dc232b.c b/target-xtensa/core-dc232b.c index c51e11e6d7..a3b914bad4 100644 --- a/target-xtensa/core-dc232b.c +++ b/target-xtensa/core-dc232b.c @@ -33,7 +33,7 @@ #include "core-dc232b/core-isa.h" #include "overlay_tool.h" -static const XtensaConfig dc232b = { +static const XtensaConfig dc232b __attribute__((unused)) = { .name = "dc232b", .gdb_regmap = { .num_regs = 120, diff --git a/target-xtensa/core-dc233c.c b/target-xtensa/core-dc233c.c index 42dd64f031..ac745d106f 100644 --- a/target-xtensa/core-dc233c.c +++ b/target-xtensa/core-dc233c.c @@ -34,7 +34,7 @@ #include "core-dc233c/core-isa.h" #include "overlay_tool.h" -static const XtensaConfig dc233c = { +static const XtensaConfig dc233c __attribute__((unused)) = { .name = "dc233c", .gdb_regmap = { .num_regs = 121, diff --git a/target-xtensa/core-fsf.c b/target-xtensa/core-fsf.c index 6859bee062..cfcc840255 100644 --- a/target-xtensa/core-fsf.c +++ b/target-xtensa/core-fsf.c @@ -33,7 +33,7 @@ #include "core-fsf/core-isa.h" #include "overlay_tool.h" -static const XtensaConfig fsf = { +static const XtensaConfig fsf __attribute__((unused)) = { .name = "fsf", /* GDB for this core is not supported currently */ .clock_freq_khz = 10000, From 0d61f18b57432e56340b2fabe01222fc464fe92a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Wed, 15 Oct 2014 08:16:31 +0100 Subject: [PATCH 05/23] target-arm: A64: remove redundant store MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There is not much point storing the same value twice in a row. Reported-by: Laurent Desnogues Signed-off-by: Alex Bennée Reviewed-by: Laurent Desnogues Signed-off-by: Michael Tokarev --- target-arm/translate-a64.c | 1 - 1 file changed, 1 deletion(-) diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c index 3a3c48acf4..80d2c07e82 100644 --- a/target-arm/translate-a64.c +++ b/target-arm/translate-a64.c @@ -748,7 +748,6 @@ static void do_fp_st(DisasContext *s, int srcidx, TCGv_i64 tcg_addr, int size) } else { TCGv_i64 tcg_hiaddr = tcg_temp_new_i64(); tcg_gen_qemu_st_i64(tmp, tcg_addr, get_mem_index(s), MO_TEQ); - tcg_gen_qemu_st64(tmp, tcg_addr, get_mem_index(s)); tcg_gen_ld_i64(tmp, cpu_env, fp_reg_hi_offset(s, srcidx)); tcg_gen_addi_i64(tcg_hiaddr, tcg_addr, 8); tcg_gen_qemu_st_i64(tmp, tcg_hiaddr, get_mem_index(s), MO_TEQ); From 2944d742f73d78120151fbc62b53f2ab79be89aa Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Wed, 15 Oct 2014 11:51:09 +0200 Subject: [PATCH 06/23] sparse: fix build c++ compiler isn't wrapped with cgcc, resulting in gcc complaining about the sparse compiler flags which it doesn't know in case qemu is built with --enable-sparse. Signed-off-by: Gerd Hoffmann Signed-off-by: Michael Tokarev --- configure | 1 + 1 file changed, 1 insertion(+) diff --git a/configure b/configure index a9e4d49483..2f17bf3803 100755 --- a/configure +++ b/configure @@ -4908,6 +4908,7 @@ echo "QEMU_CFLAGS=$QEMU_CFLAGS" >> $config_host_mak echo "QEMU_INCLUDES=$QEMU_INCLUDES" >> $config_host_mak if test "$sparse" = "yes" ; then echo "CC := REAL_CC=\"\$(CC)\" cgcc" >> $config_host_mak + echo "CXX := REAL_CC=\"\$(CXX)\" cgcc" >> $config_host_mak echo "HOST_CC := REAL_CC=\"\$(HOST_CC)\" cgcc" >> $config_host_mak echo "QEMU_CFLAGS += -Wbitwise -Wno-transparent-union -Wno-old-initializer -Wno-non-pointer-null" >> $config_host_mak fi From 404ac83efd5761c6b590fd9f00fbbe8ee2281aa6 Mon Sep 17 00:00:00 2001 From: Michal Privoznik Date: Thu, 16 Oct 2014 15:13:32 +0200 Subject: [PATCH 07/23] util: Improve os_mem_prealloc error message Currently, when the preallocating guest memory process fails, a not so helpful error message is printed out: # virsh start migt10 error: Failed to start domain migt10 error: internal error: process exited while connecting to monitor: os_mem_prealloc: failed to preallocate pages From the error message it's not clear at the first glance where the problem lies. However, changing the error message might give users a clue. Signed-off-by: Michal Privoznik Reviewed-by: Eric Blake Signed-off-by: Michael Tokarev --- util/oslib-posix.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/util/oslib-posix.c b/util/oslib-posix.c index 016a047cfc..8c9d80e9fe 100644 --- a/util/oslib-posix.c +++ b/util/oslib-posix.c @@ -390,7 +390,8 @@ void os_mem_prealloc(int fd, char *area, size_t memory) pthread_sigmask(SIG_UNBLOCK, &set, &oldset); if (sigsetjmp(sigjump, 1)) { - fprintf(stderr, "os_mem_prealloc: failed to preallocate pages\n"); + fprintf(stderr, "os_mem_prealloc: Insufficient free host memory " + "pages available to allocate guest RAM\n"); exit(1); } else { int i; From 2fd7ae36c5403b14e9c676627a1a0dd2b76b9e1b Mon Sep 17 00:00:00 2001 From: Michael Tokarev Date: Fri, 24 Oct 2014 11:33:00 +0400 Subject: [PATCH 08/23] Revert "os-posix: report error message when lock file failed" This reverts commit e5048d15ce6addae869f23514b2a1f0d4466418a. qemu_create_pidfile() is only created from main(), and there, if that function returns failure, os_pidfile_error() function is called, to, guess that, report error (which is done differently whenever we're daemonizing or not). qemu_create_pidfile() function has several error returns, this lockf() failure is one of them, there are others (another shown in the patch context too). So this patch makes whole thing inconsistent at least. If we need to show error message when we're daemonizing, it looks like we should modify os_pidfile_error() routine to always report error and only after that check for daemon mode. This way all errors will be reported the same way. Signed-off-by: Michael Tokarev --- os-posix.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/os-posix.c b/os-posix.c index 4898ebf4a2..e31a099a2b 100644 --- a/os-posix.c +++ b/os-posix.c @@ -319,8 +319,6 @@ int qemu_create_pidfile(const char *filename) return -1; } if (lockf(fd, F_TLOCK, 0) == -1) { - fprintf(stderr, "lock file '%s' failed: %s\n", - filename, strerror(errno)); close(fd); return -1; } From 44d8d2b2ddbaa3ee4125ec6cb2e8691e6348b01e Mon Sep 17 00:00:00 2001 From: Michael Tokarev Date: Sat, 25 Oct 2014 00:29:50 +0400 Subject: [PATCH 09/23] net/slirp: specify logbase for smbd It looks like smbd always logs to /var/log/samba/log.$progname even if config file specifies different logfile -- when it needs to log something before completing reading the config file. But if it can't open it for writing, it fails and exits. Tell smbd to use our temp dir as logbase (-l option) to avoid that. The same option is used by samba3 and samba4, so there should be no incompatible changes. Signed-off-by: Michael Tokarev Reviewed-by: Jan Kiszka Tested-by: Jan Kiszka --- net/slirp.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/net/slirp.c b/net/slirp.c index c171119dad..920af30bda 100644 --- a/net/slirp.c +++ b/net/slirp.c @@ -549,8 +549,8 @@ static int slirp_smb(SlirpState* s, const char *exported_dir, ); fclose(f); - snprintf(smb_cmdline, sizeof(smb_cmdline), "%s -s %s", - CONFIG_SMBD_COMMAND, smb_conf); + snprintf(smb_cmdline, sizeof(smb_cmdline), "%s -l %s -s %s", + CONFIG_SMBD_COMMAND, s->smb_dir, smb_conf); if (slirp_add_exec(s->slirp, 0, smb_cmdline, &vserver_addr, 139) < 0 || slirp_add_exec(s->slirp, 0, smb_cmdline, &vserver_addr, 445) < 0) { From 8ef2b256b94696a3a4bd8aa69a2b0fd7bc246f07 Mon Sep 17 00:00:00 2001 From: zhanghailiang Date: Thu, 30 Oct 2014 10:03:28 +0800 Subject: [PATCH 10/23] target-tricore: check return value before using it We reference the return value of cpu before checking whether it is NULL, The checking code is after that which violates code style. It makes no difference if the cpu is NULL, qemu process will terminate. But one will be 'Segmentation fault' and the other will report a error which is what we want. Signed-off-by: zhanghailiang Reviewed-by: Bastian Koppelmann Signed-off-by: Michael Tokarev --- hw/tricore/tricore_testboard.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/tricore/tricore_testboard.c b/hw/tricore/tricore_testboard.c index eeb4922c4c..a059a20a30 100644 --- a/hw/tricore/tricore_testboard.c +++ b/hw/tricore/tricore_testboard.c @@ -71,11 +71,11 @@ static void tricore_testboard_init(MachineState *machine, int board_id) machine->cpu_model = "tc1796"; } cpu = cpu_tricore_init(machine->cpu_model); - env = &cpu->env; if (!cpu) { error_report("Unable to find CPU definition"); exit(1); } + env = &cpu->env; memory_region_init_ram(ext_cram, NULL, "powerlink_ext_c.ram", 2*1024*1024, &error_abort); vmstate_register_ram_global(ext_cram); memory_region_init_ram(ext_dram, NULL, "powerlink_ext_d.ram", 4*1024*1024, &error_abort); From 660edd4eda903e32811a4929d1434cceda3284aa Mon Sep 17 00:00:00 2001 From: Michael Tokarev Date: Thu, 30 Oct 2014 10:53:02 +0300 Subject: [PATCH 11/23] virtio-9p-proxy: Fix sockfd leak If connect() in connect_namedsocket() return false, the sockfd will leak. Plug it. Signed-off-by: Michael Tokarev Signed-off-by: Gonglei --- hw/9pfs/virtio-9p-proxy.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/9pfs/virtio-9p-proxy.c b/hw/9pfs/virtio-9p-proxy.c index b57966d9d8..e6bbb06319 100644 --- a/hw/9pfs/virtio-9p-proxy.c +++ b/hw/9pfs/virtio-9p-proxy.c @@ -1112,6 +1112,7 @@ static int connect_namedsocket(const char *path) size = strlen(helper.sun_path) + sizeof(helper.sun_family); if (connect(sockfd, (struct sockaddr *)&helper, size) < 0) { fprintf(stderr, "socket error\n"); + close(sockfd); return -1; } From 6af76c6f7dbcc6255f5a5cac20ad61d1492d4052 Mon Sep 17 00:00:00 2001 From: Michael Tokarev Date: Thu, 30 Oct 2014 11:00:01 +0300 Subject: [PATCH 12/23] virtio-9p-proxy: fix error return in proxy_init() proxy_init() does not check the return value of connect_namedsocket(), fix this by rearranging code a little bit. Signed-off-by: Michael Tokarev --- hw/9pfs/virtio-9p-proxy.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/hw/9pfs/virtio-9p-proxy.c b/hw/9pfs/virtio-9p-proxy.c index e6bbb06319..2ec211bbea 100644 --- a/hw/9pfs/virtio-9p-proxy.c +++ b/hw/9pfs/virtio-9p-proxy.c @@ -1155,10 +1155,12 @@ static int proxy_init(FsContext *ctx) sock_id = atoi(ctx->fs_root); if (sock_id < 0) { fprintf(stderr, "socket descriptor not initialized\n"); - g_free(proxy); - return -1; } } + if (sock_id < 0) { + g_free(proxy); + return -1; + } g_free(ctx->fs_root); ctx->fs_root = NULL; From 7d5a8435baed70f03bbe791aba12f5586ba17d75 Mon Sep 17 00:00:00 2001 From: Michael Tokarev Date: Thu, 30 Oct 2014 11:05:13 +0300 Subject: [PATCH 13/23] virtio-9p-proxy: improve error messages in connect_namedsocket() Signed-off-by: Michael Tokarev Reviewed-by: Gonglei --- hw/9pfs/virtio-9p-proxy.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/9pfs/virtio-9p-proxy.c b/hw/9pfs/virtio-9p-proxy.c index 2ec211bbea..59c7445dea 100644 --- a/hw/9pfs/virtio-9p-proxy.c +++ b/hw/9pfs/virtio-9p-proxy.c @@ -1104,14 +1104,14 @@ static int connect_namedsocket(const char *path) sockfd = socket(AF_UNIX, SOCK_STREAM, 0); if (sockfd < 0) { - fprintf(stderr, "socket %s\n", strerror(errno)); + fprintf(stderr, "failed to create socket: %s\n", strerror(errno)); return -1; } strcpy(helper.sun_path, path); helper.sun_family = AF_UNIX; size = strlen(helper.sun_path) + sizeof(helper.sun_family); if (connect(sockfd, (struct sockaddr *)&helper, size) < 0) { - fprintf(stderr, "socket error\n"); + fprintf(stderr, "failed to connect to %s: %s\n", path, strerror(errno)); close(sockfd); return -1; } From 08a655be71d0a130a5d9bf7816d096ec31c4f055 Mon Sep 17 00:00:00 2001 From: Gonglei Date: Thu, 30 Oct 2014 14:01:17 +0800 Subject: [PATCH 14/23] dump: Fix dump-guest-memory termination and use-after-close dump_iterate() dumps blocks in a loop. Eventually, get_next_block() returns "no more". We then call dump_completed(). But we neglect to break the loop! Broken in commit 4c7e251a. Because of that, we dump the last block again. This attempts to write to s->fd, which fails if we're lucky. The error makes dump_iterate() return failure. It's the only way it can ever return. Theoretical: if we're not so lucky, something else has opened something for writing and got the same fd. dump_iterate() then keeps looping, messing up the something else's output, until a write fails, or the process mercifully terminates. The obvious fix is to restore the return lost in commit 4c7e251a. But the root cause of the bug is needlessly opaque loop control. Replace it by a clean do ... while loop. This makes the badly chosen return values of get_next_block() more visible. Cleaning that up is outside the scope of this bug fix. Signed-off-by: Gonglei Signed-off-by: Markus Armbruster Signed-off-by: Michael Tokarev --- dump.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/dump.c b/dump.c index 06a49155a2..9c7dad8f86 100644 --- a/dump.c +++ b/dump.c @@ -604,10 +604,9 @@ static void dump_iterate(DumpState *s, Error **errp) { GuestPhysBlock *block; int64_t size; - int ret; Error *local_err = NULL; - while (1) { + do { block = s->next_block; size = block->target_end - block->target_start; @@ -623,11 +622,9 @@ static void dump_iterate(DumpState *s, Error **errp) return; } - ret = get_next_block(s, block); - if (ret == 1) { - dump_completed(s); - } - } + } while (!get_next_block(s, block)); + + dump_completed(s); } static void create_vmcore(DumpState *s, Error **errp) From 0be5e436ff2cd6b8a1ff6782e3cfd09441ff3ec7 Mon Sep 17 00:00:00 2001 From: Michael Tokarev Date: Thu, 30 Oct 2014 17:30:51 +0300 Subject: [PATCH 15/23] os-posix: use global daemon_pipe instead of cryptic fds[1] When asked to -daemonize, we fork a child and setup a pipe between it and parent to pass exit status. os-posix.c used global fds[2] array for that, but actually only the writing side of the pipe is needed to be global, and this name is really too generic. Use just one interger for the writing side of the pipe, and name it daemon_pipe to be more understandable than cryptic fds[1]. Signed-off-by: Michael Tokarev Reviewed-by: Gonglei --- os-posix.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/os-posix.c b/os-posix.c index e31a099a2b..d687896f91 100644 --- a/os-posix.c +++ b/os-posix.c @@ -47,7 +47,7 @@ static struct passwd *user_pwd; static const char *chroot_dir; static int daemonize; -static int fds[2]; +static int daemon_pipe; void os_setup_early_signal_handling(void) { @@ -205,6 +205,7 @@ void os_daemonize(void) { if (daemonize) { pid_t pid; + int fds[2]; if (pipe(fds) == -1) { exit(1); @@ -236,7 +237,8 @@ void os_daemonize(void) } close(fds[0]); - qemu_set_cloexec(fds[1]); + daemon_pipe = fds[1]; + qemu_set_cloexec(daemon_pipe); setsid(); @@ -263,7 +265,7 @@ void os_setup_post(void) ssize_t len; again1: - len = write(fds[1], &status, 1); + len = write(daemon_pipe, &status, 1); if (len == -1 && (errno == EINTR)) { goto again1; } @@ -296,7 +298,7 @@ void os_pidfile_error(void) { if (daemonize) { uint8_t status = 1; - if (write(fds[1], &status, 1) != 1) { + if (write(daemon_pipe, &status, 1) != 1) { perror("daemonize. Writing to pipe\n"); } } else From ccea25f1c7cd3f0b12d878a5294620f5478729f8 Mon Sep 17 00:00:00 2001 From: Michael Tokarev Date: Thu, 30 Oct 2014 17:37:16 +0300 Subject: [PATCH 16/23] os-posix: replace goto again with a proper loop Eliminiate two fullwrite implementations with goto replacing them with a proper do..while loop. Signed-off-by: Michael Tokarev Reviewed-by: Gonglei --- os-posix.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/os-posix.c b/os-posix.c index d687896f91..eada8d4685 100644 --- a/os-posix.c +++ b/os-posix.c @@ -218,11 +218,9 @@ void os_daemonize(void) close(fds[1]); - again: - len = read(fds[0], &status, 1); - if (len == -1 && (errno == EINTR)) { - goto again; - } + do { + len = read(fds[0], &status, 1); + } while (len < 0 && errno == EINTR); if (len != 1) { exit(1); } @@ -264,11 +262,9 @@ void os_setup_post(void) uint8_t status = 0; ssize_t len; - again1: - len = write(daemon_pipe, &status, 1); - if (len == -1 && (errno == EINTR)) { - goto again1; - } + do { + len = write(daemon_pipe, &status, 1); + } while (len < 0 && errno == EINTR); if (len != 1) { exit(1); } From fee78fd6d2f8dfdfd447a33c34323dd5bd3193a2 Mon Sep 17 00:00:00 2001 From: Michael Tokarev Date: Thu, 30 Oct 2014 17:40:48 +0300 Subject: [PATCH 17/23] pidfile: stop making pidfile error a special case In case of -daemonize, we write non-zero to the daemon pipe only if pidfile creation failed, so the parent will report error about pidfile problem. There's no need to make special case for this, since all other errors are reported by the child just fine. Let the parent report error and simplify logic in os_daemonize(). This way, we don't need os_pidfile_error() function, since it only prints error now, so put the error reporting printf into the only place where qemu_create_pidfile() is called, in vl.c. While at it, fix wrong indentation in os_daemonize(). Signed-off-by: Michael Tokarev --- include/qemu-common.h | 1 - os-posix.c | 31 ++++++++----------------------- os-win32.c | 5 ----- vl.c | 2 +- 4 files changed, 9 insertions(+), 30 deletions(-) diff --git a/include/qemu-common.h b/include/qemu-common.h index b87e9c2d8b..f8622141a8 100644 --- a/include/qemu-common.h +++ b/include/qemu-common.h @@ -357,7 +357,6 @@ char *qemu_find_file(int type, const char *name); void os_setup_early_signal_handling(void); char *os_find_datadir(void); void os_parse_cmd_args(int index, const char *optarg); -void os_pidfile_error(void); /* Convert a byte between binary and BCD. */ static inline uint8_t to_bcd(uint8_t val) diff --git a/os-posix.c b/os-posix.c index eada8d4685..52e989797d 100644 --- a/os-posix.c +++ b/os-posix.c @@ -221,18 +221,14 @@ void os_daemonize(void) do { len = read(fds[0], &status, 1); } while (len < 0 && errno == EINTR); - if (len != 1) { - exit(1); - } - else if (status == 1) { - fprintf(stderr, "Could not acquire pidfile\n"); - exit(1); - } else { - exit(0); - } - } else if (pid < 0) { - exit(1); - } + + /* only exit successfully if our child actually wrote + * a one-byte zero to our pipe, upon successful init */ + exit(len == 1 && status == 0 ? 0 : 1); + + } else if (pid < 0) { + exit(1); + } close(fds[0]); daemon_pipe = fds[1]; @@ -290,17 +286,6 @@ void os_setup_post(void) } } -void os_pidfile_error(void) -{ - if (daemonize) { - uint8_t status = 1; - if (write(daemon_pipe, &status, 1) != 1) { - perror("daemonize. Writing to pipe\n"); - } - } else - fprintf(stderr, "Could not acquire pid file: %s\n", strerror(errno)); -} - void os_set_line_buffering(void) { setvbuf(stdout, NULL, _IOLBF, 0); diff --git a/os-win32.c b/os-win32.c index 5f95caac15..c0daf8e189 100644 --- a/os-win32.c +++ b/os-win32.c @@ -104,11 +104,6 @@ void os_parse_cmd_args(int index, const char *optarg) return; } -void os_pidfile_error(void) -{ - fprintf(stderr, "Could not acquire pid file: %s\n", strerror(errno)); -} - int qemu_create_pidfile(const char *filename) { char buffer[128]; diff --git a/vl.c b/vl.c index f6b3546942..150524c718 100644 --- a/vl.c +++ b/vl.c @@ -3999,7 +3999,7 @@ int main(int argc, char **argv, char **envp) #endif if (pid_file && qemu_create_pidfile(pid_file) != 0) { - os_pidfile_error(); + fprintf(stderr, "Could not acquire pid file: %s\n", strerror(errno)); exit(1); } From 25cec2b896a977565ca04e5c649aab8c6e48bda8 Mon Sep 17 00:00:00 2001 From: Michael Tokarev Date: Thu, 30 Oct 2014 17:47:46 +0300 Subject: [PATCH 18/23] os-posix: reorder parent notification for -daemonize Put "success" parent reporting in os_setup_post() to after all other initializers which may also fail, to the very end, so more possible failure cases are reported properly to the calling process. Signed-off-by: Michael Tokarev Reviewed-by: Gonglei --- os-posix.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/os-posix.c b/os-posix.c index 52e989797d..ba091f1530 100644 --- a/os-posix.c +++ b/os-posix.c @@ -255,15 +255,6 @@ void os_setup_post(void) int fd = 0; if (daemonize) { - uint8_t status = 0; - ssize_t len; - - do { - len = write(daemon_pipe, &status, 1); - } while (len < 0 && errno == EINTR); - if (len != 1) { - exit(1); - } if (chdir("/")) { perror("not able to chdir to /"); exit(1); @@ -278,11 +269,21 @@ void os_setup_post(void) change_process_uid(); if (daemonize) { + uint8_t status = 0; + ssize_t len; + dup2(fd, 0); dup2(fd, 1); dup2(fd, 2); close(fd); + + do { + len = write(daemon_pipe, &status, 1); + } while (len < 0 && errno == EINTR); + if (len != 1) { + exit(1); + } } } From b391567b64c86d3c032b500eebb7c53d332fc5f2 Mon Sep 17 00:00:00 2001 From: Gonglei Date: Fri, 31 Oct 2014 10:53:30 +0800 Subject: [PATCH 19/23] tap_int.h: remove repeating NETWORK_SCRIPT defines DEFAULT_NETWORK_SCRIPT and DEFAULT_NETWORK_DOWN_SCRIPT have been defined in net/net.h included in tap.c, which is the only C file that using those two macro. Let's remove the repeating macroinstruction. Signed-off-by: Gonglei Signed-off-by: Michael Tokarev --- net/tap_int.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/net/tap_int.h b/net/tap_int.h index 86bb224bc8..79afdf2d58 100644 --- a/net/tap_int.h +++ b/net/tap_int.h @@ -29,9 +29,6 @@ #include "qemu-common.h" #include "qapi-types.h" -#define DEFAULT_NETWORK_SCRIPT "/etc/qemu-ifup" -#define DEFAULT_NETWORK_DOWN_SCRIPT "/etc/qemu-ifdown" - int tap_open(char *ifname, int ifname_size, int *vnet_hdr, int vnet_hdr_required, int mq_required); From bb019cf911ee4a152e3b31940d702d7b8a8b8114 Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Thu, 30 Oct 2014 17:12:33 -0200 Subject: [PATCH 20/23] target-i386: Remove unused model_features_t struct The struct is not used anymore and can be removed. Signed-off-by: Eduardo Habkost Reviewed-by: Igor Mammedov Signed-off-by: Michael Tokarev --- target-i386/cpu.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index e1946016ad..e4ccee133a 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -442,14 +442,6 @@ const char *get_register_name_32(unsigned int reg) return x86_reg_info_32[reg].name; } -/* collects per-function cpuid data - */ -typedef struct model_features_t { - uint32_t *guest_feat; - uint32_t *host_feat; - FeatureWord feat_word; -} model_features_t; - /* KVM-specific features that are automatically added to all CPU models * when KVM is enabled. */ From d0caa3eb539de01043354440ce0a10b1f546cf06 Mon Sep 17 00:00:00 2001 From: Gonglei Date: Fri, 31 Oct 2014 14:11:00 +0800 Subject: [PATCH 21/23] tap: do not close(fd) in net_init_tap_one commit 5193e5fb (tap: factor out common tap initialization) introduce net_init_tap_one(). But it's inappropriate that we close fd in net_init_tap_one(), we should lay it in the caller, becuase some callers needn't to close it if we get the fd by monitor_handle_fd_param(). On the other hand, in other exceptional branches fd isn't closed, so that's incomplete anyway. Signed-off-by: Gonglei Signed-off-by: Michael Tokarev --- net/tap.c | 1 - 1 file changed, 1 deletion(-) diff --git a/net/tap.c b/net/tap.c index a40f7f023f..7bcd4c73ea 100644 --- a/net/tap.c +++ b/net/tap.c @@ -598,7 +598,6 @@ static int net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer, s = net_tap_fd_init(peer, model, name, fd, vnet_hdr); if (!s) { - close(fd); return -1; } From 84f8f3dace8c6e88b1afa828f9abe1e9a57fac1a Mon Sep 17 00:00:00 2001 From: Gonglei Date: Sun, 2 Nov 2014 13:37:17 +0800 Subject: [PATCH 22/23] tap: fix possible fd leak in net_init_tap In hotplugging scenario, taking those true branch, the file handler do not be closed. Let's close them before return. Signed-off-by: Gonglei Signed-off-by: Michael Tokarev --- net/tap.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/net/tap.c b/net/tap.c index 7bcd4c73ea..bde6b58b17 100644 --- a/net/tap.c +++ b/net/tap.c @@ -796,6 +796,7 @@ int net_init_tap(const NetClientOptions *opts, const char *name, if (net_init_tap_one(tap, peer, "bridge", name, ifname, script, downscript, vhostfdname, vnet_hdr, fd)) { + close(fd); return -1; } } else { @@ -823,6 +824,7 @@ int net_init_tap(const NetClientOptions *opts, const char *name, if (queues > 1 && i == 0 && !tap->has_ifname) { if (tap_fd_get_ifname(fd, ifname)) { error_report("Fail to get ifname"); + close(fd); return -1; } } @@ -831,6 +833,7 @@ int net_init_tap(const NetClientOptions *opts, const char *name, i >= 1 ? "no" : script, i >= 1 ? "no" : downscript, vhostfdname, vnet_hdr, fd)) { + close(fd); return -1; } } From f18a768efb990d9cd02911159de02e8ff5475239 Mon Sep 17 00:00:00 2001 From: SeokYeon Hwang Date: Fri, 31 Oct 2014 17:02:05 +0900 Subject: [PATCH 23/23] vdi: wrapped uuid_unparse() in #ifdef Wrapped uuid_unparse() in #ifdef to avoid "-Wunused-function" on clang 3.4 or later. Signed-off-by: SeokYeon Hwang Reviewed-by: Stefan Weil Signed-off-by: Michael Tokarev --- block/vdi.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/block/vdi.c b/block/vdi.c index 19701ee00d..e1d211c9f7 100644 --- a/block/vdi.c +++ b/block/vdi.c @@ -137,12 +137,14 @@ static inline int uuid_is_null(const uuid_t uu) return memcmp(uu, null_uuid, sizeof(uuid_t)) == 0; } +# if defined(CONFIG_VDI_DEBUG) static inline void uuid_unparse(const uuid_t uu, char *out) { snprintf(out, 37, UUID_FMT, uu[0], uu[1], uu[2], uu[3], uu[4], uu[5], uu[6], uu[7], uu[8], uu[9], uu[10], uu[11], uu[12], uu[13], uu[14], uu[15]); } +# endif #endif typedef struct {