[libvirt PATCH 2/2] storage_file: add support to probe cluster_size from QCOW2 images

[Pavel Hrdina]

On Wed, May 19, 2021 at 02:57:18PM +0200, Peter Krempa wrote:
> On Thu, May 13, 2021 at 13:23:05 +0200, Pavel Hrdina wrote:
> > >From QEMU docs/interop/qcow2.txt :
> > 
> >    Byte  20 - 23:   cluster_bits
> >                     Number of bits that are used for addressing an offset
> >                     within a cluster (1 << cluster_bits is the cluster size).
> > 
> > With this patch libvirt will be able to report the current cluster_size
> > for all existing storage volumes managed by storage driver.
> > 
> > Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
> > ---
> >  src/storage/storage_util.c            |  3 ++
> >  src/storage_file/storage_file_probe.c | 55 +++++++++++++++++++--------
> >  2 files changed, 42 insertions(+), 16 deletions(-)
> 
> [...]
> 
> > diff --git a/src/storage_file/storage_file_probe.c b/src/storage_file/storage_file_probe.c
> > index afe64da02e..423597049f 100644
> > --- a/src/storage_file/storage_file_probe.c
> > +++ b/src/storage_file/storage_file_probe.c
> > @@ -94,6 +94,9 @@ struct FileTypeInfo {
> >                            /* Store a COW base image path (possibly relative),
> >                             * or NULL if there is no COW base image, to RES;
> >                             * return BACKING_STORE_* */
> > +    int clusterBitsOffset; /* Byte offset from start of file where we find
> > +                            * number of cluster bits, -1 to skip. */
> > +    int clusterBitsSize;   /* Number of bytes for cluster bits. */
> 
> This isn't actually used [1].

Good point, I'll drop it.

> >      const struct FileEncryptionInfo *cryptInfo; /* Encryption info */
> >      int (*getBackingStore)(char **res, int *format,
> >                             const char *buf, size_t buf_size);
> 
> Given how the value is stored in the header it seems that it won't
> really be usable for any other format. I think we should parse it via a
> callback rather than the generic offset/size parser.
> 
> Specifically since the cluster size is stored as number of bits.

That will be way better then mangling with the crazy struct. I'll change
it for v2.

Thanks for the review, I'll make the changes and once we figure out the
placement of cluster_size I'll post v2.

Pavel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20210519/44f15c81/attachment-0001.sig>