Re: [Qemu-block] [PATCH v2 13/13] qemu-iotests: Test the x-blockdev-reopen QMP command

[Max Reitz]

On 11.04.19 15:41, Alberto Garcia wrote:
> On Wed 10 Apr 2019 07:03:50 PM CEST, Max Reitz wrote:
>>> +        # hd0 has no backing file: we can omit the 'backing' option
>>> +        self.reopen(opts)
>>
>> [...]
>>
>>> +        # Detach hd2 from hd0.
>>> +        self.reopen(opts, {'backing': None})
>>> +        self.reopen(opts, {}, "backing is missing for 'hd0'")
>>
>> I don’t understand the second test.  hd0 has no default backing file
>> and it currently has no backing child attached to it.  Why would this
>> call fail now?
> 
> I think there's a bug.
> 
> Calling x-blockdev-reopen without 'backing' should only fail if
> 
>  a) the image has a backing file attached to it.
>     In this case it doesn't: we just detached it in the previous line.
> 
>  b) there's a default backing file written on the image header.
>     In this case there isn't (hd0 is created without one in setUp()).

That’s what I thought, too, hence me applying effectively the same
change to the test in v4 of my series as you in your "Fix check for
default backing files" series:

http://lists.nongnu.org/archive/html/qemu-block/2019-04/msg00308.html

> So it should not fail. I think the bug is that the test for condition
> (b) in bdrv_reopen_prepare() that returns "backing is missing..." is
> using backing_file but it should use (correct me if I'm wrong)
> auto_backing_file.

Well, I think both should be fine, because...

> Changing that and replacing the test line with self.reopen(opts) fixes
> it for me.
> 
> Not directly related to this, but should bdrv_backing_detach() also
> clear backing_file ?

...I don’t think it should.  That’s what that my patch addresses.  The
real problem is that bs->backing_file is not a cache for
bs->backing->bs->filename, so it shouldn’t be treated as such.

Max

signature.asc
Description: OpenPGP digital signature