[libvirt] [PATCH] Block info query: Add flag to allow failure if not active

[Daniel P. Berrange]

On Wed, Dec 18, 2013 at 06:58:10AM -0500, John Ferlan wrote:
> Currently the qemuDomainGetBlockInfo will return allocation == physical
> for most backing stores. For a qcow2 block backed device it's possible
> to return the highest lv extent allocated from qemu for an active guest.
> That is a value where allocation != physical and one would hope be less.
> However, if the guest is not running, then the code falls back to returning
> allocation == physical. This turns out to be problematic for rhev which
> monitors the size of the backing store. During a migration, before the
> VM has been started on the target and while it is deemed inactive on the
> source, allocation is returned as physical triggering the code to extend
> the file unnecessarily.
> 
> Thus, this patch will check a new flag to the call to determine whether
> a failure should be returned to indicate to the caller that the code is
> unable to fetch the allocation value. This will allow the caller to decide
> how to proceed.
> 
> Note that I chose not to return a failure as a more general rule mostly
> because of backwards compatability and current expectations reasons.
> Changing a default return action could be an unexpected action. Although
> returning allocation == physical perhaps is incorrect, it is how the
> code has been functioning. Having a 'virsh domblkinfo' return a failure
> for "some" inactive domains while succeeding for others could be confusing.
> A flag forces the caller to decide how to operate.
> 
> As it turns out, there seems to be quite a bit of history regarding this.
> Details can be found in the BZ:
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1040507
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
> NOTE:
> Trying to keep this as simple as possible since it needs to be backported.
> If someone has a better idea for a flag name - I'm open to suggestions...
> 
>  include/libvirt/libvirt.h.in | 23 ++++++++++++++++++++---
>  src/libvirt.c                | 11 ++++++++++-
>  src/qemu/qemu_driver.c       | 33 +++++++++++++++++++++++++++------
>  3 files changed, 57 insertions(+), 10 deletions(-)
> 
> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> index 6f79c49..420642b 100644
> --- a/include/libvirt/libvirt.h.in
> +++ b/include/libvirt/libvirt.h.in
> @@ -2095,7 +2095,8 @@ int                     virDomainBlockResize (virDomainPtr dom,
>  
>  /** virDomainBlockInfo:
>   *
> - * This struct provides information about the size of a block device backing store
> + * This struct provides information about the size of a block device
> + * backing store
>   *
>   * Examples:
>   *
> @@ -2108,13 +2109,29 @@ int                     virDomainBlockResize (virDomainPtr dom,
>   *
>   *  - qcow2 file in filesystem
>   *       * capacity: logical size from qcow2 header
> - *       * allocation, physical: logical size of the file / highest qcow extent (identical)
> + *       * allocation, physical: logical size of the file /
> + *                               highest qcow extent (identical)
>   *
>   *  - qcow2 file in a block device
>   *       * capacity: logical size from qcow2 header
> - *       * allocation: highest qcow extent written
> + *       * allocation: highest qcow extent written for an active domain
>   *       * physical: size of the block device container
>   */
> +
> +/**
> + * virDomainGetBlockInfoFlags:
> + *
> + * Flags available for virDomainGetBlockInfo().
> + */
> +typedef enum {
> +    VIR_DOMAIN_BLOCK_CHECK_ACTIVE = 1 << 0, /* If not active, then return
> +                                             * failure when attempting to
> +                                             * fetch data from an inactive
> +                                             * domain, such as allocation
> +                                             * from a qcow2 block device
> +                                             */
> +} virDomainGetBlockInfoFlags;

I'm not convinced this flag is desirable. Can't apps just
check whether the guest is running themselves, or indeed
something like RHEV surely knows what its own VM is doing
already.

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|