[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] qemu: Transfer inactive XML among cookie



On 09/15/2011 08:04 AM, Michal Privoznik wrote:
If a domain has inactive XML we want to transfer it to destination
when migrating with VIR_MIGRATE_PERSIST_DEST. In order to harm
the migration protocol as least as possible, a optional cookie was
chosen.
---
  src/qemu/qemu_migration.c |   91 ++++++++++++++++++++++++++++++++++++++++----
  1 files changed, 82 insertions(+), 9 deletions(-)


Now I've actually looked in depth at the code.

@@ -388,15 +415,25 @@ static void qemuMigrationCookieXMLFormat(virBufferPtr buf,
          virBufferAddLit(buf, "</lockstate>\n");
      }

+    if ((mig->flags&  QEMU_MIGRATION_COOKIE_PERSISTENT)&&
+        mig->persistent) {
+        domXML = qemuDomainDefFormatXML(driver, mig->persistent,
+                                        VIR_DOMAIN_XML_INACTIVE |
+                                        VIR_DOMAIN_XML_SECURE);

I'm wondering if this should use virDomainDefParseString instead of qemuDomainDefFormatXML; after all, the latter only acts on VIR_DOMAIN_XML_UPDATE_CPU (which we aren't passing), while the former doesn't need a driver argument. On the other hand, it's the same thing we used in qemu_driver.c:qemuDomainSaveImageDefineXML, so we have precedent. I guess it's up to you whether to refactor and post v2, or to commit this as is and save that sort of cleanup for a separate patch.

+        virBufferAdd(buf, domXML, -1);
+        VIR_FREE(domXML);
+    }
+
      virBufferAddLit(buf, "</qemu-migration>\n");

Oh cool - another place besides my snapshot work where making domain XML formatting auto-indented would produce nicer-looking output. Not a show-stopper for this patch, but I'll have to remember it when I refactor domain_conf.c to pass indentation parameters around.

I don't know if you want to do a v2 that avoids adding the driver argument in that many places, but if not, then I didn't see anything wrong with this code.

I can live with ACK for this patch as-is, although if you decide to refactor the domain formatting, then it's probably worth posting a v2 for review.

--
Eric Blake   eblake redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]