From 9eee40cc540e25bb0783ef393b7bf7a74444b55c Mon Sep 17 00:00:00 2001
From: Michal Privoznik <mprivozn@redhat.com>
Date: Mon, 27 Aug 2012 17:40:33 +0200
Subject: [PATCH] qemu: Fix define logic

With current flow in qemudDomainDefine we might lose data
when updating an existing domain. We parse given XML and
overwrite the configuration.  Then we try to save the new
config. However, this step may fail and we don't perform any
roll back.  In fact, we remove the domain from the list of
domains held up by qemu driver. This is okay as long as the
domain was brand new one.
---
 src/qemu/qemu_driver.c | 43 +++++++++++++++++++++++++++++++++++-------
 1 file changed, 36 insertions(+), 7 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index f64d9ec36e..d74bf52449 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5540,6 +5540,7 @@ out:
 static virDomainPtr qemudDomainDefine(virConnectPtr conn, const char *xml) {
     struct qemud_driver *driver = conn->privateData;
     virDomainDefPtr def;
+    virDomainDefPtr def_backup = NULL;
     virDomainObjPtr vm = NULL;
     virDomainPtr dom = NULL;
     virDomainEventPtr event = NULL;
@@ -5563,20 +5564,48 @@ static virDomainPtr qemudDomainDefine(virConnectPtr conn, const char *xml) {
     if (qemuDomainAssignAddresses(def, NULL, NULL) < 0)
         goto cleanup;
 
-    if (!(vm = virDomainAssignDef(driver->caps,
-                                  &driver->domains,
-                                  def, false))) {
-        goto cleanup;
+    /* We need to differentiate two cases:
+     * a) updating an existing domain - must preserve previous definition
+     *                                  so we can roll back if something fails
+     * b) defining a brand new domain - virDomainAssignDef is just sufficient
+     */
+    if ((vm = virDomainFindByUUID(&driver->domains, def->uuid))) {
+        if (virDomainObjIsActive(vm)) {
+            def_backup = vm->newDef;
+            vm->newDef = def;
+        } else {
+            def_backup = vm->def;
+            vm->def = def;
+        }
+    } else {
+        if (!(vm = virDomainAssignDef(driver->caps,
+                                      &driver->domains,
+                                      def, false))) {
+            goto cleanup;
+        }
     }
     def = NULL;
     vm->persistent = 1;
 
     if (virDomainSaveConfig(driver->configDir,
                             vm->newDef ? vm->newDef : vm->def) < 0) {
-        VIR_INFO("Defining domain '%s'", vm->def->name);
-        qemuDomainRemoveInactive(driver, vm);
-        vm = NULL;
+        if (def_backup) {
+            /* There is backup so this VM was defined before.
+             * Just restore the backup. */
+            VIR_INFO("Restoring domain '%s' definition", vm->def->name);
+            if (virDomainObjIsActive(vm))
+                vm->newDef = def_backup;
+            else
+                vm->def = def_backup;
+        } else {
+            /* Brand new domain. Remove it */
+            VIR_INFO("Deleting domain '%s'", vm->def->name);
+            qemuDomainRemoveInactive(driver, vm);
+            vm = NULL;
+        }
         goto cleanup;
+    } else {
+        virDomainDefFree(def_backup);
     }
 
     event = virDomainEventNewFromObj(vm,