From eada6d9220b77ffee13a86ca8a7aba8fc4451e71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volker=20R=C3=BCmelin?= Date: Wed, 21 Oct 2020 08:40:33 +0200 Subject: [PATCH 1/6] qmp: fix aio_poll() assertion failure on Windows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit 9ce44e2ce2 "qmp: Move dispatcher to a coroutine" modified aio_poll() in util/aio-posix.c to avoid an assertion failure. This change is missing in util/aio-win32.c. Apply the changes to util/aio-posix.c to util/aio-win32.c too. This fixes an assertion failure on Windows whenever QEMU exits. $ ./qemu-system-x86_64.exe -machine pc,accel=tcg -display gtk ** ERROR:../qemu/util/aio-win32.c:337:aio_poll: assertion failed: (in_aio_context_home_thread(ctx)) Bail out! ERROR:../qemu/util/aio-win32.c:337:aio_poll: assertion failed: (in_aio_context_home_thread(ctx)) Fixes: 9ce44e2ce2 ("qmp: Move dispatcher to a coroutine") Signed-off-by: Volker Rümelin Message-Id: <20201021064033.8600-1-vr_qemu@t-online.de> Tested-by: Mark Cave-Ayland Signed-off-by: Kevin Wolf --- util/aio-win32.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/util/aio-win32.c b/util/aio-win32.c index e7b1d649e9..168717b51b 100644 --- a/util/aio-win32.c +++ b/util/aio-win32.c @@ -18,6 +18,7 @@ #include "qemu/osdep.h" #include "qemu-common.h" #include "block/block.h" +#include "qemu/main-loop.h" #include "qemu/queue.h" #include "qemu/sockets.h" #include "qapi/error.h" @@ -333,8 +334,13 @@ bool aio_poll(AioContext *ctx, bool blocking) * There cannot be two concurrent aio_poll calls for the same AioContext (or * an aio_poll concurrent with a GSource prepare/check/dispatch callback). * We rely on this below to avoid slow locked accesses to ctx->notify_me. + * + * aio_poll() may only be called in the AioContext's thread. iohandler_ctx + * is special in that it runs in the main thread, but that thread's context + * is qemu_aio_context. */ - assert(in_aio_context_home_thread(ctx)); + assert(in_aio_context_home_thread(ctx == iohandler_get_aio_context() ? + qemu_get_aio_context() : ctx)); progress = false; /* aio_notify can avoid the expensive event_notifier_set if From 6aec830e77a3f80952b41e421dc23c0041c1e399 Mon Sep 17 00:00:00 2001 From: Tuguoyi Date: Mon, 2 Nov 2020 09:04:57 +0000 Subject: [PATCH 2/6] qemu-img convert: Free @sn_opts in all error cases @sn_opts is initialized at the beginning, so it should be deleted after jumping to the lable 'fail_getopt' Signed-off-by: Guoyi Tu Message-Id: <6ff1c5d372944494be3932274f75485d@h3c.com> Signed-off-by: Kevin Wolf --- qemu-img.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qemu-img.c b/qemu-img.c index a968c74cba..c2c56fc797 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -2751,7 +2751,6 @@ out: qemu_progress_end(); qemu_opts_del(opts); qemu_opts_free(create_opts); - qemu_opts_del(sn_opts); qobject_unref(open_opts); blk_unref(s.target); if (s.src) { @@ -2763,6 +2762,7 @@ out: g_free(s.src_sectors); g_free(s.src_alignment); fail_getopt: + qemu_opts_del(sn_opts); g_free(options); return !!ret; From 503c2b31b675c9fba2ff9711a79e55585304895a Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Tue, 27 Oct 2020 17:38:04 +0100 Subject: [PATCH 3/6] iotests.py: Fix type check errors in wait_migration() Commit 1847a4a8c20 clarified that event_wait() can return None (though only with timeout=0) and commit f12a282ff47 annotated it as returning Optional[QMPMessage]. Type checks in wait_migration() fail because of the unexpected optional return type: iotests.py:750: error: Value of type variable "Msg" of "log" cannot be "Optional[Dict[str, Any]]" iotests.py:751: error: Value of type "Optional[Dict[str, Any]]" is not indexable iotests.py:754: error: Value of type "Optional[Dict[str, Any]]" is not indexable Fortunately, the non-zero default timeout is used in the event_wait() call, so we can make mypy happy by just asserting this. Signed-off-by: Kevin Wolf Message-Id: <20201027163806.290960-2-kwolf@redhat.com> Reviewed-by: John Snow Signed-off-by: Kevin Wolf --- tests/qemu-iotests/iotests.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py index 63d2ace93c..28388a0fbc 100644 --- a/tests/qemu-iotests/iotests.py +++ b/tests/qemu-iotests/iotests.py @@ -747,6 +747,10 @@ class VM(qtest.QEMUQtestMachine): def wait_migration(self, expect_runstate: Optional[str]) -> bool: while True: event = self.event_wait('MIGRATION') + # We use the default timeout, and with a timeout, event_wait() + # never returns None + assert event + log(event, filters=[filter_qmp_event]) if event['data']['status'] in ('completed', 'failed'): break From eeb7314c46545a2426c39e2c00f03a0ab5f4f4f1 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Tue, 27 Oct 2020 17:38:05 +0100 Subject: [PATCH 4/6] iotests: Disable unsubscriptable-object in pylint When run with Python 3.9, pylint incorrectly warns about things like Optional[foo] because it doesn't recognise Optional as unsubscriptable. This is a known pylint bug: https://github.com/PyCQA/pylint/issues/3882 Just disable this check to get rid of the warnings. Disabling this shouldn't make us miss any real bug because mypy also has a similar check ("... is not indexable"). Signed-off-by: Kevin Wolf Message-Id: <20201027163806.290960-3-kwolf@redhat.com> Reviewed-by: John Snow Signed-off-by: Kevin Wolf --- tests/qemu-iotests/pylintrc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/qemu-iotests/pylintrc b/tests/qemu-iotests/pylintrc index 5481afe528..cd3702e23c 100644 --- a/tests/qemu-iotests/pylintrc +++ b/tests/qemu-iotests/pylintrc @@ -17,6 +17,8 @@ disable=invalid-name, too-many-lines, too-many-locals, too-many-public-methods, + # pylint warns about Optional[] etc. as unsubscriptable in 3.9 + unsubscriptable-object, # These are temporary, and should be removed: missing-docstring, From 757ae0ec5cc6849b88280e2aaca70fe5eb022a0d Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Tue, 27 Oct 2020 17:38:06 +0100 Subject: [PATCH 5/6] iotests: Use Python 3 style super() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pylint complains about the use of super with the current class and instance as arguments in VM.__init__(): iotests.py:546:8: R1725: Consider using Python 3 style super() without arguments (super-with-arguments) No reason not to follow the advice and make it happy, so let's do this. Signed-off-by: Kevin Wolf Message-Id: <20201027163806.290960-4-kwolf@redhat.com> Reviewed-by: John Snow Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Kevin Wolf --- tests/qemu-iotests/iotests.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py index 28388a0fbc..814804a4c6 100644 --- a/tests/qemu-iotests/iotests.py +++ b/tests/qemu-iotests/iotests.py @@ -543,10 +543,10 @@ class VM(qtest.QEMUQtestMachine): def __init__(self, path_suffix=''): name = "qemu%s-%d" % (path_suffix, os.getpid()) - super(VM, self).__init__(qemu_prog, qemu_opts, name=name, - test_dir=test_dir, - socket_scm_helper=socket_scm_helper, - sock_dir=sock_dir) + super().__init__(qemu_prog, qemu_opts, name=name, + test_dir=test_dir, + socket_scm_helper=socket_scm_helper, + sock_dir=sock_dir) self._num_drives = 0 def add_object(self, opts): From c9eb2f3e386840844bcc91e66d0a3475bc650780 Mon Sep 17 00:00:00 2001 From: AlexChen Date: Tue, 3 Nov 2020 17:42:56 +0800 Subject: [PATCH 6/6] block/vvfat: Fix bad printf format specifiers We should use printf format specifier "%u" instead of "%d" for argument of type "unsigned int". In addition, fix two error format problems found by checkpatch.pl: ERROR: space required after that ',' (ctx:VxV) + fprintf(stderr,"%s attributes=0x%02x begin=%u size=%d\n", ^ ERROR: line over 90 characters + fprintf(stderr, "%d, %s (%u, %d)\n", i, commit->path ? commit->path : "(null)", commit->param.rename.cluster, commit->action); Reported-by: Euler Robot Signed-off-by: Alex Chen Message-Id: <5FA12620.6030705@huawei.com> Signed-off-by: Kevin Wolf --- block/vvfat.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/block/vvfat.c b/block/vvfat.c index 5abb90e7c7..54807f82ca 100644 --- a/block/vvfat.c +++ b/block/vvfat.c @@ -1437,7 +1437,7 @@ static void print_direntry(const direntry_t* direntry) for(i=0;i<11;i++) ADD_CHAR(direntry->name[i]); buffer[j] = 0; - fprintf(stderr,"%s attributes=0x%02x begin=%d size=%d\n", + fprintf(stderr, "%s attributes=0x%02x begin=%u size=%u\n", buffer, direntry->attributes, begin_of_direntry(direntry),le32_to_cpu(direntry->size)); @@ -1446,7 +1446,7 @@ static void print_direntry(const direntry_t* direntry) static void print_mapping(const mapping_t* mapping) { - fprintf(stderr, "mapping (%p): begin, end = %d, %d, dir_index = %d, " + fprintf(stderr, "mapping (%p): begin, end = %u, %u, dir_index = %u, " "first_mapping_index = %d, name = %s, mode = 0x%x, " , mapping, mapping->begin, mapping->end, mapping->dir_index, mapping->first_mapping_index, mapping->path, mapping->mode); @@ -1454,7 +1454,7 @@ static void print_mapping(const mapping_t* mapping) if (mapping->mode & MODE_DIRECTORY) fprintf(stderr, "parent_mapping_index = %d, first_dir_index = %d\n", mapping->info.dir.parent_mapping_index, mapping->info.dir.first_dir_index); else - fprintf(stderr, "offset = %d\n", mapping->info.file.offset); + fprintf(stderr, "offset = %u\n", mapping->info.file.offset); } #endif @@ -1588,7 +1588,7 @@ typedef struct commit_t { static void clear_commits(BDRVVVFATState* s) { int i; -DLOG(fprintf(stderr, "clear_commits (%d commits)\n", s->commits.next)); +DLOG(fprintf(stderr, "clear_commits (%u commits)\n", s->commits.next)); for (i = 0; i < s->commits.next; i++) { commit_t* commit = array_get(&(s->commits), i); assert(commit->path || commit->action == ACTION_WRITEOUT); @@ -2648,7 +2648,9 @@ static int handle_renames_and_mkdirs(BDRVVVFATState* s) fprintf(stderr, "handle_renames\n"); for (i = 0; i < s->commits.next; i++) { commit_t* commit = array_get(&(s->commits), i); - fprintf(stderr, "%d, %s (%d, %d)\n", i, commit->path ? commit->path : "(null)", commit->param.rename.cluster, commit->action); + fprintf(stderr, "%d, %s (%u, %d)\n", i, + commit->path ? commit->path : "(null)", + commit->param.rename.cluster, commit->action); } #endif