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

On Fri, 01 Apr 2011 14:33:23 +0800 Wen Congyang <wency(a)cn.fujitsu.com&gt; wrote: Okay, here is a replacement. it seems to work fine. Patch 3 and 4 can be applied without hunk. == From: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com&gt; 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(a)jp.fujitsu.com&gt; --- 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