From 0814465ab8a1440a6587fd9b489d13ed63595166 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 18 Jul 2019 12:22:01 +0200 Subject: [PATCH 01/13] qemu-ga: clean up TOOLS variable qemu-ga is included in the TOOLS variable without the .exe suffix, and this is then worked around twice in the Makefile. Do the right thing in configure instead. Signed-off-by: Paolo Bonzini --- Makefile | 4 ++-- configure | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 85862fb81a..00825cd270 100644 --- a/Makefile +++ b/Makefile @@ -681,7 +681,7 @@ clean: recurse-clean ! -path ./roms/edk2/BaseTools/Source/Python/UPT/Dll/sqlite3.dll \ -exec rm {} + rm -f $(edk2-decompressed) - rm -f $(filter-out %.tlb,$(TOOLS)) $(HELPERS-y) qemu-ga$(EXESUF) TAGS cscope.* *.pod *~ */*~ + rm -f $(filter-out %.tlb,$(TOOLS)) $(HELPERS-y) TAGS cscope.* *.pod *~ */*~ rm -f fsdev/*.pod scsi/*.pod rm -f qemu-img-cmds.h rm -f ui/shader/*-vert.h ui/shader/*-frag.h @@ -845,7 +845,7 @@ install: all $(if $(BUILD_DOCS),install-doc) install-datadir install-localstated $(if $(INSTALL_BLOBS),$(edk2-decompressed)) \ recurse-install ifneq ($(TOOLS),) - $(call install-prog,$(subst qemu-ga,qemu-ga$(EXESUF),$(TOOLS)),$(DESTDIR)$(bindir)) + $(call install-prog,$(TOOLS),$(DESTDIR)$(bindir)) endif ifneq ($(CONFIG_MODULES),) $(INSTALL_DIR) "$(DESTDIR)$(qemu_moddir)" diff --git a/configure b/configure index 942a73b795..e96981bbf8 100755 --- a/configure +++ b/configure @@ -6129,7 +6129,7 @@ if [ "$guest_agent" != "no" ]; then if [ "$softmmu" = no -a "$want_tools" = no ] ; then guest_agent=no elif [ "$linux" = "yes" -o "$bsd" = "yes" -o "$solaris" = "yes" -o "$mingw32" = "yes" ] ; then - tools="qemu-ga $tools" + tools="qemu-ga\$(EXESUF) $tools" guest_agent=yes elif [ "$guest_agent" != yes ]; then guest_agent=no From c932ce3144d92b3032336d02b59e6a14be68098d Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 18 Jul 2019 12:24:29 +0200 Subject: [PATCH 02/13] configure: define CONFIG_TOOLS here Defining CONFIG_TOOLS on the basis of $(TOOLS) has the disadvantage of including it also if e.g. qemu-ga is requested. The correct information is available in configure, define it there. This also has the benefit of not installing the manpages for block layer tools if the only "tool" being built is the guest agent. Signed-off-by: Paolo Bonzini --- Makefile | 5 ++--- configure | 3 +++ 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 00825cd270..574fedea6b 100644 --- a/Makefile +++ b/Makefile @@ -84,8 +84,7 @@ endif include $(SRC_PATH)/rules.mak -# notempy and lor are defined in rules.mak -CONFIG_TOOLS := $(call notempty,$(TOOLS)) +# lor is defined in rules.mak CONFIG_BLOCK := $(call lor,$(CONFIG_SOFTMMU),$(CONFIG_TOOLS)) # Create QEMU_PKGVERSION and FULL_VERSION strings @@ -809,7 +808,7 @@ ifdef CONFIG_POSIX $(INSTALL_DATA) docs/interop/qemu-qmp-ref.7 "$(DESTDIR)$(mandir)/man7" $(INSTALL_DATA) docs/qemu-block-drivers.7 "$(DESTDIR)$(mandir)/man7" $(INSTALL_DATA) docs/qemu-cpu-models.7 "$(DESTDIR)$(mandir)/man7" -ifneq ($(TOOLS),) +ifeq ($(CONFIG_TOOLS),y) $(INSTALL_DATA) qemu-img.1 "$(DESTDIR)$(mandir)/man1" $(INSTALL_DIR) "$(DESTDIR)$(mandir)/man8" $(INSTALL_DATA) qemu-nbd.8 "$(DESTDIR)$(mandir)/man8" diff --git a/configure b/configure index e96981bbf8..b7f4283451 100755 --- a/configure +++ b/configure @@ -6617,6 +6617,9 @@ fi if test "$profiler" = "yes" ; then echo "CONFIG_PROFILER=y" >> $config_host_mak fi +if test "$want_tools" = "yes" ; then + echo "CONFIG_TOOLS=y" >> $config_host_mak +fi if test "$slirp" != "no"; then echo "CONFIG_SLIRP=y" >> $config_host_mak echo "CONFIG_SMBD_COMMAND=\"$smbd\"" >> $config_host_mak From 90629122d2ef536290882c71ca16bac3df842ca1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Mon, 22 Jul 2019 17:10:46 +0400 Subject: [PATCH 03/13] module: use g_hash_table_add() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The hashtable is used like a set, use the convenience g_hash_table_add() function. Signed-off-by: Marc-André Lureau Signed-off-by: Paolo Bonzini --- util/module.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/util/module.c b/util/module.c index 142db7e911..ca9885c490 100644 --- a/util/module.c +++ b/util/module.c @@ -179,11 +179,10 @@ void module_load_one(const char *prefix, const char *lib_name) module_name = g_strdup_printf("%s%s", prefix, lib_name); - if (g_hash_table_lookup(loaded_modules, module_name)) { + if (!g_hash_table_add(loaded_modules, module_name)) { g_free(module_name); return; } - g_hash_table_insert(loaded_modules, module_name, module_name); exec_dir = qemu_get_exec_dir(); search_dir = getenv("QEMU_MODULE_DIR"); From 81d8ccb1bea4fb9eaaf4c8e30bd4021180a9a39f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Mon, 22 Jul 2019 17:13:23 +0400 Subject: [PATCH 04/13] module: return success on module load MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Let the caller know of load success. Note that this also changes slightly the behaviour of the function to try loading on subsequent calls if the previous ones failed. Signed-off-by: Marc-André Lureau Signed-off-by: Paolo Bonzini --- include/qemu/module.h | 2 +- util/module.c | 14 +++++++++++--- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/include/qemu/module.h b/include/qemu/module.h index db3065381d..65ba596e46 100644 --- a/include/qemu/module.h +++ b/include/qemu/module.h @@ -65,6 +65,6 @@ void register_module_init(void (*fn)(void), module_init_type type); void register_dso_module_init(void (*fn)(void), module_init_type type); void module_call_init(module_init_type type); -void module_load_one(const char *prefix, const char *lib_name); +bool module_load_one(const char *prefix, const char *lib_name); #endif diff --git a/util/module.c b/util/module.c index ca9885c490..e9fe3e5422 100644 --- a/util/module.c +++ b/util/module.c @@ -156,8 +156,10 @@ out: } #endif -void module_load_one(const char *prefix, const char *lib_name) +bool module_load_one(const char *prefix, const char *lib_name) { + bool success = false; + #ifdef CONFIG_MODULES char *fname = NULL; char *exec_dir; @@ -170,7 +172,7 @@ void module_load_one(const char *prefix, const char *lib_name) if (!g_module_supported()) { fprintf(stderr, "Module is not supported by system.\n"); - return; + return false; } if (!loaded_modules) { @@ -181,7 +183,7 @@ void module_load_one(const char *prefix, const char *lib_name) if (!g_hash_table_add(loaded_modules, module_name)) { g_free(module_name); - return; + return true; } exec_dir = qemu_get_exec_dir(); @@ -205,13 +207,19 @@ void module_load_one(const char *prefix, const char *lib_name) fname = NULL; /* Try loading until loaded a module file */ if (!ret) { + success = true; break; } } + if (!success) { + g_hash_table_remove(loaded_modules, module_name); + } + for (i = 0; i < n_dirs; i++) { g_free(dirs[i]); } #endif + return success; } From eb062cfa7331dd90837177b9a5a6becab305b1ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Mon, 22 Jul 2019 22:51:40 +0400 Subject: [PATCH 05/13] tests: add module loading test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This test will simply check that modules can be loaded, and no symbols are missing. Signed-off-by: Marc-André Lureau Signed-off-by: Paolo Bonzini --- qtest.c | 9 ++++++ tests/Makefile.include | 1 + tests/libqtest.c | 6 ++++ tests/libqtest.h | 2 ++ tests/modules-test.c | 71 ++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 89 insertions(+) create mode 100644 tests/modules-test.c diff --git a/qtest.c b/qtest.c index d8b4857c74..8b50e2783e 100644 --- a/qtest.c +++ b/qtest.c @@ -661,6 +661,15 @@ static void qtest_process_command(CharBackend *chr, gchar **words) qtest_send_prefix(chr); qtest_sendf(chr, "OK %"PRIi64"\n", (int64_t)qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL)); + } else if (strcmp(words[0], "module_load") == 0) { + g_assert(words[1] && words[2]); + + qtest_send_prefix(chr); + if (module_load_one(words[1], words[2])) { + qtest_sendf(chr, "OK\n"); + } else { + qtest_sendf(chr, "FAIL\n"); + } } else if (qtest_enabled() && strcmp(words[0], "clock_set") == 0) { int64_t ns; int ret; diff --git a/tests/Makefile.include b/tests/Makefile.include index 6f02dfcc01..39bed753b3 100644 --- a/tests/Makefile.include +++ b/tests/Makefile.include @@ -149,6 +149,7 @@ check-block-$(call land,$(CONFIG_POSIX),$(CONFIG_SOFTMMU)) += tests/check-block. check-qtest-generic-y += tests/qmp-test$(EXESUF) check-qtest-generic-y += tests/qmp-cmd-test$(EXESUF) +check-qtest-generic-$(CONFIG_MODULES) += tests/modules-test$(EXESUF) check-qtest-generic-y += tests/device-introspect-test$(EXESUF) check-qtest-generic-y += tests/cdrom-test$(EXESUF) diff --git a/tests/libqtest.c b/tests/libqtest.c index eb971d0d11..2713b86cf7 100644 --- a/tests/libqtest.c +++ b/tests/libqtest.c @@ -811,6 +811,12 @@ bool qtest_get_irq(QTestState *s, int num) return s->irq_level[num]; } +void qtest_module_load(QTestState *s, const char *prefix, const char *libname) +{ + qtest_sendf(s, "module_load %s %s\n", prefix, libname); + qtest_rsp(s, 0); +} + static int64_t qtest_clock_rsp(QTestState *s) { gchar **words; diff --git a/tests/libqtest.h b/tests/libqtest.h index 7833148358..07ea35867c 100644 --- a/tests/libqtest.h +++ b/tests/libqtest.h @@ -262,6 +262,8 @@ char *qtest_hmp(QTestState *s, const char *fmt, ...) GCC_FMT_ATTR(2, 3); char *qtest_vhmp(QTestState *s, const char *fmt, va_list ap) GCC_FMT_ATTR(2, 0); +void qtest_module_load(QTestState *s, const char *prefix, const char *libname); + /** * qtest_get_irq: * @s: #QTestState instance to operate on. diff --git a/tests/modules-test.c b/tests/modules-test.c new file mode 100644 index 0000000000..3aef0e5a19 --- /dev/null +++ b/tests/modules-test.c @@ -0,0 +1,71 @@ +#include "qemu/osdep.h" +#include "libqtest.h" + +static void test_modules_load(const void *data) +{ + QTestState *qts; + const char **args = data; + + qts = qtest_init(NULL); + qtest_module_load(qts, args[0], args[1]); + qtest_quit(qts); +} + +int main(int argc, char *argv[]) +{ + const char *modules[] = { +#ifdef CONFIG_CURL + "block-", "curl", +#endif +#ifdef CONFIG_GLUSTERFS + "block-", "gluster", +#endif +#ifdef CONFIG_LIBISCSI + "block-", "iscsi", +#endif +#ifdef CONFIG_LIBNFS + "block-", "nfs", +#endif +#ifdef CONFIG_LIBSSH + "block-", "ssh", +#endif +#ifdef CONFIG_RBD + "block-", "rbd", +#endif +#ifdef CONFIG_AUDIO_ALSA + "audio-", "alsa", +#endif +#ifdef CONFIG_AUDIO_OSS + "audio-", "oss", +#endif +#ifdef CONFIG_AUDIO_PA + "audio-", "pa", +#endif +#ifdef CONFIG_AUDIO_SDL + "audio-", "sdl", +#endif +#ifdef CONFIG_CURSES + "ui-", "curses", +#endif +#if defined(CONFIG_GTK) && defined(CONFIG_VTE) + "ui-", "gtk", +#endif +#ifdef CONFIG_SDL + "ui-", "sdl", +#endif +#if defined(CONFIG_SPICE) && defined(CONFIG_GIO) + "ui-", "spice-app", +#endif + }; + int i; + + g_test_init(&argc, &argv, NULL); + + for (i = 0; i < G_N_ELEMENTS(modules); i += 2) { + char *testname = g_strdup_printf("/module/load/%s", modules[i + 1]); + qtest_add_data_func(testname, modules + i, test_modules_load); + g_free(testname); + } + + return g_test_run(); +} From fbb04e760f4818a1ba9cfde0a2571a15cd4f49f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Tue, 23 Jul 2019 16:14:49 +0400 Subject: [PATCH 06/13] configure: remove AUTOCONF_HOST MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is a left-over from commit c12b6d70e384c769ca372e15ffd19b3e9f563662 ("pixman: drop submodule") Signed-off-by: Marc-André Lureau Signed-off-by: Paolo Bonzini --- configure | 5 ----- 1 file changed, 5 deletions(-) diff --git a/configure b/configure index b7f4283451..d7d6dfc6c2 100755 --- a/configure +++ b/configure @@ -7366,11 +7366,6 @@ if test "$sparse" = "yes" ; then echo "HOST_CC := REAL_CC=\"\$(HOST_CC)\" cgcc" >> $config_host_mak echo "QEMU_CFLAGS += -Wbitwise -Wno-transparent-union -Wno-old-initializer -Wno-non-pointer-null" >> $config_host_mak fi -if test "$cross_prefix" != ""; then - echo "AUTOCONF_HOST := --host=${cross_prefix%-}" >> $config_host_mak -else - echo "AUTOCONF_HOST := " >> $config_host_mak -fi echo "LDFLAGS=$LDFLAGS" >> $config_host_mak echo "LDFLAGS_NOPIE=$LDFLAGS_NOPIE" >> $config_host_mak echo "QEMU_LDFLAGS=$QEMU_LDFLAGS" >> $config_host_mak From c5b35f03c3fcb8c8e0c6efc50f3a9e70dc028c00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Thu, 25 Jul 2019 23:36:15 +0400 Subject: [PATCH 07/13] minikconf: don't print CONFIG_FOO=n lines MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit qemu in general doesn't define CONFIG_FOO if it's false. This also helps with the dumb kconfig parser from meson, as source_set considers any non-empty value as true. Signed-off-by: Marc-André Lureau Signed-off-by: Paolo Bonzini --- scripts/minikconf.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/minikconf.py b/scripts/minikconf.py index 3109a81db7..40ae1989e1 100644 --- a/scripts/minikconf.py +++ b/scripts/minikconf.py @@ -702,8 +702,8 @@ if __name__ == '__main__': config = data.compute_config() for key in sorted(config.keys()): - if key not in external_vars: - print ('CONFIG_%s=%s' % (key, ('y' if config[key] else 'n'))) + if key not in external_vars and config[key]: + print ('CONFIG_%s=y' % key) deps = open(argv[2], 'w') for fname in data.previously_included: From 9c1aa1c235c770d84462d482460a96e957e95b9c Mon Sep 17 00:00:00 2001 From: Peter Xu Date: Tue, 20 Aug 2019 22:13:27 +0800 Subject: [PATCH 08/13] memory: Refactor memory_region_clear_coalescing Removing the update variable and quit earlier if the memory region has no coalesced range. This prepares for the next patch. Fixes: 3ac7d43a6fbb5d4a3 Signed-off-by: Peter Xu Message-Id: <20190820141328.10009-4-peterx@redhat.com> Signed-off-by: Paolo Bonzini --- memory.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/memory.c b/memory.c index 4aa38eb5b1..9a1193a144 100644 --- a/memory.c +++ b/memory.c @@ -2281,7 +2281,10 @@ void memory_region_add_coalescing(MemoryRegion *mr, void memory_region_clear_coalescing(MemoryRegion *mr) { CoalescedMemoryRange *cmr; - bool updated = false; + + if (QTAILQ_EMPTY(&mr->coalesced)) { + return; + } qemu_flush_coalesced_mmio_buffer(); mr->flush_coalesced_mmio = false; @@ -2290,12 +2293,9 @@ void memory_region_clear_coalescing(MemoryRegion *mr) cmr = QTAILQ_FIRST(&mr->coalesced); QTAILQ_REMOVE(&mr->coalesced, cmr, link); g_free(cmr); - updated = true; } - if (updated) { - memory_region_update_coalesced_range(mr); - } + memory_region_update_coalesced_range(mr); } void memory_region_set_flush_coalesced(MemoryRegion *mr) From 23f1174aac4181f86bb7e13ca8bc2d4a0bdf1e5c Mon Sep 17 00:00:00 2001 From: Peter Xu Date: Tue, 20 Aug 2019 22:13:25 +0800 Subject: [PATCH 09/13] memory: Split zones when do coalesced_io_del() It is a workaround of current KVM's KVM_UNREGISTER_COALESCED_MMIO interface. The kernel interface only allows to unregister an mmio device with exactly the zone size when registered, or any smaller zone that is included in the device mmio zone. It does not support the userspace to specify a very large zone to remove all the small mmio devices within the zone covered. Logically speaking it would be nicer to fix this from KVM side, though in all cases we still need to coop with old kernels so let's do this. Fixes: 3ac7d43a6fbb5d4a3 Signed-off-by: Peter Xu Message-Id: <20190820141328.10009-2-peterx@redhat.com> Signed-off-by: Paolo Bonzini --- memory.c | 49 +++++++++++++++++++++++++++++++++++-------------- 1 file changed, 35 insertions(+), 14 deletions(-) diff --git a/memory.c b/memory.c index 9a1193a144..7124274f49 100644 --- a/memory.c +++ b/memory.c @@ -855,8 +855,39 @@ static void address_space_update_ioeventfds(AddressSpace *as) flatview_unref(view); } +/* + * Notify the memory listeners about the coalesced IO change events of + * range `cmr'. Only the part that has intersection of the specified + * FlatRange will be sent. + */ +static void flat_range_coalesced_io_notify(FlatRange *fr, AddressSpace *as, + CoalescedMemoryRange *cmr, bool add) +{ + AddrRange tmp; + + tmp = addrrange_shift(cmr->addr, + int128_sub(fr->addr.start, + int128_make64(fr->offset_in_region))); + if (!addrrange_intersects(tmp, fr->addr)) { + return; + } + tmp = addrrange_intersection(tmp, fr->addr); + + if (add) { + MEMORY_LISTENER_UPDATE_REGION(fr, as, Forward, coalesced_io_add, + int128_get64(tmp.start), + int128_get64(tmp.size)); + } else { + MEMORY_LISTENER_UPDATE_REGION(fr, as, Reverse, coalesced_io_del, + int128_get64(tmp.start), + int128_get64(tmp.size)); + } +} + static void flat_range_coalesced_io_del(FlatRange *fr, AddressSpace *as) { + CoalescedMemoryRange *cmr; + if (!fr->has_coalesced_range) { return; } @@ -865,16 +896,15 @@ static void flat_range_coalesced_io_del(FlatRange *fr, AddressSpace *as) return; } - MEMORY_LISTENER_UPDATE_REGION(fr, as, Reverse, coalesced_io_del, - int128_get64(fr->addr.start), - int128_get64(fr->addr.size)); + QTAILQ_FOREACH(cmr, &fr->mr->coalesced, link) { + flat_range_coalesced_io_notify(fr, as, cmr, false); + } } static void flat_range_coalesced_io_add(FlatRange *fr, AddressSpace *as) { MemoryRegion *mr = fr->mr; CoalescedMemoryRange *cmr; - AddrRange tmp; if (QTAILQ_EMPTY(&mr->coalesced)) { return; @@ -885,16 +915,7 @@ static void flat_range_coalesced_io_add(FlatRange *fr, AddressSpace *as) } QTAILQ_FOREACH(cmr, &mr->coalesced, link) { - tmp = addrrange_shift(cmr->addr, - int128_sub(fr->addr.start, - int128_make64(fr->offset_in_region))); - if (!addrrange_intersects(tmp, fr->addr)) { - continue; - } - tmp = addrrange_intersection(tmp, fr->addr); - MEMORY_LISTENER_UPDATE_REGION(fr, as, Forward, coalesced_io_add, - int128_get64(tmp.start), - int128_get64(tmp.size)); + flat_range_coalesced_io_notify(fr, as, cmr, true); } } From 264ef5a5c52c249ff51a16d141fc03df71714a13 Mon Sep 17 00:00:00 2001 From: Peter Xu Date: Tue, 20 Aug 2019 22:13:26 +0800 Subject: [PATCH 10/13] memory: Remove has_coalesced_range counter The has_coalesced_range could potentially be problematic in that it only works for additions of coalesced mmio ranges but not deletions. The reason is that has_coalesced_range information can be lost when the FlatView updates the topology again when the updated region is not covering the coalesced regions. When that happens, due to flatrange_equal() is not checking against has_coalesced_range, the new FlatRange will be seen as the same one as the old and the new instance (whose has_coalesced_range will be zero) will replace the old instance (whose has_coalesced_range _could_ be non-zero). The counter was originally used to make sure every FlatRange will only notify once for coalesced_io_{add|del} memory listeners, because each FlatRange can be used by multiple address spaces, so logically speaking it could be called multiple times. However we should not limit that, because memory listeners should will only be registered with specific address space rather than multiple address spaces. So let's fix this up by simply removing the whole has_coalesced_range. Fixes: 3ac7d43a6fbb5d4a3 Signed-off-by: Peter Xu Message-Id: <20190820141328.10009-3-peterx@redhat.com> Signed-off-by: Paolo Bonzini --- memory.c | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/memory.c b/memory.c index 7124274f49..6e4b06e452 100644 --- a/memory.c +++ b/memory.c @@ -217,7 +217,6 @@ struct FlatRange { bool romd_mode; bool readonly; bool nonvolatile; - int has_coalesced_range; }; #define FOR_EACH_FLAT_RANGE(var, view) \ @@ -654,7 +653,6 @@ static void render_memory_region(FlatView *view, fr.romd_mode = mr->romd_mode; fr.readonly = readonly; fr.nonvolatile = nonvolatile; - fr.has_coalesced_range = 0; /* Render the region itself into any gaps left by the current view. */ for (i = 0; i < view->nr && int128_nz(remain); ++i) { @@ -888,14 +886,6 @@ static void flat_range_coalesced_io_del(FlatRange *fr, AddressSpace *as) { CoalescedMemoryRange *cmr; - if (!fr->has_coalesced_range) { - return; - } - - if (--fr->has_coalesced_range > 0) { - return; - } - QTAILQ_FOREACH(cmr, &fr->mr->coalesced, link) { flat_range_coalesced_io_notify(fr, as, cmr, false); } @@ -910,10 +900,6 @@ static void flat_range_coalesced_io_add(FlatRange *fr, AddressSpace *as) return; } - if (fr->has_coalesced_range++) { - return; - } - QTAILQ_FOREACH(cmr, &mr->coalesced, link) { flat_range_coalesced_io_notify(fr, as, cmr, true); } From b960fc1796fb078c21121abf01499603b66b3f57 Mon Sep 17 00:00:00 2001 From: Peter Xu Date: Tue, 20 Aug 2019 22:13:28 +0800 Subject: [PATCH 11/13] memory: Fix up memory_region_{add|del}_coalescing The old memory_region_{add|clear}_coalescing() has some defects because they both changed mr->coalesced before updating the regions using memory_region_update_coalesced_range_as(). Then when the regions were updated in memory_region_update_coalesced_range_as() the mr->coalesced will always be either one more or one less. So: - For memory_region_add_coalescing: it'll always trying to remove the newly added coalesced region while it shouldn't, and, - For memory_region_clear_coalescing: when it calls the update there will be no coalesced ranges on mr->coalesced because they were all removed before hand so the update will probably do nothing for real. Let's fix this. Now we've got flat_range_coalesced_io_notify() to notify a single CoalescedMemoryRange instance change, so use it in the existing memory_region_update_coalesced_range() logic by only notify either an addition or deletion. Then we hammer both the memory_region_{add|clear}_coalescing() to use it. Fixes: 3ac7d43a6fbb5d4a3 Signed-off-by: Peter Xu Message-Id: <20190820141328.10009-5-peterx@redhat.com> Signed-off-by: Paolo Bonzini --- memory.c | 36 +++++++++++++++++------------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/memory.c b/memory.c index 6e4b06e452..7fd93b1d42 100644 --- a/memory.c +++ b/memory.c @@ -2243,27 +2243,26 @@ void memory_region_ram_resize(MemoryRegion *mr, ram_addr_t newsize, Error **errp qemu_ram_resize(mr->ram_block, newsize, errp); } -static void memory_region_update_coalesced_range_as(MemoryRegion *mr, AddressSpace *as) +/* + * Call proper memory listeners about the change on the newly + * added/removed CoalescedMemoryRange. + */ +static void memory_region_update_coalesced_range(MemoryRegion *mr, + CoalescedMemoryRange *cmr, + bool add) { + AddressSpace *as; FlatView *view; FlatRange *fr; - view = address_space_get_flatview(as); - FOR_EACH_FLAT_RANGE(fr, view) { - if (fr->mr == mr) { - flat_range_coalesced_io_del(fr, as); - flat_range_coalesced_io_add(fr, as); - } - } - flatview_unref(view); -} - -static void memory_region_update_coalesced_range(MemoryRegion *mr) -{ - AddressSpace *as; - QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) { - memory_region_update_coalesced_range_as(mr, as); + view = address_space_get_flatview(as); + FOR_EACH_FLAT_RANGE(fr, view) { + if (fr->mr == mr) { + flat_range_coalesced_io_notify(fr, as, cmr, add); + } + } + flatview_unref(view); } } @@ -2281,7 +2280,7 @@ void memory_region_add_coalescing(MemoryRegion *mr, cmr->addr = addrrange_make(int128_make64(offset), int128_make64(size)); QTAILQ_INSERT_TAIL(&mr->coalesced, cmr, link); - memory_region_update_coalesced_range(mr); + memory_region_update_coalesced_range(mr, cmr, true); memory_region_set_flush_coalesced(mr); } @@ -2299,10 +2298,9 @@ void memory_region_clear_coalescing(MemoryRegion *mr) while (!QTAILQ_EMPTY(&mr->coalesced)) { cmr = QTAILQ_FIRST(&mr->coalesced); QTAILQ_REMOVE(&mr->coalesced, cmr, link); + memory_region_update_coalesced_range(mr, cmr, false); g_free(cmr); } - - memory_region_update_coalesced_range(mr); } void memory_region_set_flush_coalesced(MemoryRegion *mr) From b65cb867cc13b581baeea7648a9f45899cb7a714 Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Mon, 12 Aug 2019 18:58:28 +0300 Subject: [PATCH 12/13] main-loop: Fix GSource leak in qio_task_thread_worker() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After g_source_attach() the GMainContext holds a reference to the GSource, so the caller does not need to keep it. qio_task_thread_worker() is not releasing its reference so the GSource is being leaked since a17536c594bfed94d05667b419f747b692f5fc7f. Signed-off-by: Alberto Garcia Reviewed-by: Daniel P. Berrangé Message-Id: <1565625509-404969-2-git-send-email-andrey.shinkevich@virtuozzo.com> Signed-off-by: Paolo Bonzini --- io/task.c | 1 + 1 file changed, 1 insertion(+) diff --git a/io/task.c b/io/task.c index 64c4c7126a..1ae7b86488 100644 --- a/io/task.c +++ b/io/task.c @@ -136,6 +136,7 @@ static gpointer qio_task_thread_worker(gpointer opaque) qio_task_thread_result, task, NULL); g_source_attach(task->thread->completion, task->thread->context); + g_source_unref(task->thread->completion); trace_qio_task_thread_source_attach(task, task->thread->completion); qemu_cond_signal(&task->thread_cond); From 78d01598aea85841f0e4f8baf62c42b76230a81c Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Mon, 12 Aug 2019 18:58:29 +0300 Subject: [PATCH 13/13] char-socket: Lock tcp_chr_disconnect() and socket_reconnect_timeout() There's a race condition in which the tcp_chr_read() ioc handler can close a connection that is being written to from another thread. Running iotest 136 in a loop triggers this problem and crashes QEMU. (gdb) bt #0 0x00005558b842902d in object_get_class (obj=0x0) at qom/object.c:860 #1 0x00005558b84f92db in qio_channel_writev_full (ioc=0x0, iov=0x7ffc355decf0, niov=1, fds=0x0, nfds=0, errp=0x0) at io/channel.c:76 #2 0x00005558b84e0e9e in io_channel_send_full (ioc=0x0, buf=0x5558baf5beb0, len=138, fds=0x0, nfds=0) at chardev/char-io.c:123 #3 0x00005558b84e4a69 in tcp_chr_write (chr=0x5558ba460380, buf=0x5558baf5beb0 "...", len=138) at chardev/char-socket.c:135 #4 0x00005558b84dca55 in qemu_chr_write_buffer (s=0x5558ba460380, buf=0x5558baf5beb0 "...", len=138, offset=0x7ffc355dedd0, write_all=false) at chardev/char.c:112 #5 0x00005558b84dcbc2 in qemu_chr_write (s=0x5558ba460380, buf=0x5558baf5beb0 "...", len=138, write_all=false) at chardev/char.c:147 #6 0x00005558b84dfb26 in qemu_chr_fe_write (be=0x5558ba476610, buf=0x5558baf5beb0 "...", len=138) at chardev/char-fe.c:42 #7 0x00005558b8088c86 in monitor_flush_locked (mon=0x5558ba476610) at monitor.c:406 #8 0x00005558b8088e8c in monitor_puts (mon=0x5558ba476610, str=0x5558ba921e49 "") at monitor.c:449 #9 0x00005558b8089178 in qmp_send_response (mon=0x5558ba476610, rsp=0x5558bb161600) at monitor.c:498 #10 0x00005558b808920c in monitor_qapi_event_emit (event=QAPI_EVENT_SHUTDOWN, qdict=0x5558bb161600) at monitor.c:526 #11 0x00005558b8089307 in monitor_qapi_event_queue_no_reenter (event=QAPI_EVENT_SHUTDOWN, qdict=0x5558bb161600) at monitor.c:551 #12 0x00005558b80896c0 in qapi_event_emit (event=QAPI_EVENT_SHUTDOWN, qdict=0x5558bb161600) at monitor.c:626 #13 0x00005558b855f23b in qapi_event_send_shutdown (guest=false, reason=SHUTDOWN_CAUSE_HOST_QMP_QUIT) at qapi/qapi-events-run-state.c:43 #14 0x00005558b81911ef in qemu_system_shutdown (cause=SHUTDOWN_CAUSE_HOST_QMP_QUIT) at vl.c:1837 #15 0x00005558b8191308 in main_loop_should_exit () at vl.c:1885 #16 0x00005558b819140d in main_loop () at vl.c:1924 #17 0x00005558b8198c84 in main (argc=18, argv=0x7ffc355df3f8, envp=0x7ffc355df490) at vl.c:4665 This patch adds a lock to protect tcp_chr_disconnect() and socket_reconnect_timeout() Signed-off-by: Alberto Garcia Signed-off-by: Andrey Shinkevich Message-Id: <1565625509-404969-3-git-send-email-andrey.shinkevich@virtuozzo.com> Signed-off-by: Paolo Bonzini --- chardev/char-socket.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/chardev/char-socket.c b/chardev/char-socket.c index 7ca5d97af3..03f03407b0 100644 --- a/chardev/char-socket.c +++ b/chardev/char-socket.c @@ -150,7 +150,7 @@ static void tcp_chr_accept(QIONetListener *listener, void *opaque); static int tcp_chr_read_poll(void *opaque); -static void tcp_chr_disconnect(Chardev *chr); +static void tcp_chr_disconnect_locked(Chardev *chr); /* Called with chr_write_lock held. */ static int tcp_chr_write(Chardev *chr, const uint8_t *buf, int len) @@ -174,7 +174,7 @@ static int tcp_chr_write(Chardev *chr, const uint8_t *buf, int len) if (ret < 0 && errno != EAGAIN) { if (tcp_chr_read_poll(chr) <= 0) { - tcp_chr_disconnect(chr); + tcp_chr_disconnect_locked(chr); return len; } /* else let the read handler finish it properly */ } @@ -469,8 +469,9 @@ static void update_disconnected_filename(SocketChardev *s) /* NB may be called even if tcp_chr_connect has not been * reached, due to TLS or telnet initialization failure, * so can *not* assume s->state == TCP_CHARDEV_STATE_CONNECTED + * This must be called with chr->chr_write_lock held. */ -static void tcp_chr_disconnect(Chardev *chr) +static void tcp_chr_disconnect_locked(Chardev *chr) { SocketChardev *s = SOCKET_CHARDEV(chr); bool emit_close = s->state == TCP_CHARDEV_STATE_CONNECTED; @@ -490,6 +491,13 @@ static void tcp_chr_disconnect(Chardev *chr) } } +static void tcp_chr_disconnect(Chardev *chr) +{ + qemu_mutex_lock(&chr->chr_write_lock); + tcp_chr_disconnect_locked(chr); + qemu_mutex_unlock(&chr->chr_write_lock); +} + static gboolean tcp_chr_read(QIOChannel *chan, GIOCondition cond, void *opaque) { Chardev *chr = CHARDEV(opaque); @@ -1131,8 +1139,10 @@ static gboolean socket_reconnect_timeout(gpointer opaque) Chardev *chr = CHARDEV(opaque); SocketChardev *s = SOCKET_CHARDEV(opaque); + qemu_mutex_lock(&chr->chr_write_lock); g_source_unref(s->reconnect_timer); s->reconnect_timer = NULL; + qemu_mutex_unlock(&chr->chr_write_lock); if (chr->be_open) { return false;