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@xxxxxxxxxx>
> ---
> 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].
> 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.
> @@ -116,7 +119,8 @@ qedGetBackingStore(char **, int *, const char *, size_t);
> #define QCOWX_HDR_VERSION (4)
> #define QCOWX_HDR_BACKING_FILE_OFFSET (QCOWX_HDR_VERSION+4)
> #define QCOWX_HDR_BACKING_FILE_SIZE (QCOWX_HDR_BACKING_FILE_OFFSET+8)
> -#define QCOWX_HDR_IMAGE_SIZE (QCOWX_HDR_BACKING_FILE_SIZE+4+4)
> +#define QCOWX_HDR_CLUSTER_BITS_OFFSET (QCOWX_HDR_BACKING_FILE_SIZE+4)
> +#define QCOWX_HDR_IMAGE_SIZE (QCOWX_HDR_CLUSTER_BITS_OFFSET+4)
>
> #define QCOW1_HDR_CRYPT (QCOWX_HDR_IMAGE_SIZE+8+1+1+2)
> #define QCOW2_HDR_CRYPT (QCOWX_HDR_IMAGE_SIZE+8)
> G_STATIC_ASSERT(G_N_ELEMENTS(fileTypeInfo) == VIR_STORAGE_FILE_LAST);
> @@ -890,6 +896,23 @@ virStorageFileProbeGetMetadata(virStorageSource *meta,
> meta->capacity *= fileTypeInfo[meta->format].sizeMultiplier;
> }
>
> + if (fileTypeInfo[meta->format].clusterBitsOffset != -1) {
> + int clusterBits = 0;
> +
> + if ((fileTypeInfo[meta->format].clusterBitsOffset + 4) > len)
[1] ... you hardcode the value.
> + return 0;
> +
> + if (fileTypeInfo[meta->format].endian == LV_LITTLE_ENDIAN)
> + clusterBits = virReadBufInt32LE(buf +
> + fileTypeInfo[meta->format].clusterBitsOffset);
> + else
> + clusterBits = virReadBufInt32BE(buf +
> + fileTypeInfo[meta->format].clusterBitsOffset);
> +
> + if (clusterBits > 0)
> + meta->clusterSize = 1 << clusterBits;
> + }
> +
> VIR_FREE(meta->backingStoreRaw);
> if (fileTypeInfo[meta->format].getBackingStore != NULL) {
> int store = fileTypeInfo[meta->format].getBackingStore(&meta->backingStoreRaw,
> --
> 2.31.1
>
