[PATCH v2 2/2] Introduce and use virDomainDiskEmptySource

[Michal Privoznik <mprivozn@xxxxxxxxxx>]

Currently, if we want to zero out disk source (e,g, due to
startupPolicy when starting up a domain) we use
virDomainDiskSetSource(disk, NULL). This works well for file
based storage (storage type file, dir, or block). But it doesn't
work at all for other types like volume and network.

So imagine that you have a domain that has a CDROM configured
which source is a volume from an inactive pool. Because it is
startupPolicy='optional', the CDROM is empty when the domain
starts. However, the source element is not cleared out in the
status XML and thus when the daemon restarts and tries to
reconnect to the domain it refreshes the disks (which fails - the
storage pool is still not running) and thus the domain is killed.

Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
---

diff to v1:
- Free more struct entries
- Set disk type
- Call the function less frequently

 src/conf/domain_conf.c   | 33 +++++++++++++++++++++++++++++++++
 src/conf/domain_conf.h   |  1 +
 src/libvirt_private.syms |  1 +
 src/qemu/qemu_domain.c   |  2 +-
 src/qemu/qemu_process.c  |  2 +-
 5 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 01553b5..f1f0b56 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1719,6 +1719,39 @@ virDomainDiskSetSource(virDomainDiskDefPtr def, const char *src)
 }
 
 
+void
+virDomainDiskEmptySource(virDomainDiskDefPtr def)
+{
+    virStorageSourcePtr src = def->src;
+
+    switch ((virStorageType) src->type) {
+    case VIR_STORAGE_TYPE_DIR:
+    case VIR_STORAGE_TYPE_FILE:
+    case VIR_STORAGE_TYPE_BLOCK:
+        VIR_FREE(src->path);
+        break;
+    case VIR_STORAGE_TYPE_NETWORK:
+        VIR_FREE(src->path);
+        VIR_FREE(src->volume);
+        virStorageNetHostDefFree(src->nhosts, src->hosts);
+        src->nhosts = 0;
+        src->hosts = NULL;
+        src->protocol = VIR_STORAGE_NET_PROTOCOL_NONE;
+        break;
+    case VIR_STORAGE_TYPE_VOLUME:
+        virStorageSourcePoolDefFree(src->srcpool);
+        src->srcpool = NULL;
+        break;
+    case VIR_STORAGE_TYPE_NONE:
+    case VIR_STORAGE_TYPE_LAST:
+        break;
+    }
+
+    virStorageSourceBackingStoreClear(src);
+    src->type = VIR_STORAGE_TYPE_FILE;
+}
+
+
 const char *
 virDomainDiskGetDriver(virDomainDiskDefPtr def)
 {
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 47eaace..26c0e6b 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2590,6 +2590,7 @@ void virDomainDiskSetType(virDomainDiskDefPtr def, int type);
 const char *virDomainDiskGetSource(virDomainDiskDef const *def);
 int virDomainDiskSetSource(virDomainDiskDefPtr def, const char *src)
     ATTRIBUTE_RETURN_CHECK;
+void virDomainDiskEmptySource(virDomainDiskDefPtr def);
 const char *virDomainDiskGetDriver(virDomainDiskDefPtr def);
 int virDomainDiskSetDriver(virDomainDiskDefPtr def, const char *name)
     ATTRIBUTE_RETURN_CHECK;
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index b551cb8..92083e5 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -299,6 +299,7 @@ virDomainDiskDetectZeroesTypeFromString;
 virDomainDiskDetectZeroesTypeToString;
 virDomainDiskDeviceTypeToString;
 virDomainDiskDiscardTypeToString;
+virDomainDiskEmptySource;
 virDomainDiskErrorPolicyTypeFromString;
 virDomainDiskErrorPolicyTypeToString;
 virDomainDiskFindByBusAndDst;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 589eb18..b733505 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4908,7 +4908,7 @@ qemuDomainCheckRemoveOptionalDisk(virQEMUDriverPtr driver,
         event = virDomainEventDiskChangeNewFromObj(vm, src, NULL,
                                                    disk->info.alias,
                                                    VIR_DOMAIN_EVENT_DISK_CHANGE_MISSING_ON_START);
-        ignore_value(virDomainDiskSetSource(disk, NULL));
+        virDomainDiskEmptySource(disk);
         /* keeping the old startup policy would be invalid for new images */
         disk->startupPolicy = VIR_DOMAIN_STARTUP_POLICY_DEFAULT;
     } else {
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index a20beb1..e450d06 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -6804,7 +6804,7 @@ qemuProcessRefreshDisks(virQEMUDriverPtr driver,
 
         if (info->removable) {
             if (info->empty)
-                ignore_value(virDomainDiskSetSource(disk, NULL));
+                virDomainDiskEmptySource(disk);
 
             if (info->tray) {
                 if (info->tray_open)
-- 
2.10.2

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list