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