From 7968d4cc1504076a3b28601ef896909961d06444 Mon Sep 17 00:00:00 2001

Message-Id: <7968d4cc1504076a3b28601ef896909961d06444@dist-git>

From: John Ferlan <jferlan@redhat.com>

Date: Fri, 14 Feb 2014 16:33:32 -0500

Subject: [PATCH] Block info query: Add check for transient domain

https://bugzilla.redhat.com/show_bug.cgi?id=1065531

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, there's a small window of time where the allocation is returned

as physical triggering the code to extend the file unnecessarily.

Since rhev uses transient domains and this is edge condition for a transient

domain, rather than returning good status and allocation == physical when

this "window of opportunity" exists, this patch will check for a transient

(or non persistent) domain and return a failure to the caller rather than

returning the defaults. For a persistent domain, the defaults will be

returned. The description for the virDomainGetBlockInfo has been updated

to describe the phenomena.

(cherry picked from commit 46a0737e13cec06a6b7cb0afdcdd200c2bec317f)

Signed-off-by: John Ferlan <jferlan@redhat.com>

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>

---

 include/libvirt/libvirt.h.in |  8 ++++---

 src/libvirt.c                | 53 ++++++++++++++++++++++++++++++++++++++++++++

 src/qemu/qemu_driver.c       | 29 +++++++++++++++++++-----

 3 files changed, 82 insertions(+), 8 deletions(-)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in

index 1bc6624..91d89ec 100644

--- a/include/libvirt/libvirt.h.in

+++ b/include/libvirt/libvirt.h.in

@@ -2079,7 +2079,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:

  *

@@ -2092,11 +2093,12 @@ 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

  */

 typedef struct _virDomainBlockInfo virDomainBlockInfo;

diff --git a/src/libvirt.c b/src/libvirt.c

index 61cae03..af94326 100644

--- a/src/libvirt.c

+++ b/src/libvirt.c

@@ -9028,6 +9028,59 @@ error:

  * can be found by calling virDomainGetXMLDesc() and inspecting

  * elements within //domain/devices/disk.

  *

+ * For QEMU domains, the allocation and physical virDomainBlockInfo

+ * values returned will generally be the same, except when using a

+ * non raw, block backing device, such as qcow2 for an active domain.

+ * When the persistent domain is not active, QEMU will return the

+ * default which is the same value for allocation and physical.

+ *

+ * Active QEMU domains can return an allocation value which is more

+ * representative of the currently used blocks by the device compared

+ * to the physical size of the device. Applications can use/monitor

+ * the allocation value with the understanding that if the domain

+ * becomes inactive during an attempt to get the value, the default

+ * values will be returned. Thus, the application should check

+ * after the call for the domain being inactive if the values are

+ * the same. Optionally, the application could be watching for a

+ * shutdown event and then ignore any values received afterwards.

+ * This can be an issue when a domain is being migrated and the

+ * exact timing of the domain being made inactive and check of

+ * the allocation value results the default being returned. For

+ * a transient domain in the similar situation, this call will return

+ * -1 and an error message indicating the "domain is not running".

+ *

+ * The following is some pseudo code illustrating the call sequence:

+ *

+ *   ...

+ *   virDomainPtr dom;

+ *   virDomainBlockInfo info;

+ *   char *device;

+ *   ...

+ *   // Either get a list of all domains or a specific domain

+ *   // via a virDomainLookupBy*() call.

+ *   //

+ *   // It's also required to fill in the device pointer, but that's

+ *   // specific to the implementation. For the purposes of this example

+ *   // a qcow2 backed device name string would need to be provided.

+ *   ...

+ *   // If the following call is made on a persistent domain with a

+ *   // qcow2 block backed block device, then it's possible the returned

+ *   // allocation equals the physical value. In that case, the domain

+ *   // that may have been active prior to calling has become inactive,

+ *   // such as is the case during a domain migration. Thus once we

+ *   // get data returned, check for active domain when the values are

+ *   // the same.

+ *   if (virDomainGetBlockInfo(dom, device, &info, 0) < 0)

+ *       goto failure;

+ *   if (info.allocation == info.physical) {

+ *       // If the domain is no longer active,

+ *       // then the defaults are being returned.

+ *       if (!virDomainIsActive())

+ *               goto ignore_return;

+ *   }

+ *   // Do something with the allocation and physical values

+ *   ...

+ *

  * Returns 0 in case of success and -1 in case of failure.

  */

 int

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c

index ee622a2..a7da257 100644

--- a/src/qemu/qemu_driver.c

+++ b/src/qemu/qemu_driver.c

@@ -10084,6 +10084,7 @@ qemuDomainGetBlockInfo(virDomainPtr dom,

     struct stat sb;

     int idx;

     int format;

+    int activeFail = false;

     virQEMUDriverConfigPtr cfg = NULL;

     char *alias = NULL;

@@ -10183,15 +10184,23 @@ qemuDomainGetBlockInfo(virDomainPtr dom,

     /* Set default value .. */

     info->allocation = info->physical;

-    /* ..but if guest is running & not using raw

-       disk format and on a block device, then query

-       highest allocated extent from QEMU */

+    /* ..but if guest is not using raw disk format and on a block device,

+     * then query highest allocated extent from QEMU

+     */

     if (disk->type == VIR_DOMAIN_DISK_TYPE_BLOCK &&

         format != VIR_STORAGE_FILE_RAW &&

-        S_ISBLK(sb.st_mode) &&

-        virDomainObjIsActive(vm)) {

+        S_ISBLK(sb.st_mode)) {

         qemuDomainObjPrivatePtr priv = vm->privateData;

+        /* If the guest is not running, then success/failure return

+         * depends on whether domain is persistent

+         */

+        if (!virDomainObjIsActive(vm)) {

+            activeFail = true;

+            ret = 0;

+            goto cleanup;

+        }

+

         if (VIR_STRDUP(alias, disk->info.alias) < 0)

             goto cleanup;

@@ -10205,6 +10214,7 @@ qemuDomainGetBlockInfo(virDomainPtr dom,

                                             &info->allocation);

             qemuDomainObjExitMonitor(driver, vm);

         } else {

+            activeFail = true;

             ret = 0;

         }

@@ -10218,6 +10228,15 @@ cleanup:

     VIR_FREE(alias);

     virStorageFileFreeMetadata(meta);

     VIR_FORCE_CLOSE(fd);

+

+    /* If we failed to get data from a domain because it's inactive and

+     * it's not a persistent domain, then force failure.

+     */

+    if (activeFail && vm && !vm->persistent) {

+        virReportError(VIR_ERR_OPERATION_INVALID, "%s",

+                       _("domain is not running"));

+        ret = -1;

+    }

     if (vm)

         virObjectUnlock(vm);

     virObjectUnref(cfg);

-- 

1.9.0