Re: [libvirt] [PATCH v2 06/12] qemu: fix bugs in blockstats

The documentation for virDomainBlockInfo was confusing: it stated that 'physical' was the size of the container, then gave an example of it being the amount of storage used by a sparse file (that is, for a sparse raw image on a regular file, the wording implied capacity==physical, while allocation was smaller; but the example instead claimed physical==allocation). Since we use 'physical' for the last offset of a block device, we should do likewise for regular files. Furthermore, the example claimed that for a qcow2 regular file, allocation==physical. At the time the code was first written, this was true (qcow2 files were allocated sequentially, and were never sparse, so the last sector written happened to also match the disk space occupied); but modern qemu does much better and can punch holes for a qcow2 with allocation < physical. Basically, after this patch, the three fields are now reliably mapped as: 'capacity' - how much storage the guest can see (equal to physical for raw images, determined by image metadata otherwise) 'allocation' - how much storage the image occupies (similar to what 'du' would report) 'physical' - the last offset of the image (similar to what 'ls' would report) 'capacity' can be larger than 'physical' (such as for a qcow2 image that does not vary much from a backing file) or smaller (such as for a qcow2 file with lots of internal snapshots). Likewise, 'allocation' can be (slightly) larger than 'physical' (such as counting the tail of cluster allocations required to round a file size up to filesystem granularity) or smaller (for a sparse file). A block-resize operation changes capacity (which, for raw images, also changes physical); many non-raw images automatically grow physical and allocation as necessary when starting with an allocation smaller than capacity; and even when capacity and physical stay unchanged, allocation can change when converting sectors from holes to data or back. Note that this does not change semantics for qcow2 images stored on block devices; there, we still rely on qemu to report the highest written extent for allocation. So using this API to track when to extend a block device because a qcow2 image is about to exceed a threshold will not see any changes. * include/libvirt/libvirt-domain.h (_virDomainBlockInfo): Tweak documentation to match saner definition. * src/qemu/qemu_driver.c (qemuDomainGetBlockInfo): For regular files, physical size is capacity, not allocation. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- include/libvirt/libvirt-domain.h | 23 ++++++++++++++-------- src/qemu/qemu_driver.c | 42 ++++++++++++++++++++++------------------ 2 files changed, 38 insertions(+), 27 deletions(-) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index ae2c49c..baef32d 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -1356,7 +1356,7 @@ int virDomainBlockResize (virDomainPtr dom, /** virDomainBlockInfo: * * This struct provides information about the size of a block device - * backing store + * backing store. * * Examples: * @@ -1364,13 +1364,13 @@ int virDomainBlockResize (virDomainPtr dom, * * capacity, allocation, physical: All the same * * - Sparse raw file in filesystem: - * * capacity: logical size of the file - * * allocation, physical: number of blocks allocated to file + * * capacity, size: logical size of the file + * * allocation: disk space occupied by file * * - qcow2 file in filesystem * * capacity: logical size from qcow2 header - * * allocation, physical: logical size of the file / - * highest qcow extent (identical) + * * allocation: disk space occupied by file + * * physical: reported size of qcow2 file * * - qcow2 file in a block device * * capacity: logical size from qcow2 header @@ -1380,9 +1380,16 @@ int virDomainBlockResize (virDomainPtr dom, typedef struct _virDomainBlockInfo virDomainBlockInfo; typedef virDomainBlockInfo *virDomainBlockInfoPtr; struct _virDomainBlockInfo { - unsigned long long capacity; /* logical size in bytes of the block device backing image */ - unsigned long long allocation; /* highest allocated extent in bytes of the block device backing image */ - unsigned long long physical; /* physical size in bytes of the container of the backing image */ + unsigned long long capacity; /* logical size in bytes of the + * image (how much storage the + * guest will see) */ + unsigned long long allocation; /* host storage in bytes occupied + * by the image (such as highest + * allocated extent if there are no + * holes, similar to 'du') */ + unsigned long long physical; /* host physical size in bytes of + * the image container (last + * offset, similar to 'ls')*/ }; int virDomainGetBlockInfo(virDomainPtr dom, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f87c44b..5e9c133 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11108,18 +11108,21 @@ qemuDomainGetBlockInfo(virDomainPtr dom, /* Get info for normal formats */ if (S_ISREG(sb.st_mode) || fd == -1) { #ifndef WIN32 - disk->src->physical = (unsigned long long)sb.st_blocks * + disk->src->allocation = (unsigned long long)sb.st_blocks * (unsigned long long)DEV_BSIZE; #else + disk->src->allocation = sb.st_size; +#endif + /* Allocation tracks when the file is sparse, physical is the + * last offset of the file. */ disk->src->physical = sb.st_size; -#endif - /* Regular files may be sparse, so logical size (capacity) is not same - * as actual physical above - */ - disk->src->capacity = sb.st_size; } else { - /* NB. Because we configure with AC_SYS_LARGEFILE, off_t should - * be 64 bits on all platforms. + /* NB. Because we configure with AC_SYS_LARGEFILE, off_t + * should be 64 bits on all platforms. For block devices, we + * have to seek (safe even if someone else is writing) to + * determine physical size, and assume that allocation is the + * same as physical (but can refine that assumption later if + * qemu is still running). */ end = lseek(fd, 0, SEEK_END); if (end == (off_t)-1) { @@ -11128,11 +11131,12 @@ qemuDomainGetBlockInfo(virDomainPtr dom, goto endjob; } disk->src->physical = end; - disk->src->capacity = end; + disk->src->allocation = end; } - /* If the file we probed has a capacity set, then override - * what we calculated from file/block extents */ + /* Raw files: capacity is physical size. For all other files: if + * the metadata has a capacity, use that, otherwise fall back to + * physical size. */ if (!(format = virDomainDiskGetFormat(disk))) { if (!cfg->allowDiskFormatProbing) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -11145,16 +11149,16 @@ qemuDomainGetBlockInfo(virDomainPtr dom, buf, len)) < 0) goto endjob; } - if (!(meta = virStorageFileGetMetadataFromBuf(disk->src->path, buf, len, - format, NULL))) + if (format == VIR_STORAGE_FILE_RAW) { + disk->src->capacity = disk->src->physical; + } else if (!(meta = virStorageFileGetMetadataFromBuf(disk->src->path, buf, + len, format, NULL))) { goto endjob; - if (meta->capacity) - disk->src->capacity = meta->capacity; + disk->src->capacity = meta->capacity ? meta->capacity + : disk->src->physical;

+ } - /* Set default value .. */ - disk->src->allocation = disk->src->physical; - - /* ..but if guest is not using raw disk format and on a block device, + /* If guest is not using raw disk format and on a block device, * then query highest allocated extent from QEMU */ if (virStorageSourceGetActualType(disk->src) == VIR_STORAGE_TYPE_BLOCK &&