Re: [PATCH v2 03/12] block/vmdk: Change extent info type

[Eric Blake]

On Thu, Jan 19, 2023 at 04:20:16PM +0100, Kevin Wolf wrote:
> Am 20.06.2022 um 18:26 hat Hanna Reitz geschrieben:
> > VMDK's implementation of .bdrv_get_specific_info() returns information
> > about its extent files, ostensibly in the form of ImageInfo objects.
> > However, it does not get this information through
> > bdrv_query_image_info(), but fills only a select few fields with custom
> > information that does not always match the fields' purposes.
> > 
> > For example, @format, which is supposed to be a block driver name, is
> > filled with the extent type, e.g. SPARSE or FLAT.
> > 
> > In ImageInfo, @compressed shows whether the data that can be seen in the
> > image is stored in compressed form or not.  For example, a compressed
> > qcow2 image will store compressed data in its data file, but when
> > accessing the qcow2 node, you will see normal data.  This is not how
> > VMDK uses the @compressed field for its extent files: Instead, it
> > signifies whether accessing the extent file will yield compressed data
> > (which the VMDK driver then (de-)compresses).
> 
> Actually, VMDK was the only user of the field in ImageInfo. qcow2
> doesn't set the field at all because it would have to parse all L2
> tables in order to set it.
> 
> So I suppose @compressed can now be removed from ImageInfo?

I think you are okay for VMDK (the new struct has the same field
names, but better meanings, and the on-the-wire representation for
someone querying a known-VMDK image doesn't change).  For non-VMDK,
any code that was querying @compressed will break, but arguably no one
was doing that since it would have always been false.  If we want to
be super-conservative, we deprecate the field now and only remove it
from ImageInfo in a later release, but I'd rather trust Markus on this
decision.

On a side note - would it be worth adding a bit to the qcow2 header
(one of the compatible_feature bits seems best) which we set when
writing a compressed cluster, so that on newer images, it is an O(1)
probe of whether the image contains (or at least has contained in the
past) a compressed cluster?  Or is that going to add needless overhead
for something we really don't need to know all that often?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org