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

[Wen Congyang]

At 04/01/2011 02:16 PM, KAMEZAWA Hiroyuki Write:
> 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 ?

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.

> 
> Thanks,
> -Kame
> 
> 
>