* [Qemu-devel] [PULL 00/25] Misc QEMU fixes for 2016-08-02
@ 2016-08-02 19:39 Paolo Bonzini
2016-08-02 19:39 ` [Qemu-devel] [PULL 01/25] util/qht: Document memory ordering assumptions Paolo Bonzini
` (25 more replies)
0 siblings, 26 replies; 29+ messages in thread
From: Paolo Bonzini @ 2016-08-02 19:39 UTC (permalink / raw)
To: qemu-devel
The following changes since commit cc0100f464c94bf80ad36cd432f4a1ed58126b60:
MAINTAINERS: Update the Xilinx maintainers (2016-08-01 15:31:32 +0100)
are available in the git repository at:
git://github.com/bonzini/qemu.git tags/for-upstream
for you to fetch changes up to 3531bd22792beae5eba181bf88337d2ff1444817:
util: Fix assertion in iov_copy() upon zero 'bytes' and non-zero 'offset' (2016-08-02 15:00:26 +0200)
----------------------------------------------------------------
* xsetbv fix (x86 targets TCG)
* remove unused functions
* qht segfault and memory leak fixes
* NBD fixes
* Fix for non-power-of-2 discard granularity
* Memory hotplug fixes
* Migration regressions
* IOAPIC fixes and (disabled by default) EOI register support
* Various other small fixes
----------------------------------------------------------------
Cao jin (3):
util: drop inet_nonblocking_connect()
util: drop unix_nonblocking_connect()
util: Drop inet_listen()
Dave Hansen (1):
target-i386: fix typo in xsetbv implementation
Emilio G. Cota (4):
qht: do not segfault when gathering stats from an uninitialized qht
qdist: fix memory leak during binning
qdist: use g_realloc_n instead of g_realloc
qdist: return "(empty)" instead of NULL when printing an empty dist
Eric Blake (4):
nbd: Fix bad flag detection on server
nbd: Limit nbdflags to 16 bits
osdep: Document differences in rounding macros
block: Cater to iscsi with non-power-of-2 discard
Fam Zheng (1):
qdev: Fix use after free in qdev_init_nofail error path
Greg Kurz (1):
numa: set the memory backend "is_mapped" field
Igor Mammedov (3):
fix qemu exit on memory hotplug when allocation fails at prealloc time
i2c: fix migration regression introduced by broadcast support
apic: fix broken migration for kvm-apic
Markus Armbruster (1):
fw_cfg: Make base type "fw_cfg" abstract
Paolo Bonzini (3):
util/qht: Document memory ordering assumptions
checkpatch: add check for bzero
mptsas: really fix migration compatibility
Peter Xu (2):
x86: ioapic: ignore level irq during processing
x86: ioapic: add support for explicit EOI
Robert Ho (1):
Reorganize help output of '-display' option
Shmulik Ladkani (1):
util: Fix assertion in iov_copy() upon zero 'bytes' and non-zero 'offset'
backends/hostmem.c | 18 +++++++---
block/io.c | 15 ++++----
block/nbd-client.h | 2 +-
exec.c | 10 ++++--
hw/core/qdev.c | 2 ++
hw/i2c/core.c | 10 ++++--
hw/intc/ioapic.c | 36 +++++++++++++++----
hw/nvram/fw_cfg.c | 1 +
hw/scsi/mptsas.c | 4 ++-
hw/scsi/mptsas.h | 2 ++
include/block/block_int.h | 37 +++++++++++---------
include/block/nbd.h | 6 ++--
include/hw/i386/ioapic_internal.h | 4 +--
include/hw/i386/pc.h | 2 +-
include/qemu/osdep.h | 8 +++--
include/qemu/qht.h | 5 +++
include/qemu/sockets.h | 8 -----
nbd/client.c | 28 ++++++++-------
nbd/server.c | 13 ++++---
numa.c | 1 +
qemu-nbd.c | 4 +--
qemu-options.hx | 29 +++++++++++----
scripts/checkpatch.pl | 5 ++-
target-i386/translate.c | 2 +-
tests/test-qdist.c | 10 ++++--
tests/test-qht.c | 4 +++
translate-all.c | 70 ++++++++++++++++++++----------------
util/iov.c | 3 +-
util/oslib-posix.c | 26 +++++++-------
util/oslib-win32.c | 2 +-
util/qdist.c | 12 ++++---
util/qemu-sockets.c | 74 ---------------------------------------
util/qht.c | 14 ++++++--
33 files changed, 251 insertions(+), 216 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 29+ messages in thread
* [Qemu-devel] [PULL 01/25] util/qht: Document memory ordering assumptions
2016-08-02 19:39 [Qemu-devel] [PULL 00/25] Misc QEMU fixes for 2016-08-02 Paolo Bonzini
@ 2016-08-02 19:39 ` Paolo Bonzini
2016-08-02 19:39 ` [Qemu-devel] [PULL 02/25] numa: set the memory backend "is_mapped" field Paolo Bonzini
` (24 subsequent siblings)
25 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2016-08-02 19:39 UTC (permalink / raw)
To: qemu-devel; +Cc: Sergey Fedorov
It is naturally expected that some memory ordering should be provided
around qht_insert() and qht_lookup(). Document these assumptions in the
header file and put some comments in the source to denote how that
memory ordering requirements are fulfilled.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
[Sergey Fedorov: commit title and message provided;
comment on qht_remove() elided]
Signed-off-by: Sergey Fedorov <serge.fdrv@gmail.com>
Message-Id: <20160715175852.30749-2-sergey.fedorov@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/qemu/qht.h | 5 +++++
util/qht.c | 7 ++++++-
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/include/qemu/qht.h b/include/qemu/qht.h
index 70bfc68..311139b 100644
--- a/include/qemu/qht.h
+++ b/include/qemu/qht.h
@@ -69,6 +69,9 @@ void qht_destroy(struct qht *ht);
* Attempting to insert a NULL @p is a bug.
* Inserting the same pointer @p with different @hash values is a bug.
*
+ * In case of successful operation, smp_wmb() is implied before the pointer is
+ * inserted into the hash table.
+ *
* Returns true on sucess.
* Returns false if the @p-@hash pair already exists in the hash table.
*/
@@ -83,6 +86,8 @@ bool qht_insert(struct qht *ht, void *p, uint32_t hash);
*
* Needs to be called under an RCU read-critical section.
*
+ * smp_read_barrier_depends() is implied before the call to @func.
+ *
* The user-provided @func compares pointers in QHT against @userp.
* If the function returns true, a match has been found.
*
diff --git a/util/qht.c b/util/qht.c
index 40d6e21..28ce289 100644
--- a/util/qht.c
+++ b/util/qht.c
@@ -445,7 +445,11 @@ void *qht_do_lookup(struct qht_bucket *head, qht_lookup_func_t func,
do {
for (i = 0; i < QHT_BUCKET_ENTRIES; i++) {
if (b->hashes[i] == hash) {
- void *p = atomic_read(&b->pointers[i]);
+ /* The pointer is dereferenced before seqlock_read_retry,
+ * so (unlike qht_insert__locked) we need to use
+ * atomic_rcu_read here.
+ */
+ void *p = atomic_rcu_read(&b->pointers[i]);
if (likely(p) && likely(func(p, userp))) {
return p;
@@ -535,6 +539,7 @@ static bool qht_insert__locked(struct qht *ht, struct qht_map *map,
atomic_rcu_set(&prev->next, b);
}
b->hashes[i] = hash;
+ /* smp_wmb() implicit in seqlock_write_begin. */
atomic_set(&b->pointers[i], p);
seqlock_write_end(&head->sequence);
return true;
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [PULL 02/25] numa: set the memory backend "is_mapped" field
2016-08-02 19:39 [Qemu-devel] [PULL 00/25] Misc QEMU fixes for 2016-08-02 Paolo Bonzini
2016-08-02 19:39 ` [Qemu-devel] [PULL 01/25] util/qht: Document memory ordering assumptions Paolo Bonzini
@ 2016-08-02 19:39 ` Paolo Bonzini
2016-08-02 19:39 ` [Qemu-devel] [PULL 03/25] fix qemu exit on memory hotplug when allocation fails at prealloc time Paolo Bonzini
` (23 subsequent siblings)
25 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2016-08-02 19:39 UTC (permalink / raw)
To: qemu-devel; +Cc: Greg Kurz
From: Greg Kurz <groug@kaod.org>
Commit 2aece63 "hostmem: detect host backend memory is being used properly"
added a way to know if a memory backend is busy or available for use. It
caused a slight regression if we pass the same backend to a NUMA node and
to a pc-dimm device:
-m 1G,slots=2,maxmem=2G \
-object memory-backend-ram,size=1G,id=mem-mem1 \
-device pc-dimm,id=dimm-mem1,memdev=mem-mem1 \
-numa node,nodeid=0,memdev=mem-mem1
Before commit 2aece63, this would cause QEMU to print an error message and
to exit gracefully:
qemu-system-ppc64: -device pc-dimm,id=dimm-mem1,memdev=mem-mem1:
can't use already busy memdev: mem-mem1
Since commit 2aece63, QEMU hits an assertion in the memory code:
qemu-system-ppc64: memory.c:1934: memory_region_add_subregion_common:
Assertion `!subregion->container' failed.
Aborted
This happens because pc-dimm devices don't use memory_region_is_mapped()
anymore and cannot guess the backend is already used by a NUMA node.
Let's revert to the previous behavior by turning the NUMA code to also
call host_memory_backend_set_mapped() when it uses a backend.
Fixes: 2aece63c8a9d2c3a8ff41d2febc4cdeff2633331
Signed-off-by: Greg Kurz <groug@kaod.org>
Message-Id: <146891691503.15642.9817215371777203794.stgit@bahia.lan>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
numa.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/numa.c b/numa.c
index cbae430..7286171 100644
--- a/numa.c
+++ b/numa.c
@@ -463,6 +463,7 @@ void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner,
exit(1);
}
+ host_memory_backend_set_mapped(backend, true);
memory_region_add_subregion(mr, addr, seg);
vmstate_register_ram_global(seg);
addr += size;
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [PULL 03/25] fix qemu exit on memory hotplug when allocation fails at prealloc time
2016-08-02 19:39 [Qemu-devel] [PULL 00/25] Misc QEMU fixes for 2016-08-02 Paolo Bonzini
2016-08-02 19:39 ` [Qemu-devel] [PULL 01/25] util/qht: Document memory ordering assumptions Paolo Bonzini
2016-08-02 19:39 ` [Qemu-devel] [PULL 02/25] numa: set the memory backend "is_mapped" field Paolo Bonzini
@ 2016-08-02 19:39 ` Paolo Bonzini
2016-08-02 19:39 ` [Qemu-devel] [PULL 04/25] checkpatch: add check for bzero Paolo Bonzini
` (22 subsequent siblings)
25 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2016-08-02 19:39 UTC (permalink / raw)
To: qemu-devel; +Cc: Igor Mammedov
From: Igor Mammedov <imammedo@redhat.com>
When adding hostmem backend at runtime, QEMU might exit with error:
"os_mem_prealloc: Insufficient free host memory pages available to allocate guest RAM"
It happens due to os_mem_prealloc() not handling errors gracefully.
Fix it by passing errp argument so that os_mem_prealloc() could
report error to callers and undo performed allocation when
os_mem_prealloc() fails.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Message-Id: <1469008443-72059-1-git-send-email-imammedo@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
backends/hostmem.c | 18 ++++++++++++++----
exec.c | 10 ++++++++--
include/qemu/osdep.h | 2 +-
util/oslib-posix.c | 26 +++++++++++++-------------
util/oslib-win32.c | 2 +-
5 files changed, 37 insertions(+), 21 deletions(-)
diff --git a/backends/hostmem.c b/backends/hostmem.c
index ac80257..b7a208d 100644
--- a/backends/hostmem.c
+++ b/backends/hostmem.c
@@ -203,6 +203,7 @@ static bool host_memory_backend_get_prealloc(Object *obj, Error **errp)
static void host_memory_backend_set_prealloc(Object *obj, bool value,
Error **errp)
{
+ Error *local_err = NULL;
HostMemoryBackend *backend = MEMORY_BACKEND(obj);
if (backend->force_prealloc) {
@@ -223,7 +224,11 @@ static void host_memory_backend_set_prealloc(Object *obj, bool value,
void *ptr = memory_region_get_ram_ptr(&backend->mr);
uint64_t sz = memory_region_size(&backend->mr);
- os_mem_prealloc(fd, ptr, sz);
+ os_mem_prealloc(fd, ptr, sz, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
+ }
backend->prealloc = true;
}
}
@@ -286,8 +291,7 @@ host_memory_backend_memory_complete(UserCreatable *uc, Error **errp)
if (bc->alloc) {
bc->alloc(backend, &local_err);
if (local_err) {
- error_propagate(errp, local_err);
- return;
+ goto out;
}
ptr = memory_region_get_ram_ptr(&backend->mr);
@@ -343,9 +347,15 @@ host_memory_backend_memory_complete(UserCreatable *uc, Error **errp)
* specified NUMA policy in place.
*/
if (backend->prealloc) {
- os_mem_prealloc(memory_region_get_fd(&backend->mr), ptr, sz);
+ os_mem_prealloc(memory_region_get_fd(&backend->mr), ptr, sz,
+ &local_err);
+ if (local_err) {
+ goto out;
+ }
}
}
+out:
+ error_propagate(errp, local_err);
}
static bool
diff --git a/exec.c b/exec.c
index 50e3ee2..8ffde75 100644
--- a/exec.c
+++ b/exec.c
@@ -1226,7 +1226,7 @@ static void *file_ram_alloc(RAMBlock *block,
char *filename;
char *sanitized_name;
char *c;
- void *area;
+ void *area = MAP_FAILED;
int fd = -1;
int64_t page_size;
@@ -1314,13 +1314,19 @@ static void *file_ram_alloc(RAMBlock *block,
}
if (mem_prealloc) {
- os_mem_prealloc(fd, area, memory);
+ os_mem_prealloc(fd, area, memory, errp);
+ if (errp && *errp) {
+ goto error;
+ }
}
block->fd = fd;
return area;
error:
+ if (area != MAP_FAILED) {
+ qemu_ram_munmap(area, memory);
+ }
if (unlink_on_error) {
unlink(path);
}
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index fbb8759..d7c111d 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -379,7 +379,7 @@ unsigned long qemu_getauxval(unsigned long type);
void qemu_set_tty_echo(int fd, bool echo);
-void os_mem_prealloc(int fd, char *area, size_t sz);
+void os_mem_prealloc(int fd, char *area, size_t sz, Error **errp);
int qemu_read_password(char *buf, int buf_size);
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 6d70d9a..f2d4e9e 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -318,7 +318,7 @@ static void sigbus_handler(int signal)
siglongjmp(sigjump, 1);
}
-void os_mem_prealloc(int fd, char *area, size_t memory)
+void os_mem_prealloc(int fd, char *area, size_t memory, Error **errp)
{
int ret;
struct sigaction act, oldact;
@@ -330,8 +330,9 @@ void os_mem_prealloc(int fd, char *area, size_t memory)
ret = sigaction(SIGBUS, &act, &oldact);
if (ret) {
- perror("os_mem_prealloc: failed to install signal handler");
- exit(1);
+ error_setg_errno(errp, errno,
+ "os_mem_prealloc: failed to install signal handler");
+ return;
}
/* unblock SIGBUS */
@@ -340,9 +341,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: Insufficient free host memory "
- "pages available to allocate guest RAM\n");
- exit(1);
+ error_setg(errp, "os_mem_prealloc: Insufficient free host memory "
+ "pages available to allocate guest RAM\n");
} else {
int i;
size_t hpagesize = qemu_fd_getpagesize(fd);
@@ -352,15 +352,15 @@ void os_mem_prealloc(int fd, char *area, size_t memory)
for (i = 0; i < numpages; i++) {
memset(area + (hpagesize * i), 0, 1);
}
+ }
- ret = sigaction(SIGBUS, &oldact, NULL);
- if (ret) {
- perror("os_mem_prealloc: failed to reinstall signal handler");
- exit(1);
- }
-
- pthread_sigmask(SIG_SETMASK, &oldset, NULL);
+ ret = sigaction(SIGBUS, &oldact, NULL);
+ if (ret) {
+ /* Terminate QEMU since it can't recover from error */
+ perror("os_mem_prealloc: failed to reinstall signal handler");
+ exit(1);
}
+ pthread_sigmask(SIG_SETMASK, &oldset, NULL);
}
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index 6debc2b..4c1dcf1 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -539,7 +539,7 @@ int getpagesize(void)
return system_info.dwPageSize;
}
-void os_mem_prealloc(int fd, char *area, size_t memory)
+void os_mem_prealloc(int fd, char *area, size_t memory, Error **errp)
{
int i;
size_t pagesize = getpagesize();
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [PULL 04/25] checkpatch: add check for bzero
2016-08-02 19:39 [Qemu-devel] [PULL 00/25] Misc QEMU fixes for 2016-08-02 Paolo Bonzini
` (2 preceding siblings ...)
2016-08-02 19:39 ` [Qemu-devel] [PULL 03/25] fix qemu exit on memory hotplug when allocation fails at prealloc time Paolo Bonzini
@ 2016-08-02 19:39 ` Paolo Bonzini
2016-08-02 19:39 ` [Qemu-devel] [PULL 05/25] util: drop inet_nonblocking_connect() Paolo Bonzini
` (21 subsequent siblings)
25 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2016-08-02 19:39 UTC (permalink / raw)
To: qemu-devel
Tested-By: Peter Xu <peterx@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
scripts/checkpatch.pl | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index afa7f79..b7cb4ab 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2544,7 +2544,7 @@ sub process {
}
}
-# check for non-portable ffs() calls that have portable alternatives in QEMU
+# check for non-portable libc calls that have portable alternatives in QEMU
if ($line =~ /\bffs\(/) {
ERROR("use ctz32() instead of ffs()\n" . $herecurr);
}
@@ -2554,6 +2554,9 @@ sub process {
if ($line =~ /\bffsll\(/) {
ERROR("use ctz64() instead of ffsll()\n" . $herecurr);
}
+ if ($line =~ /\bbzero\(/) {
+ ERROR("use memset() instead of bzero()\n" . $herecurr);
+ }
}
# If we have no input at all, then there is nothing to report on
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [PULL 05/25] util: drop inet_nonblocking_connect()
2016-08-02 19:39 [Qemu-devel] [PULL 00/25] Misc QEMU fixes for 2016-08-02 Paolo Bonzini
` (3 preceding siblings ...)
2016-08-02 19:39 ` [Qemu-devel] [PULL 04/25] checkpatch: add check for bzero Paolo Bonzini
@ 2016-08-02 19:39 ` Paolo Bonzini
2016-08-02 19:39 ` [Qemu-devel] [PULL 06/25] util: drop unix_nonblocking_connect() Paolo Bonzini
` (20 subsequent siblings)
25 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2016-08-02 19:39 UTC (permalink / raw)
To: qemu-devel; +Cc: Cao jin, Daniel P . Berrange, Gerd Hoffmann
From: Cao jin <caoj.fnst@cn.fujitsu.com>
It is never used; all nonblocking connect now goes through
socket_connect(), which calls inet_connect_addr().
Cc: Daniel P. Berrange <berrange@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
Message-Id: <1469097213-26441-2-git-send-email-caoj.fnst@cn.fujitsu.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/qemu/sockets.h | 3 ---
util/qemu-sockets.c | 30 ------------------------------
2 files changed, 33 deletions(-)
diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
index 5fe01fb..2cbe643 100644
--- a/include/qemu/sockets.h
+++ b/include/qemu/sockets.h
@@ -36,9 +36,6 @@ InetSocketAddress *inet_parse(const char *str, Error **errp);
int inet_listen(const char *str, char *ostr, int olen,
int socktype, int port_offset, Error **errp);
int inet_connect(const char *str, Error **errp);
-int inet_nonblocking_connect(const char *str,
- NonBlockingConnectHandler *callback,
- void *opaque, Error **errp);
NetworkAddressFamily inet_netfamily(int family);
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 777af49..2e0570b 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -674,36 +674,6 @@ int inet_connect(const char *str, Error **errp)
return sock;
}
-/**
- * Create a non-blocking socket and connect it to an address.
- * Calls the callback function with fd in case of success or -1 in case of
- * error.
- *
- * @str: address string
- * @callback: callback function that is called when connect completes,
- * cannot be NULL.
- * @opaque: opaque for callback function
- * @errp: set in case of an error
- *
- * Returns: -1 on immediate error, file descriptor on success.
- **/
-int inet_nonblocking_connect(const char *str,
- NonBlockingConnectHandler *callback,
- void *opaque, Error **errp)
-{
- int sock = -1;
- InetSocketAddress *addr;
-
- g_assert(callback != NULL);
-
- addr = inet_parse(str, errp);
- if (addr != NULL) {
- sock = inet_connect_saddr(addr, errp, callback, opaque);
- qapi_free_InetSocketAddress(addr);
- }
- return sock;
-}
-
#ifndef _WIN32
static int unix_listen_saddr(UnixSocketAddress *saddr,
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [PULL 06/25] util: drop unix_nonblocking_connect()
2016-08-02 19:39 [Qemu-devel] [PULL 00/25] Misc QEMU fixes for 2016-08-02 Paolo Bonzini
` (4 preceding siblings ...)
2016-08-02 19:39 ` [Qemu-devel] [PULL 05/25] util: drop inet_nonblocking_connect() Paolo Bonzini
@ 2016-08-02 19:39 ` Paolo Bonzini
2016-08-02 19:39 ` [Qemu-devel] [PULL 07/25] util: Drop inet_listen() Paolo Bonzini
` (19 subsequent siblings)
25 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2016-08-02 19:39 UTC (permalink / raw)
To: qemu-devel; +Cc: Cao jin, Daniel P . Berrange, Gerd Hoffmann
From: Cao jin <caoj.fnst@cn.fujitsu.com>
It is never used; all nonblocking connect now goes through
socket_connect(), which calls unix_connect_addr().
Cc: Daniel P. Berrange <berrange@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
Message-Id: <1469097213-26441-3-git-send-email-caoj.fnst@cn.fujitsu.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/qemu/sockets.h | 3 ---
util/qemu-sockets.c | 16 ----------------
2 files changed, 19 deletions(-)
diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
index 2cbe643..28a28c0 100644
--- a/include/qemu/sockets.h
+++ b/include/qemu/sockets.h
@@ -41,9 +41,6 @@ NetworkAddressFamily inet_netfamily(int family);
int unix_listen(const char *path, char *ostr, int olen, Error **errp);
int unix_connect(const char *path, Error **errp);
-int unix_nonblocking_connect(const char *str,
- NonBlockingConnectHandler *callback,
- void *opaque, Error **errp);
SocketAddress *socket_parse(const char *str, Error **errp);
int socket_connect(SocketAddress *addr, Error **errp,
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 2e0570b..58f9a2c 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -863,22 +863,6 @@ int unix_connect(const char *path, Error **errp)
}
-int unix_nonblocking_connect(const char *path,
- NonBlockingConnectHandler *callback,
- void *opaque, Error **errp)
-{
- UnixSocketAddress *saddr;
- int sock = -1;
-
- g_assert(callback != NULL);
-
- saddr = g_new0(UnixSocketAddress, 1);
- saddr->path = g_strdup(path);
- sock = unix_connect_saddr(saddr, errp, callback, opaque);
- qapi_free_UnixSocketAddress(saddr);
- return sock;
-}
-
SocketAddress *socket_parse(const char *str, Error **errp)
{
SocketAddress *addr;
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [PULL 07/25] util: Drop inet_listen()
2016-08-02 19:39 [Qemu-devel] [PULL 00/25] Misc QEMU fixes for 2016-08-02 Paolo Bonzini
` (5 preceding siblings ...)
2016-08-02 19:39 ` [Qemu-devel] [PULL 06/25] util: drop unix_nonblocking_connect() Paolo Bonzini
@ 2016-08-02 19:39 ` Paolo Bonzini
2016-08-02 19:39 ` [Qemu-devel] [PULL 08/25] qht: do not segfault when gathering stats from an uninitialized qht Paolo Bonzini
` (18 subsequent siblings)
25 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2016-08-02 19:39 UTC (permalink / raw)
To: qemu-devel; +Cc: Cao jin, Daniel P . Berrange, Gerd Hoffmann, Eric Blake
From: Cao jin <caoj.fnst@cn.fujitsu.com>
Since commit e65c67e4, inet_listen() is not used anymore, and all
inet listen operation goes through QIOChannel.
Cc: Daniel P. Berrange <berrange@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Eric Blake <eblake@redhat.com>
Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
Message-Id: <1469451771-1173-3-git-send-email-caoj.fnst@cn.fujitsu.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/qemu/sockets.h | 2 --
util/qemu-sockets.c | 28 ----------------------------
2 files changed, 30 deletions(-)
diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
index 28a28c0..9eb2470 100644
--- a/include/qemu/sockets.h
+++ b/include/qemu/sockets.h
@@ -33,8 +33,6 @@ int socket_set_fast_reuse(int fd);
typedef void NonBlockingConnectHandler(int fd, Error *err, void *opaque);
InetSocketAddress *inet_parse(const char *str, Error **errp);
-int inet_listen(const char *str, char *ostr, int olen,
- int socktype, int port_offset, Error **errp);
int inet_connect(const char *str, Error **errp);
NetworkAddressFamily inet_netfamily(int family);
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 58f9a2c..2aed799 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -624,34 +624,6 @@ fail:
return NULL;
}
-int inet_listen(const char *str, char *ostr, int olen,
- int socktype, int port_offset, Error **errp)
-{
- char *optstr;
- int sock = -1;
- InetSocketAddress *addr;
-
- addr = inet_parse(str, errp);
- if (addr != NULL) {
- sock = inet_listen_saddr(addr, port_offset, true, errp);
- if (sock != -1 && ostr) {
- optstr = strchr(str, ',');
- if (addr->ipv6) {
- snprintf(ostr, olen, "[%s]:%s%s",
- addr->host,
- addr->port,
- optstr ? optstr : "");
- } else {
- snprintf(ostr, olen, "%s:%s%s",
- addr->host,
- addr->port,
- optstr ? optstr : "");
- }
- }
- qapi_free_InetSocketAddress(addr);
- }
- return sock;
-}
/**
* Create a blocking socket and connect it to an address.
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [PULL 08/25] qht: do not segfault when gathering stats from an uninitialized qht
2016-08-02 19:39 [Qemu-devel] [PULL 00/25] Misc QEMU fixes for 2016-08-02 Paolo Bonzini
` (6 preceding siblings ...)
2016-08-02 19:39 ` [Qemu-devel] [PULL 07/25] util: Drop inet_listen() Paolo Bonzini
@ 2016-08-02 19:39 ` Paolo Bonzini
2016-08-02 19:39 ` [Qemu-devel] [PULL 09/25] target-i386: fix typo in xsetbv implementation Paolo Bonzini
` (17 subsequent siblings)
25 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2016-08-02 19:39 UTC (permalink / raw)
To: qemu-devel; +Cc: Emilio G. Cota
From: "Emilio G. Cota" <cota@braap.org>
So far, QHT functions assume that the passed qht has previously been
initialized--otherwise they segfault.
This patch makes an exception for qht_statistics_init, with the goal
of simplifying calling code. For instance, qht_statistics_init is
called from the 'info jit' dump, and given that under KVM the TB qht
is never initialized, we get a segfault. Thus, instead of complicating
the 'info jit' code with additional checks, let's allow passing an
uninitialized qht to qht_statistics_init.
While at it, add a test for this to test-qht.
Before the patch (for $ qemu -enable-kvm [...]):
(qemu) info jit
[...]
direct jump count 0 (0%) (2 jumps=0 0%)
Program received signal SIGSEGV, Segmentation fault.
After the patch the "TB hash buckets", "TB hash occupancy"
and "TB hash avg chain" lines are omitted.
(qemu) info jit
[...]
direct jump count 0 (0%) (2 jumps=0 0%)
TB hash buckets 0/0 (-nan% head buckets used)
TB hash occupancy nan% avg chain occ. Histogram: (null)
TB hash avg chain nan buckets. Histogram: (null)
[...]
Reported by: Changlong Xie <xiecl.fnst@cn.fujitsu.com>
Signed-off-by: Emilio G. Cota <cota@braap.org>
Message-Id: <1469205390-14369-1-git-send-email-cota@braap.org>
[Extract printing statistics to an entirely separate function. - Paolo]
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
tests/test-qht.c | 4 ++++
translate-all.c | 70 +++++++++++++++++++++++++++++++-------------------------
util/qht.c | 7 +++++-
3 files changed, 49 insertions(+), 32 deletions(-)
diff --git a/tests/test-qht.c b/tests/test-qht.c
index f1d6283..46a64b6 100644
--- a/tests/test-qht.c
+++ b/tests/test-qht.c
@@ -95,8 +95,12 @@ static void iter_check(unsigned int count)
static void qht_do_test(unsigned int mode, size_t init_entries)
{
+ /* under KVM we might fetch stats from an uninitialized qht */
+ check_n(0);
+
qht_init(&ht, 0, mode);
+ check_n(0);
insert(0, N);
check(0, N, true);
check_n(N);
diff --git a/translate-all.c b/translate-all.c
index 0d47c1c..efeba29 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -1663,15 +1663,50 @@ void tb_flush_jmp_cache(CPUState *cpu, target_ulong addr)
TB_JMP_PAGE_SIZE * sizeof(TranslationBlock *));
}
+static void print_qht_statistics(FILE *f, fprintf_function cpu_fprintf,
+ struct qht_stats hst)
+{
+ uint32_t hgram_opts;
+ size_t hgram_bins;
+ char *hgram;
+
+ if (!hst.head_buckets) {
+ return;
+ }
+ cpu_fprintf(f, "TB hash buckets %zu/%zu (%0.2f%% head buckets used)\n",
+ hst.used_head_buckets, hst.head_buckets,
+ (double)hst.used_head_buckets / hst.head_buckets * 100);
+
+ hgram_opts = QDIST_PR_BORDER | QDIST_PR_LABELS;
+ hgram_opts |= QDIST_PR_100X | QDIST_PR_PERCENT;
+ if (qdist_xmax(&hst.occupancy) - qdist_xmin(&hst.occupancy) == 1) {
+ hgram_opts |= QDIST_PR_NODECIMAL;
+ }
+ hgram = qdist_pr(&hst.occupancy, 10, hgram_opts);
+ cpu_fprintf(f, "TB hash occupancy %0.2f%% avg chain occ. Histogram: %s\n",
+ qdist_avg(&hst.occupancy) * 100, hgram);
+ g_free(hgram);
+
+ hgram_opts = QDIST_PR_BORDER | QDIST_PR_LABELS;
+ hgram_bins = qdist_xmax(&hst.chain) - qdist_xmin(&hst.chain);
+ if (hgram_bins > 10) {
+ hgram_bins = 10;
+ } else {
+ hgram_bins = 0;
+ hgram_opts |= QDIST_PR_NODECIMAL | QDIST_PR_NOBINRANGE;
+ }
+ hgram = qdist_pr(&hst.chain, hgram_bins, hgram_opts);
+ cpu_fprintf(f, "TB hash avg chain %0.3f buckets. Histogram: %s\n",
+ qdist_avg(&hst.chain), hgram);
+ g_free(hgram);
+}
+
void dump_exec_info(FILE *f, fprintf_function cpu_fprintf)
{
int i, target_code_size, max_target_code_size;
int direct_jmp_count, direct_jmp2_count, cross_page;
TranslationBlock *tb;
struct qht_stats hst;
- uint32_t hgram_opts;
- size_t hgram_bins;
- char *hgram;
target_code_size = 0;
max_target_code_size = 0;
@@ -1724,34 +1759,7 @@ void dump_exec_info(FILE *f, fprintf_function cpu_fprintf)
tcg_ctx.tb_ctx.nb_tbs : 0);
qht_statistics_init(&tcg_ctx.tb_ctx.htable, &hst);
-
- cpu_fprintf(f, "TB hash buckets %zu/%zu (%0.2f%% head buckets used)\n",
- hst.used_head_buckets, hst.head_buckets,
- (double)hst.used_head_buckets / hst.head_buckets * 100);
-
- hgram_opts = QDIST_PR_BORDER | QDIST_PR_LABELS;
- hgram_opts |= QDIST_PR_100X | QDIST_PR_PERCENT;
- if (qdist_xmax(&hst.occupancy) - qdist_xmin(&hst.occupancy) == 1) {
- hgram_opts |= QDIST_PR_NODECIMAL;
- }
- hgram = qdist_pr(&hst.occupancy, 10, hgram_opts);
- cpu_fprintf(f, "TB hash occupancy %0.2f%% avg chain occ. Histogram: %s\n",
- qdist_avg(&hst.occupancy) * 100, hgram);
- g_free(hgram);
-
- hgram_opts = QDIST_PR_BORDER | QDIST_PR_LABELS;
- hgram_bins = qdist_xmax(&hst.chain) - qdist_xmin(&hst.chain);
- if (hgram_bins > 10) {
- hgram_bins = 10;
- } else {
- hgram_bins = 0;
- hgram_opts |= QDIST_PR_NODECIMAL | QDIST_PR_NOBINRANGE;
- }
- hgram = qdist_pr(&hst.chain, hgram_bins, hgram_opts);
- cpu_fprintf(f, "TB hash avg chain %0.3f buckets. Histogram: %s\n",
- qdist_avg(&hst.chain), hgram);
- g_free(hgram);
-
+ print_qht_statistics(f, cpu_fprintf, hst);
qht_statistics_destroy(&hst);
cpu_fprintf(f, "\nStatistics:\n");
diff --git a/util/qht.c b/util/qht.c
index 28ce289..16a8d79 100644
--- a/util/qht.c
+++ b/util/qht.c
@@ -789,11 +789,16 @@ void qht_statistics_init(struct qht *ht, struct qht_stats *stats)
map = atomic_rcu_read(&ht->map);
- stats->head_buckets = map->n_buckets;
stats->used_head_buckets = 0;
stats->entries = 0;
qdist_init(&stats->chain);
qdist_init(&stats->occupancy);
+ /* bail out if the qht has not yet been initialized */
+ if (unlikely(map == NULL)) {
+ stats->head_buckets = 0;
+ return;
+ }
+ stats->head_buckets = map->n_buckets;
for (i = 0; i < map->n_buckets; i++) {
struct qht_bucket *head = &map->buckets[i];
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [PULL 09/25] target-i386: fix typo in xsetbv implementation
2016-08-02 19:39 [Qemu-devel] [PULL 00/25] Misc QEMU fixes for 2016-08-02 Paolo Bonzini
` (7 preceding siblings ...)
2016-08-02 19:39 ` [Qemu-devel] [PULL 08/25] qht: do not segfault when gathering stats from an uninitialized qht Paolo Bonzini
@ 2016-08-02 19:39 ` Paolo Bonzini
2016-08-02 19:39 ` [Qemu-devel] [PULL 10/25] qdist: fix memory leak during binning Paolo Bonzini
` (16 subsequent siblings)
25 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2016-08-02 19:39 UTC (permalink / raw)
To: qemu-devel; +Cc: Dave Hansen, qemu-stable, Eduardo Habkost
From: Dave Hansen <dave.hansen@linux.intel.com>
QEMU 2.6 added support for the XSAVE family of instructions, which
includes the XSETBV instruction which allows setting the XCR0
register.
But, when booting Linux kernels with XSAVE support enabled, I was
getting very early crashes where the instruction pointer was set
to 0x3. I tracked it down to a jump instruction generated by this:
gen_jmp_im(s->pc - pc_start);
where s->pc is pointing to the instruction after XSETBV and pc_start
is pointing _at_ XSETBV. Subtract the two and you get 0x3. Whoops.
The fix is to replace this typo with the pattern found everywhere
else in the file when folks want to end the translation buffer.
Richard Henderson confirmed that this is a bug and that this is the
correct fix.
Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
Cc: qemu-stable@nongnu.org
Cc: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target-i386/translate.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target-i386/translate.c b/target-i386/translate.c
index e81fce7..fa2ac48 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -7176,7 +7176,7 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
tcg_gen_trunc_tl_i32(cpu_tmp2_i32, cpu_regs[R_ECX]);
gen_helper_xsetbv(cpu_env, cpu_tmp2_i32, cpu_tmp1_i64);
/* End TB because translation flags may change. */
- gen_jmp_im(s->pc - pc_start);
+ gen_jmp_im(s->pc - s->cs_base);
gen_eob(s);
break;
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [PULL 10/25] qdist: fix memory leak during binning
2016-08-02 19:39 [Qemu-devel] [PULL 00/25] Misc QEMU fixes for 2016-08-02 Paolo Bonzini
` (8 preceding siblings ...)
2016-08-02 19:39 ` [Qemu-devel] [PULL 09/25] target-i386: fix typo in xsetbv implementation Paolo Bonzini
@ 2016-08-02 19:39 ` Paolo Bonzini
2016-08-02 21:13 ` Marc-André Lureau
2016-08-02 19:39 ` [Qemu-devel] [PULL 11/25] qdist: use g_realloc_n instead of g_realloc Paolo Bonzini
` (15 subsequent siblings)
25 siblings, 1 reply; 29+ messages in thread
From: Paolo Bonzini @ 2016-08-02 19:39 UTC (permalink / raw)
To: qemu-devel; +Cc: Emilio G. Cota
From: "Emilio G. Cota" <cota@braap.org>
In qdist_bin__internal(), to->entries is initialized to a 1-element array,
which we then leak when n == from->n. Fix it.
Signed-off-by: Emilio G. Cota <cota@braap.org>
Message-Id: <1469459025-23606-2-git-send-email-cota@braap.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
util/qdist.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/util/qdist.c b/util/qdist.c
index 56f5738..eb2236c 100644
--- a/util/qdist.c
+++ b/util/qdist.c
@@ -188,7 +188,7 @@ void qdist_bin__internal(struct qdist *to, const struct qdist *from, size_t n)
}
}
/* they're equally spaced, so copy the dist and bail out */
- to->entries = g_new(struct qdist_entry, from->n);
+ to->entries = g_realloc_n(to->entries, n, sizeof(*to->entries));
to->n = from->n;
memcpy(to->entries, from->entries, sizeof(*to->entries) * to->n);
return;
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [PULL 11/25] qdist: use g_realloc_n instead of g_realloc
2016-08-02 19:39 [Qemu-devel] [PULL 00/25] Misc QEMU fixes for 2016-08-02 Paolo Bonzini
` (9 preceding siblings ...)
2016-08-02 19:39 ` [Qemu-devel] [PULL 10/25] qdist: fix memory leak during binning Paolo Bonzini
@ 2016-08-02 19:39 ` Paolo Bonzini
2016-08-02 19:39 ` [Qemu-devel] [PULL 12/25] qdist: return "(empty)" instead of NULL when printing an empty dist Paolo Bonzini
` (14 subsequent siblings)
25 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2016-08-02 19:39 UTC (permalink / raw)
To: qemu-devel; +Cc: Emilio G. Cota
From: "Emilio G. Cota" <cota@braap.org>
While at it, remove the unnecessary parentheses around dist->size.
Signed-off-by: Emilio G. Cota <cota@braap.org>
Message-Id: <1469459025-23606-3-git-send-email-cota@braap.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
util/qdist.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/util/qdist.c b/util/qdist.c
index eb2236c..cc31140 100644
--- a/util/qdist.c
+++ b/util/qdist.c
@@ -62,8 +62,8 @@ void qdist_add(struct qdist *dist, double x, long count)
if (unlikely(dist->n == dist->size)) {
dist->size *= 2;
- dist->entries = g_realloc(dist->entries,
- sizeof(*dist->entries) * (dist->size));
+ dist->entries = g_realloc_n(dist->entries, dist->size,
+ sizeof(*dist->entries));
}
dist->n++;
entry = &dist->entries[dist->n - 1];
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [PULL 12/25] qdist: return "(empty)" instead of NULL when printing an empty dist
2016-08-02 19:39 [Qemu-devel] [PULL 00/25] Misc QEMU fixes for 2016-08-02 Paolo Bonzini
` (10 preceding siblings ...)
2016-08-02 19:39 ` [Qemu-devel] [PULL 11/25] qdist: use g_realloc_n instead of g_realloc Paolo Bonzini
@ 2016-08-02 19:39 ` Paolo Bonzini
2016-08-02 19:39 ` [Qemu-devel] [PULL 13/25] mptsas: really fix migration compatibility Paolo Bonzini
` (13 subsequent siblings)
25 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2016-08-02 19:39 UTC (permalink / raw)
To: qemu-devel; +Cc: Emilio G. Cota
From: "Emilio G. Cota" <cota@braap.org>
Printf'ing a NULL string is undefined behaviour. Avoid it.
Reported-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Emilio G. Cota <cota@braap.org>
Message-Id: <1469459025-23606-4-git-send-email-cota@braap.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
tests/test-qdist.c | 10 ++++++++--
util/qdist.c | 6 ++++--
2 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/tests/test-qdist.c b/tests/test-qdist.c
index 0298986..9541ce3 100644
--- a/tests/test-qdist.c
+++ b/tests/test-qdist.c
@@ -360,10 +360,16 @@ static void test_none(void)
g_assert(isnan(qdist_xmax(&dist)));
pr = qdist_pr_plain(&dist, 0);
- g_assert(pr == NULL);
+ g_assert_cmpstr(pr, ==, "(empty)");
+ g_free(pr);
pr = qdist_pr_plain(&dist, 2);
- g_assert(pr == NULL);
+ g_assert_cmpstr(pr, ==, "(empty)");
+ g_free(pr);
+
+ pr = qdist_pr(&dist, 0, QDIST_PR_BORDER);
+ g_assert_cmpstr(pr, ==, "(empty)");
+ g_free(pr);
qdist_destroy(&dist);
}
diff --git a/util/qdist.c b/util/qdist.c
index cc31140..41eff08 100644
--- a/util/qdist.c
+++ b/util/qdist.c
@@ -14,6 +14,8 @@
#define NAN (0.0 / 0.0)
#endif
+#define QDIST_EMPTY_STR "(empty)"
+
void qdist_init(struct qdist *dist)
{
dist->entries = g_malloc(sizeof(*dist->entries));
@@ -234,7 +236,7 @@ char *qdist_pr_plain(const struct qdist *dist, size_t n)
char *ret;
if (dist->n == 0) {
- return NULL;
+ return g_strdup(QDIST_EMPTY_STR);
}
qdist_bin__internal(&binned, dist, n);
ret = qdist_pr_internal(&binned);
@@ -309,7 +311,7 @@ char *qdist_pr(const struct qdist *dist, size_t n_bins, uint32_t opt)
GString *s;
if (dist->n == 0) {
- return NULL;
+ return g_strdup(QDIST_EMPTY_STR);
}
s = g_string_new("");
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [PULL 13/25] mptsas: really fix migration compatibility
2016-08-02 19:39 [Qemu-devel] [PULL 00/25] Misc QEMU fixes for 2016-08-02 Paolo Bonzini
` (11 preceding siblings ...)
2016-08-02 19:39 ` [Qemu-devel] [PULL 12/25] qdist: return "(empty)" instead of NULL when printing an empty dist Paolo Bonzini
@ 2016-08-02 19:39 ` Paolo Bonzini
2016-08-02 19:39 ` [Qemu-devel] [PULL 14/25] i2c: fix migration regression introduced by broadcast support Paolo Bonzini
` (12 subsequent siblings)
25 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2016-08-02 19:39 UTC (permalink / raw)
To: qemu-devel
Cc: Markus Armbruster, Marcel Apfelbaum, Michael S . Tsirkin,
Amit Shah, Cao jin
Commit 2e2aa316 removed internal flag msi_in_use, but it
existed in vmstate. Restore it for migration to older QEMU
versions.
Reported-by: Amit Shah <amit.shah@redhat.com>
Suggested-by: Amit Shah <amit.shah@redhat.com>
Cc: Markus Armbruster <armbru@redhat.com>
Cc: Marcel Apfelbaum <marcel@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Amit Shah <amit.shah@redhat.com>
Cc: Cao jin <caoj.fnst@cn.fujitsu.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/scsi/mptsas.c | 4 +++-
hw/scsi/mptsas.h | 2 ++
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/hw/scsi/mptsas.c b/hw/scsi/mptsas.c
index bebe513..0e0a22f 100644
--- a/hw/scsi/mptsas.c
+++ b/hw/scsi/mptsas.c
@@ -1295,6 +1295,8 @@ static void mptsas_scsi_init(PCIDevice *dev, Error **errp)
/* With msi=auto, we fall back to MSI off silently */
error_free(err);
+ /* Only used for migration. */
+ s->msi_in_use = (ret == 0);
}
memory_region_init_io(&s->mmio_io, OBJECT(s), &mptsas_mmio_ops, s,
@@ -1370,7 +1372,7 @@ static const VMStateDescription vmstate_mptsas = {
.post_load = mptsas_post_load,
.fields = (VMStateField[]) {
VMSTATE_PCI_DEVICE(dev, MPTSASState),
- VMSTATE_UNUSED(sizeof(bool)), /* Was msi_in_use */
+ VMSTATE_BOOL(msi_in_use, MPTSASState),
VMSTATE_UINT32(state, MPTSASState),
VMSTATE_UINT8(who_init, MPTSASState),
VMSTATE_UINT8(doorbell_state, MPTSASState),
diff --git a/hw/scsi/mptsas.h b/hw/scsi/mptsas.h
index da014a3..0436a33 100644
--- a/hw/scsi/mptsas.h
+++ b/hw/scsi/mptsas.h
@@ -31,6 +31,8 @@ struct MPTSASState {
OnOffAuto msi;
uint64_t sas_addr;
+ bool msi_in_use;
+
/* Doorbell register */
uint32_t state;
uint8_t who_init;
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [PULL 14/25] i2c: fix migration regression introduced by broadcast support
2016-08-02 19:39 [Qemu-devel] [PULL 00/25] Misc QEMU fixes for 2016-08-02 Paolo Bonzini
` (12 preceding siblings ...)
2016-08-02 19:39 ` [Qemu-devel] [PULL 13/25] mptsas: really fix migration compatibility Paolo Bonzini
@ 2016-08-02 19:39 ` Paolo Bonzini
2016-08-02 19:39 ` [Qemu-devel] [PULL 15/25] nbd: Fix bad flag detection on server Paolo Bonzini
` (11 subsequent siblings)
25 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2016-08-02 19:39 UTC (permalink / raw)
To: qemu-devel; +Cc: Igor Mammedov
From: Igor Mammedov <imammedo@redhat.com>
QEMU fails migration with following error:
qemu-system-x86_64: Missing section footer for i2c_bus
qemu-system-x86_64: load of migration failed: Invalid argument
when migrating from:
qemu-system-x86_64-v2.6.0 -m 256M rhel72.img -M pc-i440fx-2.6
to
qemu-system-x86_64-v2.7.0-rc0 -m 256M rhel72.img -M pc-i440fx-2.6
Regression is added by commit 2293c27f (i2c: implement broadcast write)
Fix it by dropping 'broadcast' VMState introduced by 2293c27f and
reuse broadcast 0x00 address as broadcast flag in bus->saved_address.
Then if there were ongoing broadcast at migration time, set
bus->saved_address to it and at i2c_slave_post_load() time check
for it instead of transfering and using 'broadcast' VMState.
As result of reusing existing saved_address VMState, no compat
glue will be needed to keep forward/backward compatiblity. which
makes fix much less intrusive.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Message-Id: <1469623198-177227-1-git-send-email-imammedo@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/i2c/core.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/hw/i2c/core.c b/hw/i2c/core.c
index abb3efb..4afbe0b 100644
--- a/hw/i2c/core.c
+++ b/hw/i2c/core.c
@@ -17,6 +17,8 @@ struct I2CNode {
QLIST_ENTRY(I2CNode) next;
};
+#define I2C_BROADCAST 0x00
+
struct I2CBus
{
BusState qbus;
@@ -47,6 +49,8 @@ static void i2c_bus_pre_save(void *opaque)
if (!QLIST_EMPTY(&bus->current_devs)) {
if (!bus->broadcast) {
bus->saved_address = QLIST_FIRST(&bus->current_devs)->elt->address;
+ } else {
+ bus->saved_address = I2C_BROADCAST;
}
}
}
@@ -58,7 +62,6 @@ static const VMStateDescription vmstate_i2c_bus = {
.pre_save = i2c_bus_pre_save,
.fields = (VMStateField[]) {
VMSTATE_UINT8(saved_address, I2CBus),
- VMSTATE_BOOL(broadcast, I2CBus),
VMSTATE_END_OF_LIST()
}
};
@@ -93,7 +96,7 @@ int i2c_start_transfer(I2CBus *bus, uint8_t address, int recv)
I2CSlaveClass *sc;
I2CNode *node;
- if (address == 0x00) {
+ if (address == I2C_BROADCAST) {
/*
* This is a broadcast, the current_devs will be all the devices of the
* bus.
@@ -221,7 +224,8 @@ static int i2c_slave_post_load(void *opaque, int version_id)
I2CNode *node;
bus = I2C_BUS(qdev_get_parent_bus(DEVICE(dev)));
- if ((bus->saved_address == dev->address) || (bus->broadcast)) {
+ if ((bus->saved_address == dev->address) ||
+ (bus->saved_address == I2C_BROADCAST)) {
node = g_malloc(sizeof(struct I2CNode));
node->elt = dev;
QLIST_INSERT_HEAD(&bus->current_devs, node, next);
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [PULL 15/25] nbd: Fix bad flag detection on server
2016-08-02 19:39 [Qemu-devel] [PULL 00/25] Misc QEMU fixes for 2016-08-02 Paolo Bonzini
` (13 preceding siblings ...)
2016-08-02 19:39 ` [Qemu-devel] [PULL 14/25] i2c: fix migration regression introduced by broadcast support Paolo Bonzini
@ 2016-08-02 19:39 ` Paolo Bonzini
2016-08-02 19:39 ` [Qemu-devel] [PULL 16/25] nbd: Limit nbdflags to 16 bits Paolo Bonzini
` (10 subsequent siblings)
25 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2016-08-02 19:39 UTC (permalink / raw)
To: qemu-devel
From: Eric Blake <eblake@redhat.com>
Commit ab7c548e added a check for invalid flags, but used an
early return on error instead of properly going through the
cleanup label.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1469129688-22848-2-git-send-email-eblake@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
nbd/server.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/nbd/server.c b/nbd/server.c
index 29e2099..3c1e2b3 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -1057,7 +1057,8 @@ static ssize_t nbd_co_receive_request(NBDRequest *req,
if (request->type & ~NBD_CMD_MASK_COMMAND & ~NBD_CMD_FLAG_FUA) {
LOG("unsupported flags (got 0x%x)",
request->type & ~NBD_CMD_MASK_COMMAND);
- return -EINVAL;
+ rc = -EINVAL;
+ goto out;
}
rc = 0;
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [PULL 16/25] nbd: Limit nbdflags to 16 bits
2016-08-02 19:39 [Qemu-devel] [PULL 00/25] Misc QEMU fixes for 2016-08-02 Paolo Bonzini
` (14 preceding siblings ...)
2016-08-02 19:39 ` [Qemu-devel] [PULL 15/25] nbd: Fix bad flag detection on server Paolo Bonzini
@ 2016-08-02 19:39 ` Paolo Bonzini
2016-08-02 19:39 ` [Qemu-devel] [PULL 17/25] osdep: Document differences in rounding macros Paolo Bonzini
` (9 subsequent siblings)
25 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2016-08-02 19:39 UTC (permalink / raw)
To: qemu-devel; +Cc: Eric Blake, qemu-stable
From: Eric Blake <eblake@redhat.com>
Rather than asserting that nbdflags is within range, just give
it the correct type to begin with :) nbdflags corresponds to
the per-export portion of NBD Protocol "transmission flags", which
is 16 bits in response to NBD_OPT_EXPORT_NAME and NBD_OPT_GO.
Furthermore, upstream NBD has never passed the global flags to
the kernel via ioctl(NBD_SET_FLAGS) (the ioctl was first
introduced in NBD 2.9.22; then a latent bug in NBD 3.1 actually
tried to OR the global flags with the transmission flags, with
the disaster that the addition of NBD_FLAG_NO_ZEROES in 3.9
caused all earlier NBD 3.x clients to treat every export as
read-only; NBD 3.10 and later intentionally clip things to 16
bits to pass only transmission flags). Qemu should follow suit,
since the current two global flags (NBD_FLAG_FIXED_NEWSTYLE
and NBD_FLAG_NO_ZEROES) have no impact on the kernel's behavior
during transmission.
CC: qemu-stable@nongnu.org
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1469129688-22848-3-git-send-email-eblake@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
block/nbd-client.h | 2 +-
include/block/nbd.h | 6 +++---
nbd/client.c | 28 +++++++++++++++-------------
nbd/server.c | 10 ++++------
qemu-nbd.c | 4 ++--
5 files changed, 25 insertions(+), 25 deletions(-)
diff --git a/block/nbd-client.h b/block/nbd-client.h
index fa9817b..044aca4 100644
--- a/block/nbd-client.h
+++ b/block/nbd-client.h
@@ -20,7 +20,7 @@
typedef struct NbdClientSession {
QIOChannelSocket *sioc; /* The master data channel */
QIOChannel *ioc; /* The current I/O channel which may differ (eg TLS) */
- uint32_t nbdflags;
+ uint16_t nbdflags;
off_t size;
CoMutex send_mutex;
diff --git a/include/block/nbd.h b/include/block/nbd.h
index cb91820..1897557 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -90,11 +90,11 @@ ssize_t nbd_wr_syncv(QIOChannel *ioc,
size_t niov,
size_t length,
bool do_read);
-int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint32_t *flags,
+int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint16_t *flags,
QCryptoTLSCreds *tlscreds, const char *hostname,
QIOChannel **outioc,
off_t *size, Error **errp);
-int nbd_init(int fd, QIOChannelSocket *sioc, uint32_t flags, off_t size);
+int nbd_init(int fd, QIOChannelSocket *sioc, uint16_t flags, off_t size);
ssize_t nbd_send_request(QIOChannel *ioc, struct nbd_request *request);
ssize_t nbd_receive_reply(QIOChannel *ioc, struct nbd_reply *reply);
int nbd_client(int fd);
@@ -104,7 +104,7 @@ typedef struct NBDExport NBDExport;
typedef struct NBDClient NBDClient;
NBDExport *nbd_export_new(BlockBackend *blk, off_t dev_offset, off_t size,
- uint32_t nbdflags, void (*close)(NBDExport *),
+ uint16_t nbdflags, void (*close)(NBDExport *),
Error **errp);
void nbd_export_close(NBDExport *exp);
void nbd_export_get(NBDExport *exp);
diff --git a/nbd/client.c b/nbd/client.c
index 78a7195..a92f1e2 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -408,7 +408,7 @@ static QIOChannel *nbd_receive_starttls(QIOChannel *ioc,
}
-int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint32_t *flags,
+int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint16_t *flags,
QCryptoTLSCreds *tlscreds, const char *hostname,
QIOChannel **outioc,
off_t *size, Error **errp)
@@ -468,7 +468,6 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint32_t *flags,
uint32_t opt;
uint32_t namesize;
uint16_t globalflags;
- uint16_t exportflags;
bool fixedNewStyle = false;
if (read_sync(ioc, &globalflags, sizeof(globalflags)) !=
@@ -477,7 +476,6 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint32_t *flags,
goto fail;
}
globalflags = be16_to_cpu(globalflags);
- *flags = globalflags << 16;
TRACE("Global flags are %" PRIx32, globalflags);
if (globalflags & NBD_FLAG_FIXED_NEWSTYLE) {
fixedNewStyle = true;
@@ -545,17 +543,15 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint32_t *flags,
goto fail;
}
*size = be64_to_cpu(s);
- TRACE("Size is %" PRIu64, *size);
- if (read_sync(ioc, &exportflags, sizeof(exportflags)) !=
- sizeof(exportflags)) {
+ if (read_sync(ioc, flags, sizeof(*flags)) != sizeof(*flags)) {
error_setg(errp, "Failed to read export flags");
goto fail;
}
- exportflags = be16_to_cpu(exportflags);
- *flags |= exportflags;
- TRACE("Export flags are %" PRIx16, exportflags);
+ be16_to_cpus(flags);
} else if (magic == NBD_CLIENT_MAGIC) {
+ uint32_t oldflags;
+
if (name) {
error_setg(errp, "Server does not support export names");
goto fail;
@@ -572,16 +568,22 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint32_t *flags,
*size = be64_to_cpu(s);
TRACE("Size is %" PRIu64, *size);
- if (read_sync(ioc, flags, sizeof(*flags)) != sizeof(*flags)) {
+ if (read_sync(ioc, &oldflags, sizeof(oldflags)) != sizeof(oldflags)) {
error_setg(errp, "Failed to read export flags");
goto fail;
}
- *flags = be32_to_cpu(*flags);
+ be32_to_cpus(&oldflags);
+ if (oldflags & ~0xffff) {
+ error_setg(errp, "Unexpected export flags %0x" PRIx32, oldflags);
+ goto fail;
+ }
+ *flags = oldflags;
} else {
error_setg(errp, "Bad magic received");
goto fail;
}
+ TRACE("Size is %" PRIu64 ", export flags %" PRIx16, *size, *flags);
if (read_sync(ioc, &buf, 124) != 124) {
error_setg(errp, "Failed to read reserved block");
goto fail;
@@ -593,7 +595,7 @@ fail:
}
#ifdef __linux__
-int nbd_init(int fd, QIOChannelSocket *sioc, uint32_t flags, off_t size)
+int nbd_init(int fd, QIOChannelSocket *sioc, uint16_t flags, off_t size)
{
unsigned long sectors = size / BDRV_SECTOR_SIZE;
if (size / BDRV_SECTOR_SIZE != sectors) {
@@ -689,7 +691,7 @@ int nbd_disconnect(int fd)
}
#else
-int nbd_init(int fd, QIOChannelSocket *ioc, uint32_t flags, off_t size)
+int nbd_init(int fd, QIOChannelSocket *ioc, uint16_t flags, off_t size)
{
return -ENOTSUP;
}
diff --git a/nbd/server.c b/nbd/server.c
index 3c1e2b3..80fbb4d 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -63,7 +63,7 @@ struct NBDExport {
char *name;
off_t dev_offset;
off_t size;
- uint32_t nbdflags;
+ uint16_t nbdflags;
QTAILQ_HEAD(, NBDClient) clients;
QTAILQ_ENTRY(NBDExport) next;
@@ -544,8 +544,8 @@ static coroutine_fn int nbd_negotiate(NBDClientNewData *data)
NBDClient *client = data->client;
char buf[8 + 8 + 8 + 128];
int rc;
- const int myflags = (NBD_FLAG_HAS_FLAGS | NBD_FLAG_SEND_TRIM |
- NBD_FLAG_SEND_FLUSH | NBD_FLAG_SEND_FUA);
+ const uint16_t myflags = (NBD_FLAG_HAS_FLAGS | NBD_FLAG_SEND_TRIM |
+ NBD_FLAG_SEND_FLUSH | NBD_FLAG_SEND_FUA);
bool oldStyle;
/* Old style negotiation header without options
@@ -575,7 +575,6 @@ static coroutine_fn int nbd_negotiate(NBDClientNewData *data)
oldStyle = client->exp != NULL && !client->tlscreds;
if (oldStyle) {
- assert ((client->exp->nbdflags & ~65535) == 0);
TRACE("advertising size %" PRIu64 " and flags %x",
client->exp->size, client->exp->nbdflags | myflags);
stq_be_p(buf + 8, NBD_CLIENT_MAGIC);
@@ -606,7 +605,6 @@ static coroutine_fn int nbd_negotiate(NBDClientNewData *data)
goto fail;
}
- assert ((client->exp->nbdflags & ~65535) == 0);
TRACE("advertising size %" PRIu64 " and flags %x",
client->exp->size, client->exp->nbdflags | myflags);
stq_be_p(buf + 18, client->exp->size);
@@ -810,7 +808,7 @@ static void nbd_eject_notifier(Notifier *n, void *data)
}
NBDExport *nbd_export_new(BlockBackend *blk, off_t dev_offset, off_t size,
- uint32_t nbdflags, void (*close)(NBDExport *),
+ uint16_t nbdflags, void (*close)(NBDExport *),
Error **errp)
{
NBDExport *exp = g_malloc0(sizeof(NBDExport));
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 321f02b..e3571c2 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -251,7 +251,7 @@ static void *nbd_client_thread(void *arg)
{
char *device = arg;
off_t size;
- uint32_t nbdflags;
+ uint16_t nbdflags;
QIOChannelSocket *sioc;
int fd;
int ret;
@@ -465,7 +465,7 @@ int main(int argc, char **argv)
BlockBackend *blk;
BlockDriverState *bs;
off_t dev_offset = 0;
- uint32_t nbdflags = 0;
+ uint16_t nbdflags = 0;
bool disconnect = false;
const char *bindto = "0.0.0.0";
const char *port = NULL;
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [PULL 17/25] osdep: Document differences in rounding macros
2016-08-02 19:39 [Qemu-devel] [PULL 00/25] Misc QEMU fixes for 2016-08-02 Paolo Bonzini
` (15 preceding siblings ...)
2016-08-02 19:39 ` [Qemu-devel] [PULL 16/25] nbd: Limit nbdflags to 16 bits Paolo Bonzini
@ 2016-08-02 19:39 ` Paolo Bonzini
2016-08-02 19:39 ` [Qemu-devel] [PULL 18/25] block: Cater to iscsi with non-power-of-2 discard Paolo Bonzini
` (8 subsequent siblings)
25 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2016-08-02 19:39 UTC (permalink / raw)
To: qemu-devel
From: Eric Blake <eblake@redhat.com>
Make it obvious which macros are safe in which situations.
Useful since QEMU_ALIGN_UP and ROUND_UP both purport to do
the same thing, but differ on whether the alignment must be
a power of 2.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1469129688-22848-4-git-send-email-eblake@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/qemu/osdep.h | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index d7c111d..9e9fa61 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -158,7 +158,8 @@ extern int daemon(int, int);
/* Round number down to multiple */
#define QEMU_ALIGN_DOWN(n, m) ((n) / (m) * (m))
-/* Round number up to multiple */
+/* Round number up to multiple. Safe when m is not a power of 2 (see
+ * ROUND_UP for a faster version when a power of 2 is guaranteed) */
#define QEMU_ALIGN_UP(n, m) QEMU_ALIGN_DOWN((n) + (m) - 1, (m))
/* Check if n is a multiple of m */
@@ -175,6 +176,9 @@ extern int daemon(int, int);
/* Check if pointer p is n-bytes aligned */
#define QEMU_PTR_IS_ALIGNED(p, n) QEMU_IS_ALIGNED((uintptr_t)(p), (n))
+/* Round number up to multiple. Requires that d be a power of 2 (see
+ * QEMU_ALIGN_UP for a safer but slower version on arbitrary
+ * numbers) */
#ifndef ROUND_UP
#define ROUND_UP(n,d) (((n) + (d) - 1) & -(d))
#endif
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [PULL 18/25] block: Cater to iscsi with non-power-of-2 discard
2016-08-02 19:39 [Qemu-devel] [PULL 00/25] Misc QEMU fixes for 2016-08-02 Paolo Bonzini
` (16 preceding siblings ...)
2016-08-02 19:39 ` [Qemu-devel] [PULL 17/25] osdep: Document differences in rounding macros Paolo Bonzini
@ 2016-08-02 19:39 ` Paolo Bonzini
2016-08-02 19:39 ` [Qemu-devel] [PULL 19/25] fw_cfg: Make base type "fw_cfg" abstract Paolo Bonzini
` (7 subsequent siblings)
25 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2016-08-02 19:39 UTC (permalink / raw)
To: qemu-devel
From: Eric Blake <eblake@redhat.com>
Dell Equallogic iSCSI SANs have a very unusual advertised geometry:
$ iscsi-inq -e 1 -c $((0xb0)) iscsi://XXX/0
wsnz:0
maximum compare and write length:1
optimal transfer length granularity:0
maximum transfer length:0
optimal transfer length:0
maximum prefetch xdread xdwrite transfer length:0
maximum unmap lba count:30720
maximum unmap block descriptor count:2
optimal unmap granularity:30720
ugavalid:1
unmap granularity alignment:0
maximum write same length:30720
which says that both the maximum and the optimal discard size
is 15M. It is not immediately apparent if the device allows
discard requests not aligned to the optimal size, nor if it
allows discards at a finer granularity than the optimal size.
I tried to find details in the SCSI Commands Reference Manual
Rev. A on what valid values of maximum and optimal sizes are
permitted, but while that document mentions a "Block Limits
VPD Page", I couldn't actually find documentation of that page
or what values it would have, or if a SCSI device has an
advertisement of its minimal unmap granularity. So it is not
obvious to me whether the Dell Equallogic device is compliance
with the SCSI specification.
Fortunately, it is easy enough to support non-power-of-2 sizing,
even if it means we are less efficient than truly possible when
targetting that device (for example, it means that we refuse to
unmap anything that is not a multiple of 15M and aligned to a
15M boundary, even if the device truly does support a smaller
granularity where unmapping actually works).
Reported-by: Peter Lieven <pl@kamp.de>
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1469129688-22848-5-git-send-email-eblake@redhat.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
block/io.c | 15 +++++++++------
include/block/block_int.h | 37 ++++++++++++++++++++-----------------
2 files changed, 29 insertions(+), 23 deletions(-)
diff --git a/block/io.c b/block/io.c
index 7323f0f..d5493ba 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1180,10 +1180,11 @@ static int coroutine_fn bdrv_co_do_pwrite_zeroes(BlockDriverState *bs,
int alignment = MAX(bs->bl.pwrite_zeroes_alignment,
bs->bl.request_alignment);
- assert(is_power_of_2(alignment));
- head = offset & (alignment - 1);
- tail = (offset + count) & (alignment - 1);
- max_write_zeroes &= ~(alignment - 1);
+ assert(alignment % bs->bl.request_alignment == 0);
+ head = offset % alignment;
+ tail = (offset + count) % alignment;
+ max_write_zeroes = QEMU_ALIGN_DOWN(max_write_zeroes, alignment);
+ assert(max_write_zeroes >= bs->bl.request_alignment);
while (count > 0 && !ret) {
int num = count;
@@ -2429,9 +2430,10 @@ int coroutine_fn bdrv_co_pdiscard(BlockDriverState *bs, int64_t offset,
/* Discard is advisory, so ignore any unaligned head or tail */
align = MAX(bs->bl.pdiscard_alignment, bs->bl.request_alignment);
- assert(is_power_of_2(align));
- head = MIN(count, -offset & (align - 1));
+ assert(align % bs->bl.request_alignment == 0);
+ head = offset % align;
if (head) {
+ head = MIN(count, align - head);
count -= head;
offset += head;
}
@@ -2449,6 +2451,7 @@ int coroutine_fn bdrv_co_pdiscard(BlockDriverState *bs, int64_t offset,
max_pdiscard = QEMU_ALIGN_DOWN(MIN_NON_ZERO(bs->bl.max_pdiscard, INT_MAX),
align);
+ assert(max_pdiscard);
while (count > 0) {
int ret;
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 1fe0fd9..47665be 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -330,36 +330,39 @@ typedef struct BlockLimits {
* otherwise. */
uint32_t request_alignment;
- /* maximum number of bytes that can be discarded at once (since it
- * is signed, it must be < 2G, if set), should be multiple of
+ /* Maximum number of bytes that can be discarded at once (since it
+ * is signed, it must be < 2G, if set). Must be multiple of
* pdiscard_alignment, but need not be power of 2. May be 0 if no
* inherent 32-bit limit */
int32_t max_pdiscard;
- /* optimal alignment for discard requests in bytes, must be power
- * of 2, less than max_pdiscard if that is set, and multiple of
- * bl.request_alignment. May be 0 if bl.request_alignment is good
- * enough */
+ /* Optimal alignment for discard requests in bytes. A power of 2
+ * is best but not mandatory. Must be a multiple of
+ * bl.request_alignment, and must be less than max_pdiscard if
+ * that is set. May be 0 if bl.request_alignment is good enough */
uint32_t pdiscard_alignment;
- /* maximum number of bytes that can zeroized at once (since it is
- * signed, it must be < 2G, if set), should be multiple of
+ /* Maximum number of bytes that can zeroized at once (since it is
+ * signed, it must be < 2G, if set). Must be multiple of
* pwrite_zeroes_alignment. May be 0 if no inherent 32-bit limit */
int32_t max_pwrite_zeroes;
- /* optimal alignment for write zeroes requests in bytes, must be
- * power of 2, less than max_pwrite_zeroes if that is set, and
- * multiple of bl.request_alignment. May be 0 if
- * bl.request_alignment is good enough */
+ /* Optimal alignment for write zeroes requests in bytes. A power
+ * of 2 is best but not mandatory. Must be a multiple of
+ * bl.request_alignment, and must be less than max_pwrite_zeroes
+ * if that is set. May be 0 if bl.request_alignment is good
+ * enough */
uint32_t pwrite_zeroes_alignment;
- /* optimal transfer length in bytes (must be power of 2, and
- * multiple of bl.request_alignment), or 0 if no preferred size */
+ /* Optimal transfer length in bytes. A power of 2 is best but not
+ * mandatory. Must be a multiple of bl.request_alignment, or 0 if
+ * no preferred size */
uint32_t opt_transfer;
- /* maximal transfer length in bytes (need not be power of 2, but
- * should be multiple of opt_transfer), or 0 for no 32-bit limit.
- * For now, anything larger than INT_MAX is clamped down. */
+ /* Maximal transfer length in bytes. Need not be power of 2, but
+ * must be multiple of opt_transfer and bl.request_alignment, or 0
+ * for no 32-bit limit. For now, anything larger than INT_MAX is
+ * clamped down. */
uint32_t max_transfer;
/* memory alignment, in bytes so that no bounce buffer is needed */
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [PULL 19/25] fw_cfg: Make base type "fw_cfg" abstract
2016-08-02 19:39 [Qemu-devel] [PULL 00/25] Misc QEMU fixes for 2016-08-02 Paolo Bonzini
` (17 preceding siblings ...)
2016-08-02 19:39 ` [Qemu-devel] [PULL 18/25] block: Cater to iscsi with non-power-of-2 discard Paolo Bonzini
@ 2016-08-02 19:39 ` Paolo Bonzini
2016-08-02 19:39 ` [Qemu-devel] [PULL 20/25] apic: fix broken migration for kvm-apic Paolo Bonzini
` (6 subsequent siblings)
25 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2016-08-02 19:39 UTC (permalink / raw)
To: qemu-devel; +Cc: Markus Armbruster
From: Markus Armbruster <armbru@redhat.com>
Missed when commit 5712db6 split off "fw_cfg_io" and "fw_cfg_mem".
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1469777353-9383-1-git-send-email-armbru@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/nvram/fw_cfg.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index 2873030..f10d5ec 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -990,6 +990,7 @@ static void fw_cfg_class_init(ObjectClass *klass, void *data)
static const TypeInfo fw_cfg_info = {
.name = TYPE_FW_CFG,
.parent = TYPE_SYS_BUS_DEVICE,
+ .abstract = true,
.instance_size = sizeof(FWCfgState),
.class_init = fw_cfg_class_init,
};
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [PULL 20/25] apic: fix broken migration for kvm-apic
2016-08-02 19:39 [Qemu-devel] [PULL 00/25] Misc QEMU fixes for 2016-08-02 Paolo Bonzini
` (18 preceding siblings ...)
2016-08-02 19:39 ` [Qemu-devel] [PULL 19/25] fw_cfg: Make base type "fw_cfg" abstract Paolo Bonzini
@ 2016-08-02 19:39 ` Paolo Bonzini
2016-08-02 19:39 ` [Qemu-devel] [PULL 21/25] x86: ioapic: ignore level irq during processing Paolo Bonzini
` (5 subsequent siblings)
25 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2016-08-02 19:39 UTC (permalink / raw)
To: qemu-devel; +Cc: Igor Mammedov
From: Igor Mammedov <imammedo@redhat.com>
commit f6e98444 (apic: Use apic_id as apic's migration instance_id)
breaks migration when in kernel irqchip is used for 2.6 and older
machine types.
It applies compat property only for userspace 'apic' type
instead of applying it to all apic types inherited from
'apic-common' type as it was supposed to do.
Fix it by setting compat property 'legacy-instance-id' for
'apic-common' type which affects inherited types (i.e. not
only 'apic' but also 'kvm-apic' types)
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Message-Id: <1469800542-11402-1-git-send-email-imammedo@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/hw/i386/pc.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index c87c5c1..74c175c 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -388,7 +388,7 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
.value = "off",\
},\
{\
- .driver = "apic",\
+ .driver = "apic-common",\
.property = "legacy-instance-id",\
.value = "on",\
},
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [PULL 21/25] x86: ioapic: ignore level irq during processing
2016-08-02 19:39 [Qemu-devel] [PULL 00/25] Misc QEMU fixes for 2016-08-02 Paolo Bonzini
` (19 preceding siblings ...)
2016-08-02 19:39 ` [Qemu-devel] [PULL 20/25] apic: fix broken migration for kvm-apic Paolo Bonzini
@ 2016-08-02 19:39 ` Paolo Bonzini
2016-08-02 19:39 ` [Qemu-devel] [PULL 22/25] x86: ioapic: add support for explicit EOI Paolo Bonzini
` (4 subsequent siblings)
25 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2016-08-02 19:39 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Xu
From: Peter Xu <peterx@redhat.com>
For level triggered interrupts, we will get Remote IRR bit cleared after
guest kernel finished processing specific request. Before that, we
should ignore the same interrupt from triggering again.
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <1469974685-4144-1-git-send-email-peterx@redhat.com>
[Push new "if" up so that it covers KVM split irqchip as well. - Paolo]
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/intc/ioapic.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/hw/intc/ioapic.c b/hw/intc/ioapic.c
index 2d3282a..a00d882 100644
--- a/hw/intc/ioapic.c
+++ b/hw/intc/ioapic.c
@@ -117,21 +117,25 @@ static void ioapic_service(IOAPICCommonState *s)
s->ioredtbl[i] |= IOAPIC_LVT_REMOTE_IRR;
}
+ if (coalesce) {
+ /* We are level triggered interrupts, and the
+ * guest should be still working on previous one,
+ * so skip it. */
+ continue;
+ }
+
#ifdef CONFIG_KVM
if (kvm_irqchip_is_split()) {
if (info.trig_mode == IOAPIC_TRIGGER_EDGE) {
kvm_set_irq(kvm_state, i, 1);
kvm_set_irq(kvm_state, i, 0);
} else {
- if (!coalesce) {
- kvm_set_irq(kvm_state, i, 1);
- }
+ kvm_set_irq(kvm_state, i, 1);
}
continue;
}
-#else
- (void)coalesce;
#endif
+
/* No matter whether IR is enabled, we translate
* the IOAPIC message into a MSI one, and its
* address space will decide whether we need a
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [PULL 22/25] x86: ioapic: add support for explicit EOI
2016-08-02 19:39 [Qemu-devel] [PULL 00/25] Misc QEMU fixes for 2016-08-02 Paolo Bonzini
` (20 preceding siblings ...)
2016-08-02 19:39 ` [Qemu-devel] [PULL 21/25] x86: ioapic: ignore level irq during processing Paolo Bonzini
@ 2016-08-02 19:39 ` Paolo Bonzini
2016-08-02 19:39 ` [Qemu-devel] [PULL 23/25] Reorganize help output of '-display' option Paolo Bonzini
` (3 subsequent siblings)
25 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2016-08-02 19:39 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Xu
From: Peter Xu <peterx@redhat.com>
Some old Linux kernels (upstream before v4.0), or any released RHEL
kernels has problem in sending APIC EOI when IR is enabled. Meanwhile,
many of them only support explicit EOI for IOAPIC, which is only
introduced in IOAPIC version 0x20. This patch provide a way to boost
QEMU IOAPIC to version 0x20, in order for QEMU to correctly receive EOI
messages.
Without boosting IOAPIC version to 0x20, kernels before commit d32932d
("x86/irq: Convert IOAPIC to use hierarchical irqdomain interfaces")
will have trouble enabling both IR and level-triggered interrupt devices
(like e1000).
To upgrade IOAPIC to version 0x20, we need to specify:
-global ioapic.version=0x20
To be compatible with old systems, 0x11 will still be the default IOAPIC
version. Here 0x11 and 0x20 are the only versions to be supported.
One thing to mention: this patch only applies to emulated IOAPIC. It
does not affect kernel IOAPIC behavior.
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <1470059959-372-1-git-send-email-peterx@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/intc/ioapic.c | 22 +++++++++++++++++++++-
include/hw/i386/ioapic_internal.h | 4 ++--
2 files changed, 23 insertions(+), 3 deletions(-)
diff --git a/hw/intc/ioapic.c b/hw/intc/ioapic.c
index a00d882..31791b0 100644
--- a/hw/intc/ioapic.c
+++ b/hw/intc/ioapic.c
@@ -21,6 +21,7 @@
*/
#include "qemu/osdep.h"
+#include "qemu/error-report.h"
#include "monitor/monitor.h"
#include "hw/hw.h"
#include "hw/i386/pc.h"
@@ -269,7 +270,7 @@ ioapic_mem_read(void *opaque, hwaddr addr, unsigned int size)
val = s->id << IOAPIC_ID_SHIFT;
break;
case IOAPIC_REG_VER:
- val = IOAPIC_VERSION |
+ val = s->version |
((IOAPIC_NUM_PINS - 1) << IOAPIC_VER_ENTRIES_SHIFT);
break;
default:
@@ -358,6 +359,13 @@ ioapic_mem_write(void *opaque, hwaddr addr, uint64_t val,
}
}
break;
+ case IOAPIC_EOI:
+ /* Explicit EOI is only supported for IOAPIC version 0x20 */
+ if (size != 4 || s->version != 0x20) {
+ break;
+ }
+ ioapic_eoi_broadcast(val);
+ break;
}
ioapic_update_kvm_routes(s);
@@ -391,6 +399,12 @@ static void ioapic_realize(DeviceState *dev, Error **errp)
{
IOAPICCommonState *s = IOAPIC_COMMON(dev);
+ if (s->version != 0x11 && s->version != 0x20) {
+ error_report("IOAPIC only supports version 0x11 or 0x20 "
+ "(default: 0x11).");
+ exit(1);
+ }
+
memory_region_init_io(&s->io_memory, OBJECT(s), &ioapic_io_ops, s,
"ioapic", 0x1000);
@@ -401,6 +415,11 @@ static void ioapic_realize(DeviceState *dev, Error **errp)
qemu_add_machine_init_done_notifier(&s->machine_done);
}
+static Property ioapic_properties[] = {
+ DEFINE_PROP_UINT8("version", IOAPICCommonState, version, 0x11),
+ DEFINE_PROP_END_OF_LIST(),
+};
+
static void ioapic_class_init(ObjectClass *klass, void *data)
{
IOAPICCommonClass *k = IOAPIC_COMMON_CLASS(klass);
@@ -408,6 +427,7 @@ static void ioapic_class_init(ObjectClass *klass, void *data)
k->realize = ioapic_realize;
dc->reset = ioapic_reset_common;
+ dc->props = ioapic_properties;
}
static const TypeInfo ioapic_info = {
diff --git a/include/hw/i386/ioapic_internal.h b/include/hw/i386/ioapic_internal.h
index d89ea1b..a11d86d 100644
--- a/include/hw/i386/ioapic_internal.h
+++ b/include/hw/i386/ioapic_internal.h
@@ -29,8 +29,6 @@
#define MAX_IOAPICS 1
-#define IOAPIC_VERSION 0x11
-
#define IOAPIC_LVT_DEST_SHIFT 56
#define IOAPIC_LVT_DEST_IDX_SHIFT 48
#define IOAPIC_LVT_MASKED_SHIFT 16
@@ -71,6 +69,7 @@
#define IOAPIC_IOREGSEL 0x00
#define IOAPIC_IOWIN 0x10
+#define IOAPIC_EOI 0x40
#define IOAPIC_REG_ID 0x00
#define IOAPIC_REG_VER 0x01
@@ -109,6 +108,7 @@ struct IOAPICCommonState {
uint32_t irr;
uint64_t ioredtbl[IOAPIC_NUM_PINS];
Notifier machine_done;
+ uint8_t version;
};
void ioapic_reset_common(DeviceState *dev);
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [PULL 23/25] Reorganize help output of '-display' option
2016-08-02 19:39 [Qemu-devel] [PULL 00/25] Misc QEMU fixes for 2016-08-02 Paolo Bonzini
` (21 preceding siblings ...)
2016-08-02 19:39 ` [Qemu-devel] [PULL 22/25] x86: ioapic: add support for explicit EOI Paolo Bonzini
@ 2016-08-02 19:39 ` Paolo Bonzini
2016-08-02 19:39 ` [Qemu-devel] [PULL 24/25] qdev: Fix use after free in qdev_init_nofail error path Paolo Bonzini
` (2 subsequent siblings)
25 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2016-08-02 19:39 UTC (permalink / raw)
To: qemu-devel; +Cc: Robert Ho
From: Robert Ho <robert.hu@intel.com>
The '-display' help information is not very correct. This patch sort
it a little.
Also, in its help information, reveals what implicit display option
will be chosen if no definition.
Signed-off-by: Robert Ho <robert.hu@intel.com>
Message-Id: <1469528231-26206-1-git-send-email-robert.hu@intel.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
qemu-options.hx | 29 ++++++++++++++++++++++-------
1 file changed, 22 insertions(+), 7 deletions(-)
diff --git a/qemu-options.hx b/qemu-options.hx
index 8e0d9a5..a71aaf8 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -927,10 +927,25 @@ ETEXI
DEF("display", HAS_ARG, QEMU_OPTION_display,
"-display sdl[,frame=on|off][,alt_grab=on|off][,ctrl_grab=on|off]\n"
- " [,window_close=on|off]|curses|none|\n"
- " gtk[,grab_on_hover=on|off]|\n"
- " vnc=<display>[,<optargs>]\n"
- " select display type\n", QEMU_ARCH_ALL)
+ " [,window_close=on|off][,gl=on|off]|curses|none|\n"
+ "-display gtk[,grab_on_hover=on|off][,gl=on|off]|\n"
+ "-display vnc=<display>[,<optargs>]\n"
+ "-display curses\n"
+ "-display none"
+ " select display type\n"
+ "The default display is equivalent to\n"
+#if defined(CONFIG_GTK)
+ "\t\"-display gtk\"\n"
+#elif defined(CONFIG_SDL)
+ "\t\"-display sdl\"\n"
+#elif defined(CONFIG_COCOA)
+ "\t\"-display cocoa\"\n"
+#elif defined(CONFIG_VNC)
+ "\t\"-vnc localhost:0,to=99,id=default\"\n"
+#else
+ "\t\"-display none\"\n"
+#endif
+ , QEMU_ARCH_ALL)
STEXI
@item -display @var{type}
@findex -display
@@ -977,7 +992,7 @@ the console and monitor.
ETEXI
DEF("curses", 0, QEMU_OPTION_curses,
- "-curses use a curses/ncurses interface instead of SDL\n",
+ "-curses shorthand for -display curses\n",
QEMU_ARCH_ALL)
STEXI
@item -curses
@@ -1027,7 +1042,7 @@ Disable SDL window close capability.
ETEXI
DEF("sdl", 0, QEMU_OPTION_sdl,
- "-sdl enable SDL\n", QEMU_ARCH_ALL)
+ "-sdl shorthand for -display sdl\n", QEMU_ARCH_ALL)
STEXI
@item -sdl
@findex -sdl
@@ -1224,7 +1239,7 @@ Set the initial graphical resolution and depth (PPC, SPARC only).
ETEXI
DEF("vnc", HAS_ARG, QEMU_OPTION_vnc ,
- "-vnc display start a VNC server on display\n", QEMU_ARCH_ALL)
+ "-vnc <display> shorthand for -display vnc=<display>\n", QEMU_ARCH_ALL)
STEXI
@item -vnc @var{display}[,@var{option}[,@var{option}[,...]]]
@findex -vnc
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [PULL 24/25] qdev: Fix use after free in qdev_init_nofail error path
2016-08-02 19:39 [Qemu-devel] [PULL 00/25] Misc QEMU fixes for 2016-08-02 Paolo Bonzini
` (22 preceding siblings ...)
2016-08-02 19:39 ` [Qemu-devel] [PULL 23/25] Reorganize help output of '-display' option Paolo Bonzini
@ 2016-08-02 19:39 ` Paolo Bonzini
2016-08-02 19:39 ` [Qemu-devel] [PULL 25/25] util: Fix assertion in iov_copy() upon zero 'bytes' and non-zero 'offset' Paolo Bonzini
2016-08-03 10:52 ` [Qemu-devel] [PULL 00/25] Misc QEMU fixes for 2016-08-02 Peter Maydell
25 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2016-08-02 19:39 UTC (permalink / raw)
To: qemu-devel; +Cc: Fam Zheng, Igor Mammedov
From: Fam Zheng <famz@redhat.com>
Since 69382d8b (qdev: Fix object reference leak in case device.realize()
fails), object_property_set_bool could release the object. The error
path wants the type name, so hold an reference before realizing it.
Cc: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
Message-Id: <1470109301-12966-1-git-send-email-famz@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/core/qdev.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index ee4a083..5783442 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -354,12 +354,14 @@ void qdev_init_nofail(DeviceState *dev)
assert(!dev->realized);
+ object_ref(OBJECT(dev));
object_property_set_bool(OBJECT(dev), true, "realized", &err);
if (err) {
error_reportf_err(err, "Initialization of device %s failed: ",
object_get_typename(OBJECT(dev)));
exit(1);
}
+ object_unref(OBJECT(dev));
}
void qdev_machine_creation_done(void)
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [Qemu-devel] [PULL 25/25] util: Fix assertion in iov_copy() upon zero 'bytes' and non-zero 'offset'
2016-08-02 19:39 [Qemu-devel] [PULL 00/25] Misc QEMU fixes for 2016-08-02 Paolo Bonzini
` (23 preceding siblings ...)
2016-08-02 19:39 ` [Qemu-devel] [PULL 24/25] qdev: Fix use after free in qdev_init_nofail error path Paolo Bonzini
@ 2016-08-02 19:39 ` Paolo Bonzini
2016-08-03 10:52 ` [Qemu-devel] [PULL 00/25] Misc QEMU fixes for 2016-08-02 Peter Maydell
25 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2016-08-02 19:39 UTC (permalink / raw)
To: qemu-devel; +Cc: Shmulik Ladkani
From: Shmulik Ladkani <shmulik.ladkani@ravellosystems.com>
In cases where iov_copy() is passed with zero 'bytes' argument and a
non-zero 'offset' argument, nothing gets copied - as expected.
However no copy iterations are performed, so 'offset' is left
unaltered, leading to the final assert(offset == 0) to fail.
Instead, change the loop condition to continue as long as 'offset || bytes',
similar to other iov_* functions.
This ensures 'offset' gets zeroed (even if no actual copy is made),
unless it is beyond end of source iov - which is asserted.
Signed-off-by: Shmulik Ladkani <shmulik.ladkani@ravellosystems.com>
Message-Id: <1470130880-1050-1-git-send-email-shmulik.ladkani@oracle.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
util/iov.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/util/iov.c b/util/iov.c
index 003fcce..74e6ca8 100644
--- a/util/iov.c
+++ b/util/iov.c
@@ -247,7 +247,8 @@ unsigned iov_copy(struct iovec *dst_iov, unsigned int dst_iov_cnt,
{
size_t len;
unsigned int i, j;
- for (i = 0, j = 0; i < iov_cnt && j < dst_iov_cnt && bytes; i++) {
+ for (i = 0, j = 0;
+ i < iov_cnt && j < dst_iov_cnt && (offset || bytes); i++) {
if (offset >= iov[i].iov_len) {
offset -= iov[i].iov_len;
continue;
--
2.7.4
^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PULL 10/25] qdist: fix memory leak during binning
2016-08-02 19:39 ` [Qemu-devel] [PULL 10/25] qdist: fix memory leak during binning Paolo Bonzini
@ 2016-08-02 21:13 ` Marc-André Lureau
0 siblings, 0 replies; 29+ messages in thread
From: Marc-André Lureau @ 2016-08-02 21:13 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel; +Cc: Emilio G. Cota
Hi
On Tue, Aug 2, 2016 at 11:53 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
> From: "Emilio G. Cota" <cota@braap.org>
>
> In qdist_bin__internal(), to->entries is initialized to a 1-element array,
> which we then leak when n == from->n. Fix it.
>
> Signed-off-by: Emilio G. Cota <cota@braap.org>
> Message-Id: <1469459025-23606-2-git-send-email-cota@braap.org>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> util/qdist.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/util/qdist.c b/util/qdist.c
> index 56f5738..eb2236c 100644
> --- a/util/qdist.c
> +++ b/util/qdist.c
> @@ -188,7 +188,7 @@ void qdist_bin__internal(struct qdist *to, const
> struct qdist *from, size_t n)
> }
> }
> /* they're equally spaced, so copy the dist and bail out */
> - to->entries = g_new(struct qdist_entry, from->n);
> + to->entries = g_realloc_n(to->entries, n, sizeof(*to->entries));
>
I sent that patch earlier in the leak series, if it's still time, please:
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> to->n = from->n;
> memcpy(to->entries, from->entries, sizeof(*to->entries) * to->n);
> return;
> --
> 2.7.4
>
>
>
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PULL 00/25] Misc QEMU fixes for 2016-08-02
2016-08-02 19:39 [Qemu-devel] [PULL 00/25] Misc QEMU fixes for 2016-08-02 Paolo Bonzini
` (24 preceding siblings ...)
2016-08-02 19:39 ` [Qemu-devel] [PULL 25/25] util: Fix assertion in iov_copy() upon zero 'bytes' and non-zero 'offset' Paolo Bonzini
@ 2016-08-03 10:52 ` Peter Maydell
2016-08-03 16:24 ` Paolo Bonzini
25 siblings, 1 reply; 29+ messages in thread
From: Peter Maydell @ 2016-08-03 10:52 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: QEMU Developers
On 2 August 2016 at 20:39, Paolo Bonzini <pbonzini@redhat.com> wrote:
> The following changes since commit cc0100f464c94bf80ad36cd432f4a1ed58126b60:
>
> MAINTAINERS: Update the Xilinx maintainers (2016-08-01 15:31:32 +0100)
>
> are available in the git repository at:
>
> git://github.com/bonzini/qemu.git tags/for-upstream
>
> for you to fetch changes up to 3531bd22792beae5eba181bf88337d2ff1444817:
>
> util: Fix assertion in iov_copy() upon zero 'bytes' and non-zero 'offset' (2016-08-02 15:00:26 +0200)
>
> ----------------------------------------------------------------
> * xsetbv fix (x86 targets TCG)
> * remove unused functions
> * qht segfault and memory leak fixes
> * NBD fixes
> * Fix for non-power-of-2 discard granularity
> * Memory hotplug fixes
> * Migration regressions
> * IOAPIC fixes and (disabled by default) EOI register support
> * Various other small fixes
>
> ----------------------------------------------------------------
Hi. I'm afraid this doesn't build with our minimum glib version:
/Users/pm215/src/qemu-for-merges/util/qdist.c:67:25: warning: implicit
declaration of function 'g_realloc_n' is invalid in C99
[-Wimplicit-function-declaration]
dist->entries = g_realloc_n(dist->entries, dist->size,
^
g_realloc_n() only appeared in glib 2.24, and our minimum
is 2.22.
thanks
-- PMM
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PULL 00/25] Misc QEMU fixes for 2016-08-02
2016-08-03 10:52 ` [Qemu-devel] [PULL 00/25] Misc QEMU fixes for 2016-08-02 Peter Maydell
@ 2016-08-03 16:24 ` Paolo Bonzini
0 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2016-08-03 16:24 UTC (permalink / raw)
To: Peter Maydell; +Cc: QEMU Developers
On 03/08/2016 12:52, Peter Maydell wrote:
> On 2 August 2016 at 20:39, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> The following changes since commit cc0100f464c94bf80ad36cd432f4a1ed58126b60:
>>
>> MAINTAINERS: Update the Xilinx maintainers (2016-08-01 15:31:32 +0100)
>>
>> are available in the git repository at:
>>
>> git://github.com/bonzini/qemu.git tags/for-upstream
>>
>> for you to fetch changes up to 3531bd22792beae5eba181bf88337d2ff1444817:
>>
>> util: Fix assertion in iov_copy() upon zero 'bytes' and non-zero 'offset' (2016-08-02 15:00:26 +0200)
>>
>> ----------------------------------------------------------------
>> * xsetbv fix (x86 targets TCG)
>> * remove unused functions
>> * qht segfault and memory leak fixes
>> * NBD fixes
>> * Fix for non-power-of-2 discard granularity
>> * Memory hotplug fixes
>> * Migration regressions
>> * IOAPIC fixes and (disabled by default) EOI register support
>> * Various other small fixes
>>
>> ----------------------------------------------------------------
>
> Hi. I'm afraid this doesn't build with our minimum glib version:
>
> /Users/pm215/src/qemu-for-merges/util/qdist.c:67:25: warning: implicit
> declaration of function 'g_realloc_n' is invalid in C99
> [-Wimplicit-function-declaration]
> dist->entries = g_realloc_n(dist->entries, dist->size,
> ^
>
> g_realloc_n() only appeared in glib 2.24, and our minimum
> is 2.22.
Oops, it should use g_new and g_renew instead.
Paolo
^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2016-08-03 16:24 UTC | newest]
Thread overview: 29+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-08-02 19:39 [Qemu-devel] [PULL 00/25] Misc QEMU fixes for 2016-08-02 Paolo Bonzini
2016-08-02 19:39 ` [Qemu-devel] [PULL 01/25] util/qht: Document memory ordering assumptions Paolo Bonzini
2016-08-02 19:39 ` [Qemu-devel] [PULL 02/25] numa: set the memory backend "is_mapped" field Paolo Bonzini
2016-08-02 19:39 ` [Qemu-devel] [PULL 03/25] fix qemu exit on memory hotplug when allocation fails at prealloc time Paolo Bonzini
2016-08-02 19:39 ` [Qemu-devel] [PULL 04/25] checkpatch: add check for bzero Paolo Bonzini
2016-08-02 19:39 ` [Qemu-devel] [PULL 05/25] util: drop inet_nonblocking_connect() Paolo Bonzini
2016-08-02 19:39 ` [Qemu-devel] [PULL 06/25] util: drop unix_nonblocking_connect() Paolo Bonzini
2016-08-02 19:39 ` [Qemu-devel] [PULL 07/25] util: Drop inet_listen() Paolo Bonzini
2016-08-02 19:39 ` [Qemu-devel] [PULL 08/25] qht: do not segfault when gathering stats from an uninitialized qht Paolo Bonzini
2016-08-02 19:39 ` [Qemu-devel] [PULL 09/25] target-i386: fix typo in xsetbv implementation Paolo Bonzini
2016-08-02 19:39 ` [Qemu-devel] [PULL 10/25] qdist: fix memory leak during binning Paolo Bonzini
2016-08-02 21:13 ` Marc-André Lureau
2016-08-02 19:39 ` [Qemu-devel] [PULL 11/25] qdist: use g_realloc_n instead of g_realloc Paolo Bonzini
2016-08-02 19:39 ` [Qemu-devel] [PULL 12/25] qdist: return "(empty)" instead of NULL when printing an empty dist Paolo Bonzini
2016-08-02 19:39 ` [Qemu-devel] [PULL 13/25] mptsas: really fix migration compatibility Paolo Bonzini
2016-08-02 19:39 ` [Qemu-devel] [PULL 14/25] i2c: fix migration regression introduced by broadcast support Paolo Bonzini
2016-08-02 19:39 ` [Qemu-devel] [PULL 15/25] nbd: Fix bad flag detection on server Paolo Bonzini
2016-08-02 19:39 ` [Qemu-devel] [PULL 16/25] nbd: Limit nbdflags to 16 bits Paolo Bonzini
2016-08-02 19:39 ` [Qemu-devel] [PULL 17/25] osdep: Document differences in rounding macros Paolo Bonzini
2016-08-02 19:39 ` [Qemu-devel] [PULL 18/25] block: Cater to iscsi with non-power-of-2 discard Paolo Bonzini
2016-08-02 19:39 ` [Qemu-devel] [PULL 19/25] fw_cfg: Make base type "fw_cfg" abstract Paolo Bonzini
2016-08-02 19:39 ` [Qemu-devel] [PULL 20/25] apic: fix broken migration for kvm-apic Paolo Bonzini
2016-08-02 19:39 ` [Qemu-devel] [PULL 21/25] x86: ioapic: ignore level irq during processing Paolo Bonzini
2016-08-02 19:39 ` [Qemu-devel] [PULL 22/25] x86: ioapic: add support for explicit EOI Paolo Bonzini
2016-08-02 19:39 ` [Qemu-devel] [PULL 23/25] Reorganize help output of '-display' option Paolo Bonzini
2016-08-02 19:39 ` [Qemu-devel] [PULL 24/25] qdev: Fix use after free in qdev_init_nofail error path Paolo Bonzini
2016-08-02 19:39 ` [Qemu-devel] [PULL 25/25] util: Fix assertion in iov_copy() upon zero 'bytes' and non-zero 'offset' Paolo Bonzini
2016-08-03 10:52 ` [Qemu-devel] [PULL 00/25] Misc QEMU fixes for 2016-08-02 Peter Maydell
2016-08-03 16:24 ` Paolo Bonzini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).