|
From: | Hanna Reitz |
Subject: | Re: [PATCH v5 22/31] block_int-common.h: assertion in the callers of BlockDriver function pointers |
Date: | Thu, 16 Dec 2021 19:43:47 +0100 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.3.0 |
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).
Hanna
+ job_progress_set_remaining(&s->common, 1); ret = s->drv->bdrv_co_create(s->opts, errp); job_progress_update(&s->common, 1);
[Prev in Thread] | Current Thread | [Next in Thread] |