On 16/12/2021 19:43, Hanna Reitz wrote:
On 24.11.21 07:44, Emanuele Giuseppe Esposito wrote:
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
block.c | 18 ++++++++++++++++++
block/create.c | 10 ++++++++++
2 files changed, 28 insertions(+)
[...]
diff --git a/block/create.c b/block/create.c
index 89812669df..0167118579 100644
--- a/block/create.c
+++ b/block/create.c
@@ -42,6 +42,16 @@ static int coroutine_fn blockdev_create_run(Job
*job, Error **errp)
BlockdevCreateJob *s = container_of(job, BlockdevCreateJob,
common);
int ret;
+ /*
+ * Currently there is nothing preventing this
+ * function from being called in an iothread context.
+ * However, since it will crash anyways because of the
+ * aiocontext lock not taken, we might as well make it
+ * crash with a more meaningful error, by checking that
+ * we are in the main loop
+ */
+ assert(qemu_in_main_thread());
Mostly agreed. This function is always run in the main loop right
now, so this assertion will never fail.
But that’s the “mostly”: Adding this assertion won’t give a more
meaningful error, because the problem still remains that block
drivers do not error out when encountering (or correctly handle) BDSs
in non-main contexts, and so it remains a “qemu_mutex_unlock_impl:
Operation not permitted”.
Not trying to say that that’s your problem. It’s pre-existing, and
this assertion is good. Just wanting to clarify something about the
comment that seemed unclear to me (in that I found it implied that
the qemu_mutex_unlock_impl error would be replaced by this assertion
failing).
You are right. Trying your example given in v4:
$ touch /tmp/iothread-create-test.qcow2
$ ./qemu-system-x86_64 -object iothread,id=iothr0 -qmp stdio <<EOF
{"execute": "qmp_capabilities"}
{"execute":"blockdev-add","arguments":{"node-name":"proto","driver":"file","filename":"/tmp/iothread-create-test.qcow2"}}
{"execute":"x-blockdev-set-iothread","arguments":{"node-name":"proto","iothread":"iothr0"}}
{"execute":"blockdev-create","arguments":{"job-id":"create","options":{"driver":"qcow2","file":"proto","size":0}}}
EOF
I still get "qemu_mutex_unlock_impl: Operation not permitted"
I will remove the comment above the assertion, makes no sense.
Or should I replace it with a TODO/FIXME explaining the above?
Something like:
/*
* TODO: it is currently possible to run a blockdev-create job in an
* I/O thread, for example by doing:
* [ command line above ]
* This should not be allowed.
*/