On 14.05.19 13:30, Alberto Garcia wrote: > On Sat 13 Apr 2019 02:53:42 AM CEST, Max Reitz wrote: >>> 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... > > Why would both be fine? backing_file refers to the backing file > currently attached, and auto_backing_file refers to the one written on > the image metadata, or am I wrong?
After my series, backing_file refers to the image metadata. >>> 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. > > But what's the point of having backing_file set if no backing file is > attached? What’s the point of having backing_file set to the same value as bs->backing->bs->filename? We need some storage for “What does the image header say”. Currently, that sometimes is BDS.backing_file. Sometimes it isn’t. That’s broken. See http://lists.nongnu.org/archive/html/qemu-block/2019-04/msg00308.html. BDS.auto_backing_file does not refer to the image metadata. It refers to something like bdrv_refresh_filename(bdrv_open(bs->backing_file)). We need this just to calculate @backing_overridden in bdrv_refresh_filename(). Max
signature.asc
Description: OpenPGP digital signature
