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

1 Apr 2011


      At 04/01/2011 02:16 PM, KAMEZAWA Hiroyuki Write:
...
On Fri, 01 Apr 2011 14:10:18 +0800
Wen Congyang <wency@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@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