* [PULL 1/6] qmp: fix aio_poll() assertion failure on Windows
2020-11-03 15:26 [PULL 0/6] Block layer patches Kevin Wolf
@ 2020-11-03 15:26 ` Kevin Wolf
2020-11-03 15:26 ` [PULL 2/6] qemu-img convert: Free @sn_opts in all error cases Kevin Wolf
` (5 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Kevin Wolf @ 2020-11-03 15:26 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
From: Volker Rümelin <vr_qemu@t-online.de>
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 <vr_qemu@t-online.de>
Message-Id: <20201021064033.8600-1-vr_qemu@t-online.de>
Tested-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
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
--
2.28.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PULL 2/6] qemu-img convert: Free @sn_opts in all error cases
2020-11-03 15:26 [PULL 0/6] Block layer patches Kevin Wolf
2020-11-03 15:26 ` [PULL 1/6] qmp: fix aio_poll() assertion failure on Windows Kevin Wolf
@ 2020-11-03 15:26 ` Kevin Wolf
2020-11-03 15:26 ` [PULL 3/6] iotests.py: Fix type check errors in wait_migration() Kevin Wolf
` (4 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Kevin Wolf @ 2020-11-03 15:26 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
From: Tuguoyi <tu.guoyi@h3c.com>
@sn_opts is initialized at the beginning, so it should be deleted
after jumping to the lable 'fail_getopt'
Signed-off-by: Guoyi Tu <tu.guoyi@h3c.com>
Message-Id: <6ff1c5d372944494be3932274f75485d@h3c.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
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;
--
2.28.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PULL 3/6] iotests.py: Fix type check errors in wait_migration()
2020-11-03 15:26 [PULL 0/6] Block layer patches Kevin Wolf
2020-11-03 15:26 ` [PULL 1/6] qmp: fix aio_poll() assertion failure on Windows Kevin Wolf
2020-11-03 15:26 ` [PULL 2/6] qemu-img convert: Free @sn_opts in all error cases Kevin Wolf
@ 2020-11-03 15:26 ` Kevin Wolf
2020-11-03 15:26 ` [PULL 4/6] iotests: Disable unsubscriptable-object in pylint Kevin Wolf
` (3 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Kevin Wolf @ 2020-11-03 15:26 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
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 <kwolf@redhat.com>
Message-Id: <20201027163806.290960-2-kwolf@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
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
--
2.28.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PULL 4/6] iotests: Disable unsubscriptable-object in pylint
2020-11-03 15:26 [PULL 0/6] Block layer patches Kevin Wolf
` (2 preceding siblings ...)
2020-11-03 15:26 ` [PULL 3/6] iotests.py: Fix type check errors in wait_migration() Kevin Wolf
@ 2020-11-03 15:26 ` Kevin Wolf
2020-11-03 15:26 ` [PULL 5/6] iotests: Use Python 3 style super() Kevin Wolf
` (2 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Kevin Wolf @ 2020-11-03 15:26 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
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 <kwolf@redhat.com>
Message-Id: <20201027163806.290960-3-kwolf@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
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,
--
2.28.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PULL 5/6] iotests: Use Python 3 style super()
2020-11-03 15:26 [PULL 0/6] Block layer patches Kevin Wolf
` (3 preceding siblings ...)
2020-11-03 15:26 ` [PULL 4/6] iotests: Disable unsubscriptable-object in pylint Kevin Wolf
@ 2020-11-03 15:26 ` Kevin Wolf
2020-11-03 15:26 ` [PULL 6/6] block/vvfat: Fix bad printf format specifiers Kevin Wolf
2020-11-03 16:53 ` [PULL 0/6] Block layer patches Peter Maydell
6 siblings, 0 replies; 8+ messages in thread
From: Kevin Wolf @ 2020-11-03 15:26 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
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 <kwolf@redhat.com>
Message-Id: <20201027163806.290960-4-kwolf@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
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):
--
2.28.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PULL 6/6] block/vvfat: Fix bad printf format specifiers
2020-11-03 15:26 [PULL 0/6] Block layer patches Kevin Wolf
` (4 preceding siblings ...)
2020-11-03 15:26 ` [PULL 5/6] iotests: Use Python 3 style super() Kevin Wolf
@ 2020-11-03 15:26 ` Kevin Wolf
2020-11-03 16:53 ` [PULL 0/6] Block layer patches Peter Maydell
6 siblings, 0 replies; 8+ messages in thread
From: Kevin Wolf @ 2020-11-03 15:26 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
From: AlexChen <alex.chen@huawei.com>
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 <euler.robot@huawei.com>
Signed-off-by: Alex Chen <alex.chen@huawei.com>
Message-Id: <5FA12620.6030705@huawei.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
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
--
2.28.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PULL 0/6] Block layer patches
2020-11-03 15:26 [PULL 0/6] Block layer patches Kevin Wolf
` (5 preceding siblings ...)
2020-11-03 15:26 ` [PULL 6/6] block/vvfat: Fix bad printf format specifiers Kevin Wolf
@ 2020-11-03 16:53 ` Peter Maydell
6 siblings, 0 replies; 8+ messages in thread
From: Peter Maydell @ 2020-11-03 16:53 UTC (permalink / raw)
To: Kevin Wolf; +Cc: QEMU Developers, Qemu-block
On Tue, 3 Nov 2020 at 15:27, Kevin Wolf <kwolf@redhat.com> wrote:
>
> The following changes since commit 83851c7c60c90e9fb6a23ff48076387a77bc33cd:
>
> Merge remote-tracking branch 'remotes/mdroth/tags/qga-pull-2020-10-27-v3-tag' into staging (2020-11-03 12:47:58 +0000)
>
> are available in the Git repository at:
>
> git://repo.or.cz/qemu/kevin.git tags/for-upstream
>
> for you to fetch changes up to c9eb2f3e386840844bcc91e66d0a3475bc650780:
>
> block/vvfat: Fix bad printf format specifiers (2020-11-03 16:24:56 +0100)
>
> ----------------------------------------------------------------
> Block layer patches:
>
> - iotests: Fix pylint/mypy warnings with Python 3.9
> - qmp: fix aio_poll() assertion failure on Windows
> - Some minor fixes
>
> ----------------------------------------------------------------
Applied, thanks.
Please update the changelog at https://wiki.qemu.org/ChangeLog/5.2
for any user-visible changes.
-- PMM
^ permalink raw reply [flat|nested] 8+ messages in thread