Re: [libvirt] [PATCH 04/12] qemu: let blockinfo reuse virStorageSource

On 12/06/14 09:14, Eric Blake wrote: > Right now, grabbing blockinfo always calls stat on the disk, then > opens the image to determine the capacity, using a throw-away > virStorageSourcePtr. This has a couple of drawbacks: >

> + * For read-only disks, nothing should be changing unless the user > + * has requested a block-commit action. For read-write disks, we > + * know some special cases: capacity should not change without a > + * block-resize (where capacity is the only stat that requires > + * opening a file, and even then, only for non-raw files); and > + * physical size of a raw image or of a block device should > + * likewise not be changing without block-resize. On the other > + * hand, allocation of a raw file can change (if the file is > + * sparse, but the amount of sparseness changes due to writes or > + * punching holes), and physical size of a non-raw file can > + * change. For a live VM we should grab all of the above directly from the monitor and not ever touch the files on the disk. We do that already for the bulk stats and for getting the right size when doing storage migration.

This function unfortunately is legacy code compared to the stuff I've pointed out

> +++ b/src/util/virstoragefile.h > @@ -257,8 +257,9 @@ struct _virStorageSource { > > virStoragePermsPtr perms; > virStorageTimestampsPtr timestamps; > - unsigned long long allocation; /* in bytes, 0 if unknown */ Spurious move? > unsigned long long capacity; /* in bytes, 0 if unknown */ > + unsigned long long allocation; /* in bytes, 0 if unknown */ > + unsigned long long physical; /* in bytes, 0 if unknown */

> size_t nseclabels; > virSecurityDeviceLabelDefPtr *seclabels; > Also an addition to virStorageSourceCopy is missing. ACK with the tweak to virStorageSourceCopy