Re: [PATCHv8 2/4] libvirt/qemu - synchronous update of device definition

[KAMEZAWA Hiroyuki <kamezawa.hiroyu@xxxxxxxxxxxxxx>]

On Fri, 01 Apr 2011 14:33:23 +0800
Wen Congyang <wency@xxxxxxxxxxxxxx> wrote:

> I found that virDomainObjAssignDef() can work. We can call it like this:
> virDomainObjAssignDef(vm, copy, false).
> 
> We modify vm->newdef if domain is active, and modify vm->def if domain is inactive.
> 

Okay, here is a replacement. it seems to work fine. Patch 3 and 4 can be applied
without hunk.

==
>From b743fc6a0a449efa738638e165b139afab91439f Mon Sep 17 00:00:00 2001
From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@xxxxxxxxxxxxxx>
Date: Fri, 1 Apr 2011 16:02:31 +0900
Subject: [PATCH 2/2]  libvirt/qemu - rollback for persistent modification.

At persistent modification of inactive domains, we go several steps as
  - insert disk to vmdef
  - assign controller if necessary
  - assign pci address if necessary
  - save it to file

If failure happens in above sequence, that implies there is inconsistency
between XML definition in file and vmdef cached in memory. So, it's better
to have some rollback method. Implementing undo in each stage doesn't seem
very neat, this patch implements a generic one.

This patch adds virDomainObjCopyPersistentDef(). This will create a
copy of persistent def. The caller update this and later replace
current one as:

    copy = virDomainObjCopyPersistentDef()
    .....update....
   if (error)
       virDomainObjAssignDef(dom, copy);
   else
       virDomainDefFree(copy).

Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@xxxxxxxxxxxxxx>
---
 src/conf/domain_conf.c   |   18 ++++++++++++++++++
 src/conf/domain_conf.h   |    3 +++
 src/libvirt_private.syms |    1 +
 src/qemu/qemu_driver.c   |    8 +++++++-
 4 files changed, 29 insertions(+), 1 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b6aaf33..31641f1 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -9368,3 +9368,21 @@ cleanup:
 
     return ret;
 }
+
+virDomainDefPtr
+virDomainObjCopyPersistentDef(virCapsPtr caps, virDomainObjPtr dom)
+{
+    char *xml;
+    virDomainDefPtr cur, ret;
+
+    cur = virDomainObjGetPersistentDef(caps, dom);
+
+    xml = virDomainDefFormat(cur, VIR_DOMAIN_XML_WRITE_FLAGS);
+    if (!xml)
+        return NULL;
+
+    ret = virDomainDefParseString(caps, xml, VIR_DOMAIN_XML_READ_FLAGS);
+
+    VIR_FREE(xml);
+    return ret;
+}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 10e73cb..20bb4ed 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1377,6 +1377,9 @@ int virDomainDiskDefForeachPath(virDomainDiskDefPtr disk,
                                 virDomainDiskDefPathIterator iter,
                                 void *opaque);
 
+virDomainDefPtr
+virDomainObjCopyPersistentDef(virCapsPtr caps, virDomainObjPtr dom);
+
 typedef const char* (*virLifecycleToStringFunc)(int type);
 typedef int (*virLifecycleFromStringFunc)(const char *type);
 
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 65a86d3..ec69967 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -278,6 +278,7 @@ virDomainMemballoonModelTypeToString;
 virDomainNetDefFree;
 virDomainNetTypeToString;
 virDomainObjAssignDef;
+virDomainObjCopyPersistentDef;
 virDomainObjSetDefTransient;
 virDomainObjGetPersistentDef;
 virDomainObjIsDuplicate;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b89bc8f..96c4df2 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3952,7 +3952,7 @@ static int qemuDomainModifyDevicePersistent(virDomainPtr dom,
         goto endjob;
     }
 
-    vmdef = virDomainObjGetPersistentDef(driver->caps, vm);
+    vmdef = virDomainObjCopyPersistentDef(driver->caps, vm);
 
     if (!vmdef)
         goto endjob;
@@ -3973,6 +3973,12 @@ static int qemuDomainModifyDevicePersistent(virDomainPtr dom,
     if (!ret)
         ret = virDomainSaveConfig(driver->configDir, vmdef);
 
+    /* If successfully saved the new vmdef, update it. this never fails */
+    if (!ret)
+        virDomainObjAssignDef(vm, vmdef, false);
+    else /* discard */
+        virDomainDefFree(vmdef);
+
     virDomainDeviceDefFree(device);
 
 endjob:
-- 
1.7.4.1


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