On Tue, May 23, 2023 at 06:39:00PM -0300, Fabiano Rosas wrote:
> From: Lin Ma <l...@suse.com>
>
> Sometimes the query-block performs time-consuming I/O(say waiting for
> the fstat of NFS complete), So let's make this QMP handler runs in a
> coroutine.
Grammar suggestions:
Sometimes the query-block command performs time-consuming I/O (say
waiting for the fstat of NFS to complete), so let's make this QMP
handler run in a coroutine.
>
> The following patch moves the fstat() into a thread pool.
>
> Signed-off-by: Lin Ma <l...@suse.com>
> Signed-off-by: Fabiano Rosas <faro...@suse.de>
> ---
> +++ b/qapi/block-core.json
> @@ -838,7 +838,7 @@
> # }
> ##
> { 'command': 'query-block', 'returns': ['BlockInfo'],
> - 'allow-preconfig': true }
> + 'allow-preconfig': true, 'coroutine': true }
Rereading docs/devel/qapi-code-gen.rst:
| Coroutine safety can be hard to prove, similar to thread safety. Common
| pitfalls are:
|
| - The global mutex isn't held across ``qemu_coroutine_yield()``, so
| operations that used to assume that they execute atomically may have
| to be more careful to protect against changes in the global state.
|
| - Nested event loops (``AIO_WAIT_WHILE()`` etc.) are problematic in
| coroutine context and can easily lead to deadlocks. They should be
| replaced by yielding and reentering the coroutine when the condition
| becomes false.
|
| Since the command handler may assume coroutine context, any callers
| other than the QMP dispatcher must also call it in coroutine context.
| In particular, HMP commands calling such a QMP command handler must be
| marked ``.coroutine = true`` in hmp-commands.hx.
I agree that you also need to change the HMP handler, but the commit
message didn't mention that other than in the subject line. The
commit message could also do a better job at explaining how you have
audited that merely adding the coroutine marker is safe (ie. that all
of the calls made by query_block are already coroutine safe). While
the intent behind this patch is on the right track, I'm not sure if I
have enough information to say whether it is safe, or if there are
other lurking problems we will need to fix first.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
