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