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

[KAMEZAWA Hiroyuki]

On Fri, 01 Apr 2011 14:10:18 +0800
Wen Congyang <wency at cn.fujitsu.com> 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 at jp.fujitsu.com>
> > 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