Re: [Qemu-devel] blkdebug and VMDK (iotests 033 failure on monolithicFlat)

[Fam Zheng]

On Wed, 05/06 16:41, Max Reitz wrote:
> On 06.05.2015 16:38, Fam Zheng wrote:
> >On Wed, 05/06 16:12, Max Reitz wrote:
> >>On 06.05.2015 15:59, Fam Zheng wrote:
> >>>On Wed, 05/06 15:45, Max Reitz wrote:
> >>>>On 05.05.2015 12:44, Fam Zheng wrote:
> >>>>>Max,
> >>>>>
> >>>>>Since you once fixed VMDK with the json descriptor filename, could you 
> >>>>>take a
> >>>>>look at the error:
> >>>>>
> >>>>>$ ./check -vmdk -o "subformat=monolithicFlat" 033
> >>>>>
> >>>>>033         - output mismatch (see 033.out.bad)
> >>>>>--- /home/fam/qemu/tests/qemu-iotests/033.out   2015-05-05 
> >>>>>10:52:50.524378312 +0800
> >>>>>+++ 033.out.bad 2015-05-05 17:38:36.147369924 +0800
> >>>>>@@ -2,54 +2,36 @@
> >>>>>  Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
> >>>>>  == preparing image ==
> >>>>>-wrote 1024/1024 bytes at offset 512
> >>>>>-1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> >>>>>-wrote 1536/1536 bytes at offset 131072
> >>>>>-1.500 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> >>>>>-wrote 131072/131072 bytes at offset 1024
> >>>>>-128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> >>>>>+qemu-io: can't open device blkdebug::/tmp/qemu-iotests/t.vmdk: Cannot 
> >>>>>use relative extent paths with VMDK descriptor file 'json:{"image": 
> >>>>>{"driver": "file", "filename": "/tmp/qemu-iotests/t.vmdk"}, "driver": 
> >>>>>"blkdebug", "align": "512"}'
> >>>>>+qemu-io: can't open device blkdebug::/tmp/qemu-iotests/t.vmdk: Cannot 
> >>>>>use relative extent paths with VMDK descriptor file 'json:{"image": 
> >>>>>{"driver": "file", "filename": "/tmp/qemu-iotests/t.vmdk"}, "driver": 
> >>>>>"blkdebug", "align": "512"}'
> >>>>>+qemu-io: can't open device blkdebug::/tmp/qemu-iotests/t.vmdk: Cannot 
> >>>>>use relative extent paths with VMDK descriptor file 'json:{"image": 
> >>>>>{"driver": "file", "filename": "/tmp/qemu-iotests/t.vmdk"}, "driver": 
> >>>>>"blkdebug", "align": "512"}'
> >>>>>
> >>>>>...
> >>>>>
> >>>>>
> >>>>>I'm not sure what's the best fix here. It's possible to dig out
> >>>>>bs->file->file->filename in this case, but I'm not sure what's the rule 
> >>>>>of this
> >>>>>filename mystery.
> >>>>>
> >>>>>Fam
> >>>>Hi Fam,
> >>>>
> >>>>The problem is that the align option is (consciously) set for blkdebug, so
> >>>>the blkdebug driver cannot just convert it to a blkdebug::$foo filename, 
> >>>>but
> >>>>must create a JSON filename so that the align option is included.
> >>>>
> >>>>I seem to recall that someone (me? I don't know) once proposed a way for
> >>>>block drivers not only to present a file name (which is json:{${baz}} in
> >>>>this case), but also the name of the directory they are in, or better, a
> >>>>template to which a relative path can be appended (in Unix syntax, that 
> >>>>is,
> >>>>with / as separators and .. and so on). The difference to the file name
> >>>>would be that we can ignore all the options given to the block driver for
> >>>>controlling its behavior, we just need a path name.
> >>>>
> >>>>I don't know why we didn't pursue that proposal, but it does seem to be
> >>>>necessary here.
> >>>>
> >>>>I think we can integrate it into the existing bdrv_refresh_filename()
> >>>>"infrastructure", so it shouldn't be too difficult. I can take a shot at 
> >>>>it,
> >>>>if you agree.
> >>>>
> >>>Sounds consistent with what we have now, but to be honest I don't have the
> >>>background to agree or disagree. Could you explain (again) why does 
> >>>filename
> >>>have to reflect its option?
> >>Because in a certain way, having the align option or not results in a
> >>different file (at least in a different behavior). While one could argue
> >>that in this case, it does not make that much of a difference, one can
> >>easily conceive scenarios in which it does (for instance, for blkdebug it
> >>might be an inject-error option). The idea is that the filename reported for
> >>a BDS should result in the same BDS being constructed when used, or at least
> >>one which behaves in the same way.
> >Who's making use of the json file name?
> 
> qemu-img info and block-info, for one. Other than that, it can be used as a
> backing file name (where you can only specify a filename, but no additional
> options).

OK, get it now. This is kind of convenient, though I don't know whether it's to
my taste. Since it's already here, and as you know the idea much better than
me, please go ahead with what you propsed in bdrv_refresh_filename. :)

Thanks!
Fam

> 
> Max
> 
> >>If you open a file named blkdebug::x.img
> >>with -o inject-error.event=read_aio, then just reporting blkdebug::x.img
> >>would not be correct, since opening a BDS using the filename blkdebug::x.img
> >>alone would not result in a BDS behaving the same. Therefore, we have to
> >>generate a JSON object as the filename which includes this option.
> >>
> >>So while we could solve the problem in this case by ignoring the "align"
> >>option when constructing the filename for blkdebug, it would not work for
> >>the general case.
> >>
> >
>