On 13.12.2016 21:58, John Ferlan wrote: > According to commit id '0282ca45a' the 'physical' value should > essentially be the last offset of the image or the host physical > size in bytes of the image container. However, commit id '15fa84ac' > refactored the GetBlockInfo to use the same returned data as the > GetStatsBlock API for an active domain. For the 'entry->physical' > that would end up being the "actual-size" as set through the > qemuMonitorJSONBlockStatsUpdateCapacityOne (commit '7b11f5e5'). > Digging deeper into QEMU code one finds that actual_size is > filled in using the same algorithm as GetBlockInfo has used for > setting the 'allocation' field when the domain is inactive. > > The difference in values is seen primarily in sparse raw files > and other container type files (such as qcow2), which will return > a smaller value via the stat API for 'st_blocks'. Additionally > for container files, the 'capacity' field (populated via the > QEMU "virtual-size" value) may be slightly different (smaller) > in order to accomodate the overhead for the container. For > sparse files, the state 'st_size' field is returned. > > This patch thus alters the allocation and physical values for > sparse backed storage files to be more appropriate to the API > contract. The result for GetBlockInfo is the following: > > capacity: logical size in bytes of the image (how much storage > the guest will see) > allocation: host storage in bytes occupied by the image (such > as highest allocated extent if there are no holes, > similar to 'du') > physical: host physical size in bytes of the image container > (last offset, similar to 'ls') > > NB: The GetStatsBlock API allows a different contract for the > values: > > "block.<num>.allocation" - offset of the highest written sector > as unsigned long long. > "block.<num>.capacity" - logical size in bytes of the block device > backing image as unsigned long long. > "block.<num>.physical" - physical size in bytes of the container > of the backing image as unsigned long long. > > Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx> > --- > > Initially based on a question posed on libvirt-users: > > https://www.redhat.com/archives/libvirt-users/2016-November/msg00050.html > > related to determining whether the provided physical value for an > inactive guest would be valid I found that virDomainGetBlockInfo was > returning different results for capacity and physical when the domain > was running. As I dug deeper I found Eric Blake's commit regarding how > libvirt uses/describes the allocation, capacity, and physical and felt > that the changes made to virDomainGetBlockInfo especially for sparse files > and "container" files (such as qcow2) would essentially provide incorrect > and perhaps unexpected results. > > If it's felt that the live/active domain should return/display the same > as the virConnectGetAllDomainStats, then I can drop the patch. I guess I > find it odd that the physical size could be 4K matching the allocation > value for a sparse file. Likewise, for a fully preallocated file was > showing an allocation value of 0 since wr_highest_offset hadn't yet > been updated. > > I've been testing with a guest with different types of "file" and "block" > storage backings where the 'block' storage is essentially iSCSI devices > added to the guest. Logically the changes result in what I'd expect for > results based upon reading the API docs. > > src/qemu/qemu_driver.c | 22 +++++++++++++++++++--- > 1 file changed, 19 insertions(+), 3 deletions(-) > > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > index 88778d4..c537891 100644 > --- a/src/qemu/qemu_driver.c > +++ b/src/qemu/qemu_driver.c > @@ -11808,13 +11808,29 @@ qemuDomainGetBlockInfo(virDomainPtr dom, > info->allocation = entry->wr_highest_offset; > } > > - if (entry->physical) { > - info->physical = entry->physical; > - } else { > + /* Unlike GetStatsBlock, this API has defined the expected return values > + * for allocation and physical slightly differently. > + * > + * Having a zero for either or if they're the same is an indication that > + * there's a sparse file backing this device. In this case, we'll force > + * the setting of physical based on the on disk file size. > + * > + * Additionally, if qemu hasn't written to the file yet, then set the > + * allocation to whatever qemu returned for physical (e.g. the "actual- > + * size" from the json query) as that will match the expected allocation > + * value for this API. */ > + if (entry->physical == 0 || info->allocation == 0 || > + info->allocation == entry->physical) { > + info->allocation = entry->physical; > + if (info->allocation == 0) > + info->allocation = entry->physical; > + > if (qemuDomainStorageUpdatePhysical(driver, cfg, vm, disk->src) < 0) > goto endjob; > > info->physical = disk->src->physical; > + } else { > + info->physical = entry->physical; > } > > info->capacity = entry->capacity; > ACK Michal -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
