Re: [PATCH v5 03/31] assertions for block global state API

[Hanna Reitz]

On 24.11.21 07:43, Emanuele Giuseppe Esposito wrote:
> All the global state (GS) API functions will check that
> qemu_in_main_thread() returns true. If not, it means
> that the safety of BQL cannot be guaranteed, and
> they need to be moved to I/O.
>
> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
> ---
>   block.c        | 135 ++++++++++++++++++++++++++++++++++++++++++++++++-
>   block/commit.c |   2 +
>   block/io.c     |  14 +++++
>   blockdev.c     |   1 +
>   4 files changed, 150 insertions(+), 2 deletions(-)
>
> diff --git a/block.c b/block.c
> index 84de6867e6..49bee69e27 100644
> --- a/block.c
> +++ b/block.c

[...]

> @@ -975,6 +992,7 @@ BlockDriver *bdrv_probe_all(const uint8_t *buf, int 
> buf_size,
>   {
>       int score_max = 0, score;
>       BlockDriver *drv = NULL, *d;
> +    assert(qemu_in_main_thread());

While reviewing patch 13 and the find_image_format() it touches, I 
noticed that this function is called from raw_co_pwritev() to prevent 
the guest from writing image headers into probed-raw images.

Reproducible like so:

$ qemu-img create -f raw test.img 64M
Formatting 'test.img', fmt=raw size=67108864
$ ./qemu-system-x86_64 \
    -object iothread,id=iothr0 \
    -drive if=none,id=drv0,file=test.img \
    -device virtio-blk,drive=drv0,iothread=iothr0 \
    -monitor stdio
WARNING: Image format was not specified for 'test.img' and probing 
guessed raw.
         Automatically detecting the format is dangerous for raw 
images, write operations on block 0 will be restricted.
         Specify the 'raw' format explicitly to remove the restrictions.
QEMU 6.1.93 monitor - type 'help' for more information
(qemu) qemu-io drv0 "write 0 512"
qemu-system-x86_64: ../block.c:1004: bdrv_probe_all: Assertion 
`qemu_in_main_thread()' failed.
[1]    108256 IOT instruction (core dumped)  ./qemu-system-x86_64 
-object iothread,id=iothr0 -drive  -device  -monitor


I don’t think there’s any reason not to classify bdrv_probe_all() as an 
I/O function then, is there?

(Also, bdrv_probe_all() is part of block_int.h, so this assertion would 
actually belong in patch 8, not here.)

Hanna

>   
>       QLIST_FOREACH(d, &bdrv_drivers, list) {
>           if (d->bdrv_probe) {