Re: [Qemu-devel] [PATCH v8 03/26] block: Add BDS.backing_overridden

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2680 bytes --]

So, while tackling this once more, this is the current state, in short.

I wanted to add a new field BDS.backing_file_canonical which basically
gets set to bs->backing->bs->filename whenever we know that bs->backing
was opened through bs->backing_file.  That is,
bs->backing_file_canonical would be bs->backing_file, but after going
through the whole hoop of bdrv_open() and bdrv_refresh_filename().

With such a field, we could compare bs->backing->bs->filename against
bs->backing_file_canonical, and thus see whether the backing BDS matches
the image header filename.  I hope this would do the right thing
whenever qemu tries to figure out backing filenames on its own; it will
break, though, when the user just uses some filename and that doesn't
happen to coincide with qemu's generated filenames.

(For instance, if you create a new overlay manually, give it some
backing filename, and that backing filename isn't 1:1 what qemu would
reconstruct; and then you open that file with backing=null, and do a
blockdev-snapshot; then qemu will give you a json:{} filename for the
overlay because due to backing=null, it didn't open the overlay's
backing_file and thus doesn't know how it would look reconstructed.  But
I think that's something we could live with.)

((The only real way to fix this I can imagine is to clone the whole
bdrv_parse_filename()/bdrv_open()/bdrv_refresh_filename()
infrastructure, but just for filenames instead of real BDSs.  Yeah, no,
I won't do that.))

OK, so that was an idea, but now it turns out that bs->backing_file
actually isn't that image header's idea of the backing file.  It appears
to mostly be a clone of bs->backing->bs->filename, but not really.
Honestly, I have no idea what it is, but as a matter of fact, it is
modified by bdrv_backing_attach(), so it will probably usually match
bs->backing->bs->filename (unless there is no bs->backing, in which case
it just retains its old value?).  So it pretty much is completely
useless for any comparison here.

So I suppose I'll rename my BDS.backing_file_canonical attempt to
BDS.auto_backing_file, and this will be what the image header contains
(unless we have opened a BDS from it, in which case it will be that
BDS's filename, so it is canonicalized).

Well, or I could just go with backing_overridden, because honestly that
didn't seem so bad.

Max


PS: Or, we could argue that nobody needs filenames anyway and that
they're just for show and debugging nowadays, so nobody actually needs
backing chain information in them.  May sound a bit stupid, but then
again nobody has ever complained that that's in fact the current state.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]