From 297b044a7fccd1571cf94713a7caf2fc1a980841 Mon Sep 17 00:00:00 2001 From: Jarkko Lavinen Date: Tue, 28 Jun 2016 21:51:52 +0300 Subject: scsi-bus: Add SCSI scanner support Add support for missing scanner specific SCSI commands and their xfer lenghts as per ANSI spec section 15. Signed-off-by: Jarkko Lavinen Signed-off-by: Paolo Bonzini --- 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 ad6f398c32..14c0aa557f 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 a311341e63..8b966d754c 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 -- cgit v1.2.3 From 6959e508c6de656015e0cd4fd21da34d6f08e8ac Mon Sep 17 00:00:00 2001 From: Jarkko Lavinen Date: Wed, 29 Jun 2016 03:11:46 +0300 Subject: scsi-bus: Use longer sense buffer with scanners Scanners can provide additional sense bytes beyond 18 bytes. VueScan uses 32 bytes alloc length with Request Sense command. Signed-off-by: Jarkko Lavinen Signed-off-by: Paolo Bonzini --- 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 14c0aa557f..297216dfcb 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 8acd3fa998..94d7868105 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; -- cgit v1.2.3 From 540aecd0991d1fcbb2204a06fcdbda32c87e788a Mon Sep 17 00:00:00 2001 From: Sean Bruno Date: Tue, 14 Jun 2016 11:07:34 -0700 Subject: Use "-s" instead of "--quiet" to resolve non-fatal build error on FreeBSD. 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 Reviewed-by: Fam Zheng Signed-off-by: Sean Bruno Message-Id: <20160614180734.8782-1-sbruno@freebsd.org> Signed-off-by: Paolo Bonzini --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index c054bc6356..45706375b2 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 -- cgit v1.2.3 From 28ba61e7ff2a824e79a477192aee8ee20b95f194 Mon Sep 17 00:00:00 2001 From: Cao jin Date: Wed, 6 Jul 2016 18:08:59 +0800 Subject: main-loop: check return value before using pointer pointer 'qemu_aio_context' should be checked first before it is used. qemu_bh_new() will use it. Signed-off-by: Cao jin Message-Id: <1467799740-26079-2-git-send-email-caoj.fnst@cn.fujitsu.com> Signed-off-by: Paolo Bonzini --- main-loop.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/main-loop.c b/main-loop.c index 89a699419f..6a7f8d30bd 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); -- cgit v1.2.3 From a942d8fa01f65279cdc135f4294db611bbc088ef Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 4 Jul 2016 14:40:59 +0200 Subject: json-streamer: fix double-free on exiting during a parse 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 Signed-off-by: Paolo Bonzini Message-Id: <1467636059-12557-1-git-send-email-pbonzini@redhat.com> Signed-off-by: Paolo Bonzini --- 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 7164390cf5..c51c2021f9 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; } -- cgit v1.2.3 From 1e13c01d2a4eadb9c498caa809a21e3b5672b411 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 7 Jul 2016 14:07:33 +0200 Subject: disas: avoid including everything in headers compiled from C++ 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 Tested-by: Sean Bruno Cc: Peter Maydell Cc: Markus Armbruster Signed-off-by: Paolo Bonzini --- 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 a112e9c8c3..a87b8a1eb9 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; -- cgit v1.2.3 From 73f40c1895980dc705b6b0594ace6580b3f68537 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Wed, 6 Jul 2016 18:42:46 +0200 Subject: qemu-sockets: use qapi_free_SocketAddress in cleanup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit 74b6ce43e3 uses the wrong free API for a SocketAddress, that may leak some linked data. Signed-off-by: Marc-André Lureau Message-Id: <20160706164246.22116-1-marcandre.lureau@redhat.com> Signed-off-by: Paolo Bonzini --- 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 fb83d48944..777af49d53 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) -- cgit v1.2.3 From d27ba624aa1dfe5c07cc01200d95967ffce905d9 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Tue, 12 Jul 2016 14:48:33 +0800 Subject: util: Fix MIN_NON_ZERO MIN_NON_ZERO(1, 0) is evaluated to 0. Rewrite the macro to fix it. Reported-by: Miroslav Rezanina Signed-off-by: Fam Zheng Message-Id: <1468306113-847-1-git-send-email-famz@redhat.com> Reviewed-by: Eric Blake Signed-off-by: Paolo Bonzini --- 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 e63da2831a..e4c6ae6b38 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 */ -- cgit v1.2.3 From 9e32ff32995f8c18f85f20e28ef09bae4ff2e48a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Mon, 11 Jul 2016 16:48:47 +0200 Subject: tap: use an exit notifier to call down_script MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Suggested-by: Paolo Bonzini Message-Id: <20160711144847.16651-1-marcandre.lureau@redhat.com> Reviewed-by: Jason Wang Signed-off-by: Paolo Bonzini --- net/tap.c | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/net/tap.c b/net/tap.c index 676bad4e11..e9c32f3a44 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; } -- cgit v1.2.3 From f6c2e66ae8c8a03d3044dc9054aa7c16e7f817ee Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 12 Jul 2016 09:57:12 +0200 Subject: slirp: use exit notifier for slirp_smb_cleanup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Jason Wang Signed-off-by: Paolo Bonzini --- net/slirp.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/net/slirp.c b/net/slirp.c index 31630f005c..28207b6614 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: -- cgit v1.2.3 From 8caf911d3316b6eccc503cff0d9a68cf922950f1 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 8 Jul 2016 17:28:34 +0200 Subject: net: do not use atexit for cleanup 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 --- vl.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/vl.c b/vl.c index 356713ea07..cad4da2487 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; } -- cgit v1.2.3 From aa5cb7f5e8bf2e6737d0cb36c118332ca26e7797 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Mon, 4 Jul 2016 17:38:23 +0200 Subject: char: do not use atexit cleanup handler MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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=, 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=) at /home/drjones/code/qemu/qemu-char.c:4038 #6 0x00005555558b05f9 in qemu_chr_delete (chr=) 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=, 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 Reported-by: Andrew Jones Message-Id: <20160704153823.16879-1-marcandre.lureau@redhat.com> Signed-off-by: Paolo Bonzini --- 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 57df10aa00..0ea9eacc40 100644 --- a/include/sysemu/char.h +++ b/include/sysemu/char.h @@ -151,6 +151,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: * diff --git a/qemu-char.c b/qemu-char.c index 0698b98750..e4b8448422 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 cad4da2487..d3ec5320ea 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; } -- cgit v1.2.3 From 1454d33f0507cb54d62ed80f494884157c9e7130 Mon Sep 17 00:00:00 2001 From: Xiao Guangrong Date: Wed, 13 Jul 2016 12:18:05 +0800 Subject: hostmem: fix QEMU crash by 'info memdev' '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 Signed-off-by: Paolo Bonzini --- backends/hostmem.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/backends/hostmem.c b/backends/hostmem.c index 6e28be11eb..8dede4db2e 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); } -- cgit v1.2.3 From 2aece63c8a9d2c3a8ff41d2febc4cdeff2633331 Mon Sep 17 00:00:00 2001 From: Xiao Guangrong Date: Wed, 13 Jul 2016 12:18:06 +0800 Subject: hostmem: detect host backend memory is being used properly 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 Signed-off-by: Paolo Bonzini --- 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 8dede4db2e..ac802570a8 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 249193a543..9e8dab0e89 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 c4dde3a52e..7e7c843b32 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 4d6617eab7..c903404308 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 -- cgit v1.2.3