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

On Fri, 01 Apr 2011 14:10:18 +0800 Wen Congyang <wency(a)cn.fujitsu.com&gt; wrote: > At 04/01/2011 12:19 PM, KAMEZAWA Hiroyuki Write: >> >From 229cfc90781bfd7024f79db1aed8bea5963757e3 Mon Sep 17 00:00:00 2001 >> From: KAMEZAWA Hiroyuki <kamezawa.hiroyu(a)jp.fujitsu.com&gt; >> Date: Thu, 31 Mar 2011 18:52:24 +0900 >> Subject: [PATCHv8 2/4] libvirt/qemu - synchronous update of device definition. >> >> At persistent modification of inactive domains, we go several steps as >> - insert disk to vmdef >> - assign pci address if necessary >> - assign controller if necessary >> - save it to file > > Step 2 should be after step 3. > yes ;) >> >> 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 3 calls. >> virDomainTemporalCopyAlloc(vmdef) >> virDomainTemporalCopySync(orig, copy) >> virDomainTemporalCopyUndo(orig, copy) >> >> CopyAlloc() will create a copy of vmdef, CopySync() is for synchronizing >> copy and orginal, updating origina., CopyUndo() is for discarding for >> discarding unnecessary things in copy at failure. With these, vmdef >> modification can be done in following manner. >> >> copy = virDomainTemporalCopyAlloc(orig) >> ....do update on copy >> ....save updated data to its file >> if (error) >> virDomainTemporalCopyUndo(orig, copy); >> else >> virDomainTemporalCopySync(orig, copy) # this never fails. > > You only copy arrays in orig. How about copy all from orig to copy? > And we can introduce a new API virDomainObjAssignPersistentDef() that is > like the API virDomainObjAssignDef(). > > So vmdef modification can be done like this: > copy = virDomainTemporalCopyAlloc(orig) > ....do update on copy > if (error) { > virDomainDefFree(copy); > } else { > virDomainObjAssignPersistentDef(vm, copy) # this never fails. > ....save updated data to its file > } > > We can copy vmdef easily: > 1. xml = virDomainDefFormat(def, VIR_DOMAIN_XML_WRITE_FLAGS) > 2. newdef = virDomainDefParseString(caps, xml, VIR_DOMAIN_XML_READ_FLAGS) > Hmm, could you explain virDomainObjAssignPersistentDef() ? I think I should finally support moficication of active domains. Can I replace the whole vmdef of active domain ?

Thanks, -Kame