File libvirt-snapshot-qemu-Fix-segfault-and-vanishing-snapshots-when-redefining.patch of Package libvirt

From 1299be7cdf9d96be26f3e377a2777445aad3c00f Mon Sep 17 00:00:00 2001
Message-Id: <1299be7cdf9d96be26f3e377a2777445aad3c00f.1357740563.git.jdenemar@redhat.com>
From: Peter Krempa <pkrempa@redhat.com>
Date: Sat, 5 Jan 2013 08:59:29 +0100
Subject: [PATCH] snapshot: qemu: Fix segfault and vanishing snapshots when
 redefining

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

When the disk alignment check done while redefining an existing snapshot
failed, the qemu driver attempted to free the existing snapshot. As in
the cleanup path the definition of the snapshot wasn't assigned, the
cleanup code dereferenced a NULL pointer.

This patch changes the behavior on error paths while redefining snapshot
in two ways:

1) On failure, modifications done on the snapshot definition object are
rolled back.

2) The previous definition of the data isn't freed until it's certain it
won't be needed any more.

This change avoids the segfault and additionally the snapshot doesn't
vanish if redefinition fails for some reason.
(cherry picked from commit 709b0f37c5cb22b8846e59c2259ea30e73d72d92)
---
 src/qemu/qemu_driver.c | 51 +++++++++++++++++++++++++++++++++++---------------
 1 file changed, 36 insertions(+), 15 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 22c437f..8be9586 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11610,6 +11610,24 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
                 }
             }
 
+            if (def->dom) {
+                if (def->state == VIR_DOMAIN_DISK_SNAPSHOT ||
+                    def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) {
+                    align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL;
+                    align_match = false;
+                }
+
+                if (virDomainSnapshotAlignDisks(def, align_location,
+                                                align_match) < 0) {
+                    /* revert stealing of the snapshot domain definition */
+                    if (def->dom && !other->def->dom) {
+                        other->def->dom = def->dom;
+                        def->dom = NULL;
+                    }
+                    goto cleanup;
+                }
+            }
+
             if (other == vm->current_snapshot) {
                 update_current = true;
                 vm->current_snapshot = NULL;
@@ -11619,18 +11637,20 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
              * child relations by reusing snap.  */
             virDomainSnapshotDropParent(other);
             virDomainSnapshotDefFree(other->def);
-            other->def = NULL;
+            other->def = def;
+            def = NULL;
             snap = other;
-        }
-        if (def->dom) {
-            if (def->state == VIR_DOMAIN_DISK_SNAPSHOT ||
-                def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) {
-                align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL;
-                align_match = false;
+        } else {
+            if (def->dom) {
+                if (def->state == VIR_DOMAIN_DISK_SNAPSHOT ||
+                    def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) {
+                    align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL;
+                    align_match = false;
+                }
+                if (virDomainSnapshotAlignDisks(def, align_location,
+                                                align_match) < 0)
+                    goto cleanup;
             }
-            if (virDomainSnapshotAlignDisks(def, align_location,
-                                            align_match) < 0)
-                goto cleanup;
         }
     } else {
         /* Easiest way to clone inactive portion of vm->def is via
@@ -11665,11 +11685,12 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
             goto cleanup;
     }
 
-    if (snap)
-        snap->def = def;
-    else if (!(snap = virDomainSnapshotAssignDef(vm->snapshots, def)))
-        goto cleanup;
-    def = NULL;
+    if (!snap) {
+        if (!(snap = virDomainSnapshotAssignDef(vm->snapshots, def)))
+            goto cleanup;
+
+        def = NULL;
+    }
 
     if (update_current)
         snap->def->current = true;
-- 
1.8.1