Re: Using json: in common.rc's TEST_IMG

Re: Using json: in common.rc's TEST_IMG

[Hanna Czenczek]

On 31.05.23 21:09, Stefan Hajnoczi wrote:
> Another issue is that 145 uses $TEST_IMG as follows:
>
>    SYSEMU_DRIVE_ARG=if=none,file="$TEST_IMG",driver=$IMGFMT
>
> That doesn't work when json: contains a comma, since commas need to be
> doubled up to escape them. This fails:
>
>    $ qemu-system-x86_64 -drive
> 'if=none,file=json:{"driver":"io_uring","filename":"test.img"}'
>    qemu-system-x86_64: -drive
> if=none,file=json:{"driver":"io_uring","filename":"test.img"}:
> warning: short-form boolean option '"filename":"test.img"}' deprecated
>    Please use "filename":"test.img"}=on instead
>
> This works:
>
>    qemu-system-x86_64 -drive
> 'if=none,file=json:{"driver":"io_uring",,"filename":"test.img"}'
>
> Maybe it's simply not possible to use TEST_IMG=json: in qemu-iotests?

Probably not as-is.  None of the tests has been written with that in 
mind, and many have been written at a time where TEST_IMG was basically 
always a plain filename.  We’ve had a lot of churn in the past e.g. to 
separate TEST_IMG_FILE out from TEST_IMG, and I suspect if you want 
json:{} to work, that would be even more churn.

Admittedly I don’t remember how it’s to be done, but looking at the code 
(common.rc starting from line 274), it seems clear what other protocols 
do: If $IMGOPTSSYNTAX is true, use a dotted-key-value syntax; and 
otherwise, make use of the protocol prefix.  Now, "io_uring:test.img" 
doesn’t work, because while bdrv_io_uring has .protocol_name set, it 
doesn’t implement .bdrv_parse_filename(). file-posix for example does 
(see raw_parse_filename()), and all it does is to strip the protocol prefix.

So I think you should be able to get the non-IMGOPTSSYNTAX case to work 
by adding a trivial .bdrv_parse_filename() implementation to each blkio 
driver, which just strips the protocol prefix, and then use 
"io_uring:$TEST_IMG_FILE".

Hanna

> The alternative is to always set IMGOPTSSYNTAX=true and then find the
> test cases that fail because they contain non-IMGOPTSSYNTAX commands.
>
> Stefan
>

Re: Using json: in common.rc's TEST_IMG

[Kevin Wolf]

Am 31.05.2023 um 21:00 hat Stefan Hajnoczi geschrieben:
> Hi Hanna,
> I'm adding the io_uring block driver (block/blkio.c) to qemu-iotests
> and hit a syntax issue with json:.
> 
> In tests/qemu-iotests/common.rc where TEST_IMG_FILE and TEST_IMG are
> defined for each IMGPROTO, I'm unable to come up with a TEST_IMG that
> works with all test cases. Some test cases want:
> 
>   TEST_IMG="json:{\"file.driver\":\"io_uring\",\"file.filename\":\"$TEST_IMG_FILE\"}"
> 
> While others want:
> 
>   TEST_IMG="json:{\"driver\":\"io_uring\",\"filename\":\"$TEST_IMG_FILE\"}"
> 
> It seems to depend on how TEST_IMG is used by the tests.

The first one makes more sense, doesn't the second one override the
image format and effectively use raw images by going to the protocol
driver directly?

But should we use json: at all? All the other protocols don't make use
of it, but just use the real protocol prefix, i.e. what I would expect
here is io_uring:/foo/bar (which is already covered by the else branch
in common.rc).

Maybe this doesn't work currently for the libblkio block driver because
it doesn't implement .bdrv_parse_filename, but it would be nice to
support for human users anyway, so maybe just implement it as a small
wrapper around bdrv_parse_filename_strip_prefix()?

> For example, 001 works with the first version. When I use the second
> version it fails with:
> 
>   qemu-io: can't open device
> json:{"driver":"io_uring","filename":"/home/stefanha/qemu/build/scratch/raw-io_uring-001/t.raw"}:
> A block device must be specified for "file"
> 
> The opposite is true for 077, which works with the second version but
> fails as follows with the first version:
> 
>   qemu-io: can't open device
> blkdebug::json:{"file.driver":"io_uring","file.filename":"/home/stefanha/qemu/build/scratch/raw-io_uring-077/t.raw"}:
> Must specify either driver or file
> 
> This only seems to happen with json: because the other protocols use
> TEST_IMG=nfs://... and so on without any problems.
> 
> Any ideas?

Another problem I remember with json: is that you need different
escaping in qemu-img and -drive in qemu-system-*: The latter requires
commas to be doubled.

So with a single $TEST_IMG, you won't be able to cover both the tools
and the system emulator anyway.

Kevin

Re: Using json: in common.rc's TEST_IMG

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 391 bytes --]

Thanks, Hanna and Kevin!

I didn't implement bdrv_parse_filename() because I didn't want to invent
more legacy syntax. But maybe that legacy syntax does have a use and I just
didn't realize it :).

It's easy for io_uring, we can just take the filename and not worry about
other parameters (they can largely be set via existing QEMU command-line
syntax already).

I'll give it a try.

Stefan

[-- Attachment #2: Type: text/html, Size: 622 bytes --]