[libvirt] [PATCH v2 02/12] qemu: let blockinfo reuse virStorageSource

[Peter Krempa]

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:
> 
> 1. We are calling stat and opening a file on every invocation of
> the API.  However, there are cases where the stats should NOT be
> changing between successive calls (if a domain is running, no
> one should be changing the physical size of a block device or raw
> image behind our backs; capacity of read-only files should not
> be changing; and we are the gateway to the block-resize command
> to know when the capacity of read-write files should be changing).
> True, we still have to use stat in some cases (a sparse raw file
> changes allocation if it is read-write and the amount of holes is
> changing, and a read-write qcow2 image stored in a file changes
> physical size if it was not fully pre-allocated).  But for
> read-only images, even this should be something we can remember
> from the previous time, rather than repeating every call.
> 
> 2. We want to enhance the power of virDomainListGetStats, by
> sharing code.  But we already have a virStorageSourcePtr for
> each disk, and it would be easier to reuse the common structure
> than to have to worry about the one-off virDomainBlockInfoPtr.
> 
> 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.
> 
> * src/util/virstoragefile.h (_virStorageSource): Add physical, to
> mirror virDomainBlockInfo; rearrange fields to match public struct.
> (virStorageSourceCopy): Copy the new field.
> * src/qemu/qemu_driver.c (qemuDomainGetBlockInfo): Store into
> storage source, then copy to block info.
> 
> Signed-off-by: Eric Blake <eblake at redhat.com>
> ---
>  src/qemu/qemu_driver.c    | 42 ++++++++++++++++++++++++++++++++++--------
>  src/util/virstoragefile.c |  3 ++-
>  src/util/virstoragefile.h |  3 ++-
>  3 files changed, 38 insertions(+), 10 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 8775ab2..b13c5e1 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -11045,6 +11045,26 @@ qemuDomainGetBlockInfo(virDomainPtr dom,
> 
>      disk = vm->def->disks[idx];
> 
> +    /* 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?

>      if (virStorageSourceIsLocalStorage(disk->src)) {
>          if (!disk->src->path) {
>              virReportError(VIR_ERR_INVALID_ARG,

ACK

Peter


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20141216/a86bbc75/attachment-0001.sig>