On 12/16/2014 05:41 AM, Peter Krempa wrote:
> On 12/16/14 09:04, 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:
>>
>>
>> While this patch does not optimize reuse of information in point
>> 1, it does get us closer to being able to do so; by updating a
>> structure that survives between consecutive calls.
>>
>>
>> + /* FIXME: For an offline domain, we always want to check current
>> + * on-disk statistics (as users have been known to change offline
>> + * images behind our backs). For a running domain, however, it
>> + * would be nice to avoid opening a file (particularly since
>> + * reading a file while qemu is writing it risks the reader seeing
>> + * bogus data), or even avoid a stat, if the information
>> + * remembered from the previous run is still viable.
>> + *
>> + * 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.
>> + */
>
> I still don't see this comment vanishing in this series. Are you planing
> on getting rid of the code that opens the file on disk while the VM is
> alive?
Correct, that's where my other email (subject "virDomainGetBlockInfo
meanings") would come into play - I'm still trying to work on additional
patches to reuse code correctly and minimize file probes where possible,
but those can be followup patches. So I will go ahead and push the
patches that have been ACKed so far.
>
>> if (virStorageSourceIsLocalStorage(disk->src)) {
>> if (!disk->src->path) {
>> virReportError(VIR_ERR_INVALID_ARG,
>
> ACK
Thanks for the reviews :)
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list