[Qemu-devel] [PULL 00/14] SCSI, chardev, build fixes for 2016-07-13

[Qemu-devel] [PULL 00/14] SCSI, chardev, build fixes for 2016-07-13

[Paolo Bonzini]

The following changes since commit a91a4e7d8cfe6ece610aacf7c52738188f5b5cb5:

  Merge remote-tracking branch 'remotes/ehabkost/tags/x86-pull-request' into staging (2016-07-11 15:08:47 +0100)

are available in the git repository at:


  git://github.com/bonzini/qemu.git tags/for-upstream

for you to fetch changes up to 2aece63c8a9d2c3a8ff41d2febc4cdeff2633331:

  hostmem: detect host backend memory is being used properly (2016-07-13 13:30:04 +0200)

----------------------------------------------------------------
* SCSI scanner support
* fixes to qemu-char and net exit
* FreeBSD fixes
* Other small bugfixes

----------------------------------------------------------------
Cao jin (1):
      main-loop: check return value before using pointer

Fam Zheng (1):
      util: Fix MIN_NON_ZERO

Jarkko Lavinen (2):
      scsi-bus: Add SCSI scanner support
      scsi-bus: Use longer sense buffer with scanners

Marc-André Lureau (3):
      qemu-sockets: use qapi_free_SocketAddress in cleanup
      tap: use an exit notifier to call down_script
      char: do not use atexit cleanup handler

Paolo Bonzini (4):
      json-streamer: fix double-free on exiting during a parse
      disas: avoid including everything in headers compiled from C++
      slirp: use exit notifier for slirp_smb_cleanup
      net: do not use atexit for cleanup

Sean Bruno (1):
      Use "-s" instead of "--quiet" to resolve non-fatal build error on FreeBSD.

Xiao Guangrong (2):
      hostmem: fix QEMU crash by 'info memdev'
      hostmem: detect host backend memory is being used properly

 Makefile                 |  2 +-
 backends/hostmem.c       | 37 +++++++++++++++++++++++++------------
 hw/mem/pc-dimm.c         | 18 +++++++++++-------
 hw/misc/ivshmem.c        | 14 ++++++++++----
 hw/scsi/scsi-bus.c       | 41 ++++++++++++++++++++++++++++++++++++++++-
 include/block/scsi.h     |  6 ++++++
 include/disas/bfd.h      |  2 +-
 include/hw/scsi/scsi.h   |  7 ++++---
 include/qemu/osdep.h     |  3 ++-
 include/sysemu/char.h    |  7 +++++++
 include/sysemu/hostmem.h |  4 +++-
 main-loop.c              |  2 +-
 net/slirp.c              | 11 +++++++++++
 net/tap.c                | 27 ++++++++++++++++++++-------
 qemu-char.c              |  4 +---
 qobject/json-streamer.c  |  8 ++++++--
 util/qemu-sockets.c      |  2 +-
 vl.c                     |  7 ++++---
 18 files changed, 154 insertions(+), 48 deletions(-)
-- 
1.8.3.1

[Qemu-devel] [PULL 01/14] scsi-bus: Add SCSI scanner support

[Paolo Bonzini]

From: Jarkko Lavinen <jarkko.lavinen@iki.fi>

Add support for missing scanner specific SCSI commands and their xfer
lenghts as per ANSI spec section 15.

Signed-off-by: Jarkko Lavinen <jarkko.lavinen@iki.fi>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi/scsi-bus.c   | 31 +++++++++++++++++++++++++++++++
 include/block/scsi.h |  6 ++++++
 2 files changed, 37 insertions(+)

diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index ad6f398..14c0aa5 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -1132,6 +1132,29 @@ static int scsi_req_medium_changer_xfer(SCSICommand *cmd, SCSIDevice *dev, uint8
     return 0;
 }
 
+static int scsi_req_scanner_length(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf)
+{
+    switch (buf[0]) {
+    /* Scanner commands */
+    case OBJECT_POSITION:
+        cmd->xfer = 0;
+        break;
+    case SCAN:
+        cmd->xfer = buf[4];
+        break;
+    case READ_10:
+    case SEND:
+    case GET_WINDOW:
+    case SET_WINDOW:
+        cmd->xfer = buf[8] | (buf[7] << 8) | (buf[6] << 16);
+        break;
+    default:
+        /* GET_DATA_BUFFER_STATUS xfer handled by scsi_req_xfer */
+        return scsi_req_xfer(cmd, dev, buf);
+    }
+
+    return 0;
+}
 
 static void scsi_cmd_xfer_mode(SCSICommand *cmd)
 {
@@ -1178,6 +1201,11 @@ static void scsi_cmd_xfer_mode(SCSICommand *cmd)
     case SEND_DVD_STRUCTURE:
     case PERSISTENT_RESERVE_OUT:
     case MAINTENANCE_OUT:
+    case SET_WINDOW:
+    case SCAN:
+        /* SCAN conflicts with START_STOP.  START_STOP has cmd->xfer set to 0 for
+         * non-scanner devices, so we only get here for SCAN and not for START_STOP.
+         */
         cmd->mode = SCSI_XFER_TO_DEV;
         break;
     case ATA_PASSTHROUGH_12:
@@ -1258,6 +1286,9 @@ int scsi_req_parse_cdb(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf)
     case TYPE_MEDIUM_CHANGER:
         rc = scsi_req_medium_changer_xfer(cmd, dev, buf);
         break;
+    case TYPE_SCANNER:
+        rc = scsi_req_scanner_length(cmd, dev, buf);
+        break;
     default:
         rc = scsi_req_xfer(cmd, dev, buf);
         break;
diff --git a/include/block/scsi.h b/include/block/scsi.h
index a311341..8b966d7 100644
--- a/include/block/scsi.h
+++ b/include/block/scsi.h
@@ -48,13 +48,17 @@
 #define ERASE                 0x19
 #define MODE_SENSE            0x1a
 #define LOAD_UNLOAD           0x1b
+#define SCAN                  0x1b
 #define START_STOP            0x1b
 #define RECEIVE_DIAGNOSTIC    0x1c
 #define SEND_DIAGNOSTIC       0x1d
 #define ALLOW_MEDIUM_REMOVAL  0x1e
+#define SET_WINDOW            0x24
 #define READ_CAPACITY_10      0x25
+#define GET_WINDOW            0x25
 #define READ_10               0x28
 #define WRITE_10              0x2a
+#define SEND                  0x2a
 #define SEEK_10               0x2b
 #define LOCATE_10             0x2b
 #define POSITION_TO_ELEMENT   0x2b
@@ -62,10 +66,12 @@
 #define VERIFY_10             0x2f
 #define SEARCH_HIGH           0x30
 #define SEARCH_EQUAL          0x31
+#define OBJECT_POSITION       0x31
 #define SEARCH_LOW            0x32
 #define SET_LIMITS            0x33
 #define PRE_FETCH             0x34
 #define READ_POSITION         0x34
+#define GET_DATA_BUFFER_STATUS 0x34
 #define SYNCHRONIZE_CACHE     0x35
 #define LOCK_UNLOCK_CACHE     0x36
 #define INITIALIZE_ELEMENT_STATUS_WITH_RANGE 0x37
-- 
1.8.3.1

[Qemu-devel] [PULL 02/14] scsi-bus: Use longer sense buffer with scanners

[Paolo Bonzini]

From: Jarkko Lavinen <jarkko.lavinen@iki.fi>

Scanners can provide additional sense bytes beyond 18 bytes.
VueScan uses 32 bytes alloc length with Request Sense command.

Signed-off-by: Jarkko Lavinen <jarkko.lavinen@iki.fi>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi/scsi-bus.c     | 10 +++++++++-
 include/hw/scsi/scsi.h |  7 ++++---
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index 14c0aa5..297216d 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -461,6 +461,14 @@ static bool scsi_target_emulate_inquiry(SCSITargetReq *r)
     return true;
 }
 
+static size_t scsi_sense_len(SCSIRequest *req)
+{
+    if (req->dev->type == TYPE_SCANNER)
+        return SCSI_SENSE_LEN_SCANNER;
+    else
+        return SCSI_SENSE_LEN;
+}
+
 static int32_t scsi_target_send_command(SCSIRequest *req, uint8_t *buf)
 {
     SCSITargetReq *r = DO_UPCAST(SCSITargetReq, req, req);
@@ -477,7 +485,7 @@ static int32_t scsi_target_send_command(SCSIRequest *req, uint8_t *buf)
         }
         break;
     case REQUEST_SENSE:
-        scsi_target_alloc_buf(&r->req, SCSI_SENSE_LEN);
+        scsi_target_alloc_buf(&r->req, scsi_sense_len(req));
         r->len = scsi_device_get_sense(r->req.dev, r->buf,
                                        MIN(req->cmd.xfer, r->buf_len),
                                        (req->cmd.buf[1] & 1) == 0);
diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index 8acd3fa..94d7868 100644
--- a/include/hw/scsi/scsi.h
+++ b/include/hw/scsi/scsi.h
@@ -8,9 +8,10 @@
 
 #define MAX_SCSI_DEVS	255
 
-#define SCSI_CMD_BUF_SIZE     16
-#define SCSI_SENSE_LEN      18
-#define SCSI_INQUIRY_LEN    36
+#define SCSI_CMD_BUF_SIZE      16
+#define SCSI_SENSE_LEN         18
+#define SCSI_SENSE_LEN_SCANNER 32
+#define SCSI_INQUIRY_LEN       36
 
 typedef struct SCSIBus SCSIBus;
 typedef struct SCSIBusInfo SCSIBusInfo;
-- 
1.8.3.1

[Qemu-devel] [PULL 03/14] Use "-s" instead of "--quiet" to resolve non-fatal build error on FreeBSD.

[Paolo Bonzini]

From: Sean Bruno <sbruno@freebsd.org>

The --quiet argument is not available on all operating systems.  Use -s
instead to match the rest of the Makefile uses.  This fixes a non-fatal
error seen on FreeBSD.

Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Sean Bruno <sbruno@freebsd.org>
Message-Id: <20160614180734.8782-1-sbruno@freebsd.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index c054bc6..4570637 100644
--- a/Makefile
+++ b/Makefile
@@ -185,7 +185,7 @@ qemu-version.h: FORCE
 				printf '""\n'; \
 			fi; \
 		fi) > $@.tmp)
-	$(call quiet-command, cmp --quiet $@ $@.tmp || mv $@.tmp $@)
+	$(call quiet-command, cmp -s $@ $@.tmp || mv $@.tmp $@)
 
 config-host.h: config-host.h-timestamp
 config-host.h-timestamp: config-host.mak
-- 
1.8.3.1

[Qemu-devel] [PULL 04/14] main-loop: check return value before using pointer

[Paolo Bonzini]

From: Cao jin <caoj.fnst@cn.fujitsu.com>

pointer 'qemu_aio_context' should be checked first before it is used.
qemu_bh_new() will use it.

Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
Message-Id: <1467799740-26079-2-git-send-email-caoj.fnst@cn.fujitsu.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 main-loop.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/main-loop.c b/main-loop.c
index 89a6994..6a7f8d3 100644
--- a/main-loop.c
+++ b/main-loop.c
@@ -154,11 +154,11 @@ int qemu_init_main_loop(Error **errp)
     }
 
     qemu_aio_context = aio_context_new(&local_error);
-    qemu_notify_bh = qemu_bh_new(notify_event_cb, NULL);
     if (!qemu_aio_context) {
         error_propagate(errp, local_error);
         return -EMFILE;
     }
+    qemu_notify_bh = qemu_bh_new(notify_event_cb, NULL);
     gpollfds = g_array_new(FALSE, FALSE, sizeof(GPollFD));
     src = aio_get_g_source(qemu_aio_context);
     g_source_attach(src, NULL);
-- 
1.8.3.1

[Qemu-devel] [PULL 05/14] json-streamer: fix double-free on exiting during a parse

[Paolo Bonzini]

Now that json-streamer tries not to leak tokens on incomplete parse,
the tokens can be freed twice if QEMU destroys the json-streamer
object during the parser->emit call.  To fix this, create the new
empty GQueue earlier, so that it is already in place when the old
one is passed to parser->emit.

Reported-by: Changlong Xie <xiecl.fnst@cn.fujitsu.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <1467636059-12557-1-git-send-email-pbonzini@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qobject/json-streamer.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/qobject/json-streamer.c b/qobject/json-streamer.c
index 7164390..c51c202 100644
--- a/qobject/json-streamer.c
+++ b/qobject/json-streamer.c
@@ -39,6 +39,7 @@ static void json_message_process_token(JSONLexer *lexer, GString *input,
 {
     JSONMessageParser *parser = container_of(lexer, JSONMessageParser, lexer);
     JSONToken *token;
+    GQueue *tokens;
 
     switch (type) {
     case JSON_LCURLY:
@@ -96,9 +97,12 @@ out_emit:
     /* send current list of tokens to parser and reset tokenizer */
     parser->brace_count = 0;
     parser->bracket_count = 0;
-    /* parser->emit takes ownership of parser->tokens.  */
-    parser->emit(parser, parser->tokens);
+    /* parser->emit takes ownership of parser->tokens.  Remove our own
+     * reference to parser->tokens before handing it out to parser->emit.
+     */
+    tokens = parser->tokens;
     parser->tokens = g_queue_new();
+    parser->emit(parser, tokens);
     parser->token_size = 0;
 }
 
-- 
1.8.3.1

[Qemu-devel] [PULL 06/14] disas: avoid including everything in headers compiled from C++

[Paolo Bonzini]

disas/arm-a64.cc is careful to include only the bare minimum that
it needs---qemu/osdep.h and disas/bfd.h.  Unfortunately, disas/bfd.h
then includes qemu-common.h, which brings in qemu/option.h and from
there we get the kitchen sink.

This causes problems because for example QEMU's atomic macros
conflict with C++ atomic types.  But really all that bfd.h needs
is the fprintf_function typedef, so replace the inclusion of
qemu-common.h with qemu/fprintf-fn.h.

Reported-by: Sean Bruno <sbruno@freebsd.org>
Tested-by: Sean Bruno <sbruno@freebsd.org>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/disas/bfd.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/disas/bfd.h b/include/disas/bfd.h
index a112e9c..a87b8a1 100644
--- a/include/disas/bfd.h
+++ b/include/disas/bfd.h
@@ -9,7 +9,7 @@
 #ifndef DIS_ASM_H
 #define DIS_ASM_H
 
-#include "qemu-common.h"
+#include "qemu/fprintf-fn.h"
 
 typedef void *PTR;
 typedef uint64_t bfd_vma;
-- 
1.8.3.1

[Qemu-devel] [PULL 07/14] qemu-sockets: use qapi_free_SocketAddress in cleanup

[Paolo Bonzini]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Commit 74b6ce43e3 uses the wrong free API for a SocketAddress, that
may leak some linked data.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20160706164246.22116-1-marcandre.lureau@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 util/qemu-sockets.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index fb83d48..777af49 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -1012,7 +1012,7 @@ void socket_listen_cleanup(int fd, Error **errp)
         }
     }
 
-    g_free(addr);
+    qapi_free_SocketAddress(addr);
 }
 
 int socket_dgram(SocketAddress *remote, SocketAddress *local, Error **errp)
-- 
1.8.3.1

[Qemu-devel] [PULL 08/14] util: Fix MIN_NON_ZERO

[Paolo Bonzini]

From: Fam Zheng <famz@redhat.com>

MIN_NON_ZERO(1, 0) is evaluated to 0. Rewrite the macro to fix it.

Reported-by: Miroslav Rezanina <mrezanin@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
Message-Id: <1468306113-847-1-git-send-email-famz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/qemu/osdep.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index e63da28..e4c6ae6 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -151,7 +151,8 @@ extern int daemon(int, int);
 /* Minimum function that returns zero only iff both values are zero.
  * Intended for use with unsigned values only. */
 #ifndef MIN_NON_ZERO
-#define MIN_NON_ZERO(a, b) (((a) != 0 && (a) < (b)) ? (a) : (b))
+#define MIN_NON_ZERO(a, b) ((a) == 0 ? (b) : \
+                                ((b) == 0 ? (a) : (MIN(a, b))))
 #endif
 
 /* Round number down to multiple */
-- 
1.8.3.1

[Qemu-devel] [PULL 09/14] tap: use an exit notifier to call down_script

[Paolo Bonzini]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

We would like to move back net_cleanup() at the end of main function,
like it used to be until f30dbae63a46f23116715dff8d130c, but minimum
tap cleanup is necessary regarless at exit() time. Use an exit notifier
to call TAP down_script. If net_cleanup() is called first, then remove
the exit notifier as it will become a dangling pointer otherwise.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20160711144847.16651-1-marcandre.lureau@redhat.com>
Reviewed-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 net/tap.c | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/net/tap.c b/net/tap.c
index 676bad4..e9c32f3 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -58,6 +58,7 @@ typedef struct TAPState {
     bool enabled;
     VHostNetState *vhost_net;
     unsigned host_vnet_hdr_len;
+    Notifier exit;
 } TAPState;
 
 static void launch_script(const char *setup_script, const char *ifname,
@@ -292,10 +293,22 @@ static void tap_set_offload(NetClientState *nc, int csum, int tso4,
     tap_fd_set_offload(s->fd, csum, tso4, tso6, ecn, ufo);
 }
 
+static void tap_exit_notify(Notifier *notifier, void *data)
+{
+    TAPState *s = container_of(notifier, TAPState, exit);
+    Error *err = NULL;
+
+    if (s->down_script[0]) {
+        launch_script(s->down_script, s->down_script_arg, s->fd, &err);
+        if (err) {
+            error_report_err(err);
+        }
+    }
+}
+
 static void tap_cleanup(NetClientState *nc)
 {
     TAPState *s = DO_UPCAST(TAPState, nc, nc);
-    Error *err = NULL;
 
     if (s->vhost_net) {
         vhost_net_cleanup(s->vhost_net);
@@ -304,12 +317,8 @@ static void tap_cleanup(NetClientState *nc)
 
     qemu_purge_queued_packets(nc);
 
-    if (s->down_script[0]) {
-        launch_script(s->down_script, s->down_script_arg, s->fd, &err);
-        if (err) {
-            error_report_err(err);
-        }
-    }
+    tap_exit_notify(&s->exit, NULL);
+    qemu_remove_exit_notifier(&s->exit);
 
     tap_read_poll(s, false);
     tap_write_poll(s, false);
@@ -379,6 +388,10 @@ static TAPState *net_tap_fd_init(NetClientState *peer,
     }
     tap_read_poll(s, true);
     s->vhost_net = NULL;
+
+    s->exit.notify = tap_exit_notify;
+    qemu_add_exit_notifier(&s->exit);
+
     return s;
 }
 
-- 
1.8.3.1

[Qemu-devel] [PULL 10/14] slirp: use exit notifier for slirp_smb_cleanup

[Paolo Bonzini]

We would like to move back net_cleanup() at the end of main function,
like it used to be until f30dbae63a46f23116715dff8d130c, but minimum
cleanup is needed regardless at exit() time for slirp's SMB
functionality.  Use an exit notifier to call slirp_smb_cleanup.
If net_cleanup() is called first, then remove the exit notifier as it
will become a dangling pointer otherwise.

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 net/slirp.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/net/slirp.c b/net/slirp.c
index 31630f0..28207b6 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -38,6 +38,7 @@
 #include "slirp/libslirp.h"
 #include "slirp/ip6.h"
 #include "sysemu/char.h"
+#include "sysemu/sysemu.h"
 #include "qemu/cutils.h"
 
 static int get_str_sep(char *buf, int buf_size, const char **pp, int sep)
@@ -76,6 +77,7 @@ typedef struct SlirpState {
     NetClientState nc;
     QTAILQ_ENTRY(SlirpState) entry;
     Slirp *slirp;
+    Notifier exit_notifier;
 #ifndef _WIN32
     char smb_dir[128];
 #endif
@@ -118,11 +120,18 @@ static ssize_t net_slirp_receive(NetClientState *nc, const uint8_t *buf, size_t
     return size;
 }
 
+static void slirp_smb_exit(Notifier *n, void *data)
+{
+    SlirpState *s = container_of(n, SlirpState, exit_notifier);
+    slirp_smb_cleanup(s);
+}
+
 static void net_slirp_cleanup(NetClientState *nc)
 {
     SlirpState *s = DO_UPCAST(SlirpState, nc, nc);
 
     slirp_cleanup(s->slirp);
+    qemu_remove_exit_notifier(&s->exit_notifier);
     slirp_smb_cleanup(s);
     QTAILQ_REMOVE(&slirp_stacks, s, entry);
 }
@@ -349,6 +358,8 @@ static int net_slirp_init(NetClientState *peer, const char *model,
     }
 #endif
 
+    s->exit_notifier.notify = slirp_smb_exit;
+    qemu_add_exit_notifier(&s->exit_notifier);
     return 0;
 
 error:
-- 
1.8.3.1

[Qemu-devel] [PULL 11/14] net: do not use atexit for cleanup

[Paolo Bonzini]

This will be necessary in the next patch, which stops using atexit for
character devices; without it, vhost-user and the redirector filter
will cause a use-after-free.  Relying on the ordering of atexit calls
is also brittle, even now that both the network and chardev
subsystems are using atexit.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 vl.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/vl.c b/vl.c
index 356713e..cad4da2 100644
--- a/vl.c
+++ b/vl.c
@@ -4345,9 +4345,6 @@ int main(int argc, char **argv, char **envp)
         qemu_opts_del(icount_opts);
     }
 
-    /* clean up network at qemu process termination */
-    atexit(&net_cleanup);
-
     if (default_net) {
         QemuOptsList *net = qemu_find_opts("net");
         qemu_opts_set(net, NULL, "type", "nic", &error_abort);
@@ -4611,5 +4608,7 @@ int main(int argc, char **argv, char **envp)
     tpm_cleanup();
 #endif
 
+    net_cleanup();
+
     return 0;
 }
-- 
1.8.3.1

[Qemu-devel] [PULL 12/14] char: do not use atexit cleanup handler

[Paolo Bonzini]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

It turns out qemu is calling exit() in various places from various
threads without taking much care of resources state. The atexit()
cleanup handlers cannot easily destroy resources that are in use (by
the same thread or other).

Since c1111a24a3, TCG arm guests run into the following abort() when
running tests, the chardev mutex is locked during the write, so
qemu_mutex_destroy() returns an error:

 #0  0x00007fffdbb806f5 in raise () at /lib64/libc.so.6
 #1  0x00007fffdbb822fa in abort () at /lib64/libc.so.6
 #2  0x00005555557616fe in error_exit (err=<optimized out>, msg=msg@entry=0x555555c38c30 <__func__.14622> "qemu_mutex_destroy")
     at /home/drjones/code/qemu/util/qemu-thread-posix.c:39
 #3  0x0000555555b0be20 in qemu_mutex_destroy (mutex=mutex@entry=0x5555566aa0e0) at /home/drjones/code/qemu/util/qemu-thread-posix.c:57
 #4  0x00005555558aab00 in qemu_chr_free_common (chr=0x5555566aa0e0) at /home/drjones/code/qemu/qemu-char.c:4029
 #5  0x00005555558b05f9 in qemu_chr_delete (chr=<optimized out>) at /home/drjones/code/qemu/qemu-char.c:4038
 #6  0x00005555558b05f9 in qemu_chr_delete (chr=<optimized out>) at /home/drjones/code/qemu/qemu-char.c:4044
 #7  0x00005555558b062c in qemu_chr_cleanup () at /home/drjones/code/qemu/qemu-char.c:4557
 #8  0x00007fffdbb851e8 in __run_exit_handlers () at /lib64/libc.so.6
 #9  0x00007fffdbb85235 in  () at /lib64/libc.so.6
 #10 0x00005555558d1b39 in testdev_write (testdev=0x5555566aa0a0) at /home/drjones/code/qemu/backends/testdev.c:71
 #11 0x00005555558d1b39 in testdev_write (chr=<optimized out>, buf=0x7fffc343fd9a "", len=0) at /home/drjones/code/qemu/backends/testdev.c:95
 #12 0x00005555558adced in qemu_chr_fe_write (s=0x5555566aa0e0, buf=buf@entry=0x7fffc343fd98 "0q", len=len@entry=2) at /home/drjones/code/qemu/qemu-char.c:282

Instead of using a atexit() handler, only run the chardev cleanup as
initially proposed at the end of main(), where there are less chances
(hic) of conflicts or other races.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reported-by: Andrew Jones <drjones@redhat.com>
Message-Id: <20160704153823.16879-1-marcandre.lureau@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/sysemu/char.h | 7 +++++++
 qemu-char.c           | 4 +---
 vl.c                  | 2 ++
 3 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/include/sysemu/char.h b/include/sysemu/char.h
index 57df10a..0ea9eac 100644
--- a/include/sysemu/char.h
+++ b/include/sysemu/char.h
@@ -152,6 +152,13 @@ CharDriverState *qemu_chr_new(const char *label, const char *filename,
 void qemu_chr_disconnect(CharDriverState *chr);
 
 /**
+ * @qemu_chr_cleanup:
+ *
+ * Delete all chardevs (when leaving qemu)
+ */
+void qemu_chr_cleanup(void);
+
+/**
  * @qemu_chr_new_noreplay:
  *
  * Create a new character backend from a URI.
diff --git a/qemu-char.c b/qemu-char.c
index 0698b98..e4b8448 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -4548,7 +4548,7 @@ void qmp_chardev_remove(const char *id, Error **errp)
     qemu_chr_delete(chr);
 }
 
-static void qemu_chr_cleanup(void)
+void qemu_chr_cleanup(void)
 {
     CharDriverState *chr, *tmp;
 
@@ -4603,8 +4603,6 @@ static void register_types(void)
      * is specified
      */
     qemu_add_machine_init_done_notifier(&muxes_realize_notify);
-
-    atexit(qemu_chr_cleanup);
 }
 
 type_init(register_types);
diff --git a/vl.c b/vl.c
index cad4da2..d3ec532 100644
--- a/vl.c
+++ b/vl.c
@@ -4608,7 +4608,9 @@ int main(int argc, char **argv, char **envp)
     tpm_cleanup();
 #endif
 
+    /* vhost-user must be cleaned up before chardevs.  */
     net_cleanup();
+    qemu_chr_cleanup();
 
     return 0;
 }
-- 
1.8.3.1

[Qemu-devel] [PULL 13/14] hostmem: fix QEMU crash by 'info memdev'

[Paolo Bonzini]

From: Xiao Guangrong <guangrong.xiao@linux.intel.com>

'info memdev' crashes QEMU:
   (qemu) info memdev
   Unexpected error in parse_str() at qapi/string-input-visitor.c:111:
   Parameter 'null' expects an int64 value or range
It is caused by null uint16List is returned if 'host-nodes' is the default
value

Return MAX_NODES under this case to fix this bug

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 backends/hostmem.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/backends/hostmem.c b/backends/hostmem.c
index 6e28be1..8dede4d 100644
--- a/backends/hostmem.c
+++ b/backends/hostmem.c
@@ -64,6 +64,14 @@ out:
     error_propagate(errp, local_err);
 }
 
+static uint16List **host_memory_append_node(uint16List **node,
+                                            unsigned long value)
+{
+     *node = g_malloc0(sizeof(**node));
+     (*node)->value = value;
+     return &(*node)->next;
+}
+
 static void
 host_memory_backend_get_host_nodes(Object *obj, Visitor *v, const char *name,
                                    void *opaque, Error **errp)
@@ -74,25 +82,23 @@ host_memory_backend_get_host_nodes(Object *obj, Visitor *v, const char *name,
     unsigned long value;
 
     value = find_first_bit(backend->host_nodes, MAX_NODES);
+
+    node = host_memory_append_node(node, value);
+
     if (value == MAX_NODES) {
-        return;
+        goto out;
     }
 
-    *node = g_malloc0(sizeof(**node));
-    (*node)->value = value;
-    node = &(*node)->next;
-
     do {
         value = find_next_bit(backend->host_nodes, MAX_NODES, value + 1);
         if (value == MAX_NODES) {
             break;
         }
 
-        *node = g_malloc0(sizeof(**node));
-        (*node)->value = value;
-        node = &(*node)->next;
+        node = host_memory_append_node(node, value);
     } while (true);
 
+out:
     visit_type_uint16List(v, name, &host_nodes, errp);
 }
 
-- 
1.8.3.1

[Qemu-devel] [PULL 14/14] hostmem: detect host backend memory is being used properly

[Paolo Bonzini]

From: Xiao Guangrong <guangrong.xiao@linux.intel.com>

Currently, we use memory_region_is_mapped() to detect if the host
backend memory is being used. This works if the memory is directly
mapped into guest's address space, however, it is not true for
nvdimm as it uses aliased memory region to map the memory. This is
why this bug can happen:
   https://bugzilla.redhat.com/show_bug.cgi?id=1352769

Fix it by introduce a new filed, is_mapped, to HostMemoryBackend,
we set/clear this filed accordingly when the device link/unlink to
host backend memory

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 backends/hostmem.c       | 15 +++++++++++----
 hw/mem/pc-dimm.c         | 18 +++++++++++-------
 hw/misc/ivshmem.c        | 14 ++++++++++----
 include/sysemu/hostmem.h |  4 +++-
 4 files changed, 35 insertions(+), 16 deletions(-)

diff --git a/backends/hostmem.c b/backends/hostmem.c
index 8dede4d..ac80257 100644
--- a/backends/hostmem.c
+++ b/backends/hostmem.c
@@ -264,6 +264,16 @@ host_memory_backend_get_memory(HostMemoryBackend *backend, Error **errp)
     return memory_region_size(&backend->mr) ? &backend->mr : NULL;
 }
 
+void host_memory_backend_set_mapped(HostMemoryBackend *backend, bool mapped)
+{
+    backend->is_mapped = mapped;
+}
+
+bool host_memory_backend_is_mapped(HostMemoryBackend *backend)
+{
+    return backend->is_mapped;
+}
+
 static void
 host_memory_backend_memory_complete(UserCreatable *uc, Error **errp)
 {
@@ -341,10 +351,7 @@ host_memory_backend_memory_complete(UserCreatable *uc, Error **errp)
 static bool
 host_memory_backend_can_be_deleted(UserCreatable *uc, Error **errp)
 {
-    MemoryRegion *mr;
-
-    mr = host_memory_backend_get_memory(MEMORY_BACKEND(uc), errp);
-    if (memory_region_is_mapped(mr)) {
+    if (host_memory_backend_is_mapped(MEMORY_BACKEND(uc))) {
         return false;
     } else {
         return true;
diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
index 249193a..9e8dab0 100644
--- a/hw/mem/pc-dimm.c
+++ b/hw/mem/pc-dimm.c
@@ -369,14 +369,9 @@ static void pc_dimm_get_size(Object *obj, Visitor *v, const char *name,
 static void pc_dimm_check_memdev_is_busy(Object *obj, const char *name,
                                       Object *val, Error **errp)
 {
-    MemoryRegion *mr;
     Error *local_err = NULL;
 
-    mr = host_memory_backend_get_memory(MEMORY_BACKEND(val), &local_err);
-    if (local_err) {
-        goto out;
-    }
-    if (memory_region_is_mapped(mr)) {
+    if (host_memory_backend_is_mapped(MEMORY_BACKEND(val))) {
         char *path = object_get_canonical_path_component(val);
         error_setg(&local_err, "can't use already busy memdev: %s", path);
         g_free(path);
@@ -384,7 +379,6 @@ static void pc_dimm_check_memdev_is_busy(Object *obj, const char *name,
         qdev_prop_allow_set_link_before_realize(obj, name, val, &local_err);
     }
 
-out:
     error_propagate(errp, local_err);
 }
 
@@ -421,6 +415,15 @@ static void pc_dimm_realize(DeviceState *dev, Error **errp)
     if (ddc->realize) {
         ddc->realize(dimm, errp);
     }
+
+    host_memory_backend_set_mapped(dimm->hostmem, true);
+}
+
+static void pc_dimm_unrealize(DeviceState *dev, Error **errp)
+{
+    PCDIMMDevice *dimm = PC_DIMM(dev);
+
+    host_memory_backend_set_mapped(dimm->hostmem, false);
 }
 
 static MemoryRegion *pc_dimm_get_memory_region(PCDIMMDevice *dimm)
@@ -439,6 +442,7 @@ static void pc_dimm_class_init(ObjectClass *oc, void *data)
     PCDIMMDeviceClass *ddc = PC_DIMM_CLASS(oc);
 
     dc->realize = pc_dimm_realize;
+    dc->unrealize = pc_dimm_unrealize;
     dc->props = pc_dimm_properties;
     dc->desc = "DIMM memory module";
 
diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index c4dde3a..7e7c843 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -1008,10 +1008,7 @@ static const TypeInfo ivshmem_common_info = {
 static void ivshmem_check_memdev_is_busy(Object *obj, const char *name,
                                          Object *val, Error **errp)
 {
-    MemoryRegion *mr;
-
-    mr = host_memory_backend_get_memory(MEMORY_BACKEND(val), &error_abort);
-    if (memory_region_is_mapped(mr)) {
+    if (host_memory_backend_is_mapped(MEMORY_BACKEND(val))) {
         char *path = object_get_canonical_path_component(val);
         error_setg(errp, "can't use already busy memdev: %s", path);
         g_free(path);
@@ -1060,6 +1057,14 @@ static void ivshmem_plain_realize(PCIDevice *dev, Error **errp)
     }
 
     ivshmem_common_realize(dev, errp);
+    host_memory_backend_set_mapped(s->hostmem, true);
+}
+
+static void ivshmem_plain_exit(PCIDevice *pci_dev)
+{
+    IVShmemState *s = IVSHMEM_COMMON(pci_dev);
+
+    host_memory_backend_set_mapped(s->hostmem, false);
 }
 
 static void ivshmem_plain_class_init(ObjectClass *klass, void *data)
@@ -1068,6 +1073,7 @@ static void ivshmem_plain_class_init(ObjectClass *klass, void *data)
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
     k->realize = ivshmem_plain_realize;
+    k->exit = ivshmem_plain_exit;
     dc->props = ivshmem_plain_properties;
     dc->vmsd = &ivshmem_plain_vmsd;
 }
diff --git a/include/sysemu/hostmem.h b/include/sysemu/hostmem.h
index 4d6617e..c903404 100644
--- a/include/sysemu/hostmem.h
+++ b/include/sysemu/hostmem.h
@@ -53,7 +53,7 @@ struct HostMemoryBackend {
     /* protected */
     uint64_t size;
     bool merge, dump;
-    bool prealloc, force_prealloc;
+    bool prealloc, force_prealloc, is_mapped;
     DECLARE_BITMAP(host_nodes, MAX_NODES + 1);
     HostMemPolicy policy;
 
@@ -63,4 +63,6 @@ struct HostMemoryBackend {
 MemoryRegion *host_memory_backend_get_memory(HostMemoryBackend *backend,
                                              Error **errp);
 
+void host_memory_backend_set_mapped(HostMemoryBackend *backend, bool mapped);
+bool host_memory_backend_is_mapped(HostMemoryBackend *backend);
 #endif
-- 
1.8.3.1

Re: [Qemu-devel] [PULL 00/14] SCSI, chardev, build fixes for 2016-07-13

[Peter Maydell]

On 13 July 2016 at 14:26, Paolo Bonzini <pbonzini@redhat.com> wrote:
> The following changes since commit a91a4e7d8cfe6ece610aacf7c52738188f5b5cb5:
>
>   Merge remote-tracking branch 'remotes/ehabkost/tags/x86-pull-request' into staging (2016-07-11 15:08:47 +0100)
>
> are available in the git repository at:
>
>
>   git://github.com/bonzini/qemu.git tags/for-upstream
>
> for you to fetch changes up to 2aece63c8a9d2c3a8ff41d2febc4cdeff2633331:
>
>   hostmem: detect host backend memory is being used properly (2016-07-13 13:30:04 +0200)
>
> ----------------------------------------------------------------
> * SCSI scanner support
> * fixes to qemu-char and net exit
> * FreeBSD fixes
> * Other small bugfixes
>
> ----------------------------------------------------------------

Applied, thanks.

-- PMM