When pivoting after a completed block job, only save the domain's
persistent configuration if the domain is actually marked persistent.

This commit also refactors the logic surrounding the copying of the new
disk definition into vm->newDef to avoid a NULL pointer dereference if
virStorageSourceCopy were to fail to allocate memory.

Signed-off-by: Michael Chapman <m...@very.puzzling.org>
---
 src/qemu/qemu_blockjob.c | 34 ++++++++++++++--------------------
 1 file changed, 14 insertions(+), 20 deletions(-)

diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index 1d5b7ce..ae936a2 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -116,26 +116,20 @@ qemuBlockJobEventProcess(virQEMUDriverPtr driver,
     switch ((virConnectDomainEventBlockJobStatus) status) {
     case VIR_DOMAIN_BLOCK_JOB_COMPLETED:
         if (disk->mirrorState == VIR_DOMAIN_DISK_MIRROR_STATE_PIVOT) {
-            if (vm->newDef) {
-                virStorageSourcePtr copy = NULL;
-
-                if ((persistDisk = virDomainDiskByName(vm->newDef,
-                                                       disk->dst, false))) {
-                    copy = virStorageSourceCopy(disk->mirror, false);
-                    if (virStorageSourceInitChainElement(copy,
-                                                         persistDisk->src,
-                                                         true) < 0) {
-                        VIR_WARN("Unable to update persistent definition "
-                                 "on vm %s after block job",
-                                 vm->def->name);
-                        virStorageSourceFree(copy);
-                        copy = NULL;
-                        persistDisk = NULL;
-                    }
-                }
-                if (copy) {
+            if (vm->newDef &&
+                (persistDisk = virDomainDiskByName(vm->newDef, disk->dst, 
false))) {
+                virStorageSourcePtr copy = virStorageSourceCopy(disk->mirror, 
false);
+                if (copy && virStorageSourceInitChainElement(copy,
+                                                             persistDisk->src,
+                                                             true) == 0) {
                     virStorageSourceFree(persistDisk->src);
                     persistDisk->src = copy;
+                } else {
+                    VIR_WARN("Unable to update persistent definition "
+                             "on vm %s after block job",
+                             vm->def->name);
+                    virStorageSourceFree(copy);
+                    persistDisk = NULL;
                 }
             }
 
@@ -188,8 +182,8 @@ qemuBlockJobEventProcess(virQEMUDriverPtr driver,
         if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
             VIR_WARN("Unable to save status on vm %s after block job",
                      vm->def->name);
-        if (persistDisk && virDomainSaveConfig(cfg->configDir,
-                                               vm->newDef) < 0)
+        if (vm->persistent && persistDisk &&
+            virDomainSaveConfig(cfg->configDir, vm->newDef) < 0)
             VIR_WARN("Unable to update persistent definition on vm %s "
                      "after block job", vm->def->name);
     }
-- 
2.4.3

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

Reply via email to