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

[Max Reitz <mreitz@redhat.com>]

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. 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.

Max