[Qemu-devel] [PULL 00/23] Trivial patches for 2014-11-02

[Qemu-devel] [PULL 00/23] Trivial patches for 2014-11-02

[Michael Tokarev]

Here's an almost 3-week trivial patches pull request.
Lots of small things here and there, many small bugfixes
too.  This is also a first pull request for which a
signature should finally be known to others.

Please consider applying/pulling.

/mjt

The following changes since commit ee29498e4f0f3eff90eeeb7f5fa1703abedd2fb6:

  Merge remote-tracking branch 'remotes/sstabellini/xen-2014-10-30' into staging (2014-10-30 20:28:09 +0000)

are available in the git repository at:

  git://git.corpit.ru/qemu.git tags/pull-trivial-patches-2014-11-02

for you to fetch changes up to f18a768efb990d9cd02911159de02e8ff5475239:

  vdi: wrapped uuid_unparse() in #ifdef (2014-11-02 10:05:35 +0300)

----------------------------------------------------------------
trivial patches for 2014-11-02

----------------------------------------------------------------
Alex Bennée (1):
      target-arm: A64: remove redundant store

Eduardo Habkost (4):
      tests: Add missing include to test-bitops.c
      bitops.h: Don't include qemu-common.h
      bitmap.h: Don't include qemu-common.h
      target-i386: Remove unused model_features_t struct

Gerd Hoffmann (1):
      sparse: fix build

Gonglei (4):
      dump: Fix dump-guest-memory termination and use-after-close
      tap_int.h: remove repeating NETWORK_SCRIPT defines
      tap: do not close(fd) in net_init_tap_one
      tap: fix possible fd leak in net_init_tap

Michael Tokarev (9):
      Revert "os-posix: report error message when lock file failed"
      net/slirp: specify logbase for smbd
      virtio-9p-proxy: Fix sockfd leak
      virtio-9p-proxy: fix error return in proxy_init()
      virtio-9p-proxy: improve error messages in connect_namedsocket()
      os-posix: use global daemon_pipe instead of cryptic fds[1]
      os-posix: replace goto again with a proper loop
      pidfile: stop making pidfile error a special case
      os-posix: reorder parent notification for -daemonize

Michal Privoznik (1):
      util: Improve os_mem_prealloc error message

Peter Maydell (1):
      target-xtensa: mark XtensaConfig structs as unused

SeokYeon Hwang (1):
      vdi: wrapped uuid_unparse() in #ifdef

zhanghailiang (1):
      target-tricore: check return value before using it

 block/vdi.c                    |    2 ++
 configure                      |    1 +
 dump.c                         |   11 +++----
 hw/9pfs/virtio-9p-proxy.c      |   11 ++++---
 hw/tricore/tricore_testboard.c |    2 +-
 include/qemu-common.h          |    1 -
 include/qemu/bitmap.h          |    6 +++-
 include/qemu/bitops.h          |    4 ++-
 net/slirp.c                    |    4 +--
 net/tap.c                      |    4 ++-
 net/tap_int.h                  |    3 --
 os-posix.c                     |   66 +++++++++++++++-------------------------
 os-win32.c                     |    5 ---
 target-arm/translate-a64.c     |    1 -
 target-i386/cpu.c              |    8 -----
 target-xtensa/core-dc232b.c    |    2 +-
 target-xtensa/core-dc233c.c    |    2 +-
 target-xtensa/core-fsf.c       |    2 +-
 tests/test-bitops.c            |    1 +
 util/oslib-posix.c             |    3 +-
 vl.c                           |    2 +-
 21 files changed, 59 insertions(+), 82 deletions(-)

[Qemu-devel] [PULL 01/23] tests: Add missing include to test-bitops.c

[Michael Tokarev]

From: Eduardo Habkost <ehabkost@redhat.com>

The test code needs osdep.h for the ARRAY_SIZE macro.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
---
 tests/test-bitops.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/test-bitops.c b/tests/test-bitops.c
index 8238eb5..47b5d3e 100644
--- a/tests/test-bitops.c
+++ b/tests/test-bitops.c
@@ -8,6 +8,7 @@
 
 #include <glib.h>
 #include <stdint.h>
+#include "qemu/osdep.h"
 #include "qemu/bitops.h"
 
 typedef struct {
-- 
1.7.10.4

[Qemu-devel] [PULL 02/23] bitops.h: Don't include qemu-common.h

[Michael Tokarev]

From: Eduardo Habkost <ehabkost@redhat.com>

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 <ehabkost@redhat.com>
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
---
 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 7e2d5c9..181bd46 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 <stdint.h>
+#include <assert.h>
+
 #include "host-utils.h"
 
 #define BITS_PER_BYTE           CHAR_BIT
-- 
1.7.10.4

[Qemu-devel] [PULL 03/23] bitmap.h: Don't include qemu-common.h

[Michael Tokarev]

From: Eduardo Habkost <ehabkost@redhat.com>

This will avoid unexpected circular header dependencies in the future.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
---
 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 edf4f17..f0273c9 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 <glib.h>
+#include <string.h>
+#include <stdlib.h>
+
+#include "qemu/osdep.h"
 #include "qemu/bitops.h"
 
 /*
-- 
1.7.10.4

[Qemu-devel] [PULL 04/23] target-xtensa: mark XtensaConfig structs as unused

[Michael Tokarev]

From: Peter Maydell <peter.maydell@linaro.org>

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 <peter.maydell@linaro.org>
Acked-by: Max Filippov <jcmvbkbc@gmail.com>
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
---
 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 c51e11e..a3b914b 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 42dd64f..ac745d1 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 6859bee..cfcc840 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,
-- 
1.7.10.4

[Qemu-devel] [PULL 05/23] target-arm: A64: remove redundant store

[Michael Tokarev]

From: Alex Bennée <alex.bennee@linaro.org>

There is not much point storing the same value twice in a row.

Reported-by: Laurent Desnogues <laurent.desnogues@gmail.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Laurent Desnogues <laurent.desnogues@gmail.com>
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
---
 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 3a3c48a..80d2c07 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);
-- 
1.7.10.4

[Qemu-devel] [PULL 06/23] sparse: fix build

[Michael Tokarev]

From: Gerd Hoffmann <kraxel@redhat.com>

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 <kraxel@redhat.com>
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
---
 configure |    1 +
 1 file changed, 1 insertion(+)

diff --git a/configure b/configure
index a9e4d49..2f17bf3 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
-- 
1.7.10.4

[Qemu-devel] [PULL 07/23] util: Improve os_mem_prealloc error message

[Michael Tokarev]

From: Michal Privoznik <mprivozn@redhat.com>

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 <mprivozn@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
---
 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 016a047..8c9d80e 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;
-- 
1.7.10.4

[Qemu-devel] [PULL 08/23] Revert "os-posix: report error message when lock file failed"

[Michael Tokarev]

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 <mjt@tls.msk.ru>
---
 os-posix.c |    2 --
 1 file changed, 2 deletions(-)

diff --git a/os-posix.c b/os-posix.c
index 4898ebf..e31a099 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;
     }
-- 
1.7.10.4

[Qemu-devel] [PULL 09/23] net/slirp: specify logbase for smbd

[Michael Tokarev]

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 <mjt@tls.msk.ru>
Reviewed-by: Jan Kiszka <jan.kiszka@siemens.com>
Tested-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 net/slirp.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/slirp.c b/net/slirp.c
index c171119..920af30 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) {
-- 
1.7.10.4

[Qemu-devel] [PULL 10/23] target-tricore: check return value before using it

[Michael Tokarev]

From: zhanghailiang <zhang.zhanghailiang@huawei.com>

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 <zhang.zhanghailiang@huawei.com>
Reviewed-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
---
 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 eeb4922..a059a20 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);
-- 
1.7.10.4

[Qemu-devel] [PULL 11/23] virtio-9p-proxy: Fix sockfd leak

[Michael Tokarev]

If connect() in connect_namedsocket() return false, the sockfd will leak.
Plug it.

Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 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 b57966d..e6bbb06 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;
     }
 
-- 
1.7.10.4

[Qemu-devel] [PULL 12/23] virtio-9p-proxy: fix error return in proxy_init()

[Michael Tokarev]

proxy_init() does not check the return value of connect_namedsocket(),
fix this by rearranging code a little bit.

Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
---
 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 e6bbb06..2ec211b 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;
 
-- 
1.7.10.4

[Qemu-devel] [PULL 13/23] virtio-9p-proxy: improve error messages in connect_namedsocket()

[Michael Tokarev]

Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
Reviewed-by: Gonglei <arei.gonglei@huawei.com>
---
 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 2ec211b..59c7445 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;
     }
-- 
1.7.10.4

[Qemu-devel] [PULL 14/23] dump: Fix dump-guest-memory termination and use-after-close

[Michael Tokarev]

From: Gonglei <arei.gonglei@huawei.com>

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 <arei.gonglei@huawei.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
---
 dump.c |   11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/dump.c b/dump.c
index 06a4915..9c7dad8 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)
-- 
1.7.10.4

[Qemu-devel] [PULL 15/23] os-posix: use global daemon_pipe instead of cryptic fds[1]

[Michael Tokarev]

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 <mjt@tls.msk.ru>
Reviewed-by: Gonglei <arei.gonglei@huawei.com>
---
 os-posix.c |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/os-posix.c b/os-posix.c
index e31a099..d687896 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
-- 
1.7.10.4

[Qemu-devel] [PULL 16/23] os-posix: replace goto again with a proper loop

[Michael Tokarev]

Eliminiate two fullwrite implementations with goto replacing them with
a proper do..while loop.

Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
Reviewed-by: Gonglei <arei.gonglei@huawei.com>
---
 os-posix.c |   16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/os-posix.c b/os-posix.c
index d687896..eada8d4 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);
         }
-- 
1.7.10.4

[Qemu-devel] [PULL 17/23] pidfile: stop making pidfile error a special case

[Michael Tokarev]

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 <mjt@tls.msk.ru>
---
 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 b87e9c2..f862214 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 eada8d4..52e9897 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 5f95caa..c0daf8e 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 f6b3546..150524c 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);
     }
 
-- 
1.7.10.4

[Qemu-devel] [PULL 18/23] os-posix: reorder parent notification for -daemonize

[Michael Tokarev]

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 <mjt@tls.msk.ru>
Reviewed-by: Gonglei <arei.gonglei@huawei.com>
---
 os-posix.c |   19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/os-posix.c b/os-posix.c
index 52e9897..ba091f1 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);
+        }
     }
 }
 
-- 
1.7.10.4

[Qemu-devel] [PULL 19/23] tap_int.h: remove repeating NETWORK_SCRIPT defines

[Michael Tokarev]

From: Gonglei <arei.gonglei@huawei.com>

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 <arei.gonglei@huawei.com>
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
---
 net/tap_int.h |    3 ---
 1 file changed, 3 deletions(-)

diff --git a/net/tap_int.h b/net/tap_int.h
index 86bb224..79afdf2 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);
 
-- 
1.7.10.4

[Qemu-devel] [PULL 20/23] target-i386: Remove unused model_features_t struct

[Michael Tokarev]

From: Eduardo Habkost <ehabkost@redhat.com>

The struct is not used anymore and can be removed.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
---
 target-i386/cpu.c |    8 --------
 1 file changed, 8 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index e194601..e4ccee1 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.
  */
-- 
1.7.10.4

[Qemu-devel] [PULL 21/23] tap: do not close(fd) in net_init_tap_one

[Michael Tokarev]

From: Gonglei <arei.gonglei@huawei.com>

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 <arei.gonglei@huawei.com>
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
---
 net/tap.c |    1 -
 1 file changed, 1 deletion(-)

diff --git a/net/tap.c b/net/tap.c
index a40f7f0..7bcd4c7 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;
     }
 
-- 
1.7.10.4

[Qemu-devel] [PULL 22/23] tap: fix possible fd leak in net_init_tap

[Michael Tokarev]

From: Gonglei <arei.gonglei@huawei.com>

In hotplugging scenario, taking those true branch, the file
handler do not be closed. Let's close them before return.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
---
 net/tap.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/tap.c b/net/tap.c
index 7bcd4c7..bde6b58 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;
             }
         }
-- 
1.7.10.4

[Qemu-devel] [PULL 23/23] vdi: wrapped uuid_unparse() in #ifdef

[Michael Tokarev]

From: SeokYeon Hwang <syeon.hwang@samsung.com>

Wrapped uuid_unparse() in #ifdef to avoid "-Wunused-function"
on clang 3.4 or later.

Signed-off-by: SeokYeon Hwang <syeon.hwang@samsung.com>
Reviewed-by: Stefan Weil <sw@weilnetz.de>
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
---
 block/vdi.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/block/vdi.c b/block/vdi.c
index 19701ee..e1d211c 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 {
-- 
1.7.10.4

Re: [Qemu-devel] [PULL 00/23] Trivial patches for 2014-11-02

[Peter Maydell]

On 2 November 2014 11:57, Michael Tokarev <mjt@tls.msk.ru> wrote:
> Here's an almost 3-week trivial patches pull request.
> Lots of small things here and there, many small bugfixes
> too.  This is also a first pull request for which a
> signature should finally be known to others.
>
> Please consider applying/pulling.
>
> /mjt
>
> The following changes since commit ee29498e4f0f3eff90eeeb7f5fa1703abedd2fb6:
>
>   Merge remote-tracking branch 'remotes/sstabellini/xen-2014-10-30' into staging (2014-10-30 20:28:09 +0000)
>
> are available in the git repository at:
>
>   git://git.corpit.ru/qemu.git tags/pull-trivial-patches-2014-11-02
>
> for you to fetch changes up to f18a768efb990d9cd02911159de02e8ff5475239:
>
>   vdi: wrapped uuid_unparse() in #ifdef (2014-11-02 10:05:35 +0300)
>
> ----------------------------------------------------------------
> trivial patches for 2014-11-02
>
> ----------------------------------------------------------------

Applied, thanks.

-- PMM