[Pkg-libvirt-commits] [libguestfs] 10/63: p2v: Use libxml2 to write the libvirt XML document instead of printfs.

Hilko Bengen bengen at moszumanska.debian.org
Fri Oct 3 14:43:23 UTC 2014


This is an automated email from the git hooks/post-receive script.

bengen pushed a commit to annotated tag debian/1%1.27.39-1
in repository libguestfs.

commit c902ede1cd345258d6c388ab804fbf6a92b24ffd
Author: Richard W.M. Jones <rjones at redhat.com>
Date:   Tue Sep 2 14:25:16 2014 +0100

    p2v: Use libxml2 to write the libvirt XML document instead of printfs.
    
    Solves quoting issues.  This is just code reorganization, there is no
    substantive change in the output.
---
 p2v/conversion.c | 329 +++++++++++++++++++++++++++++++++++++------------------
 v2v/TODO         |   1 -
 2 files changed, 224 insertions(+), 106 deletions(-)

diff --git a/p2v/conversion.c b/p2v/conversion.c
index e422f2b..fc08ba2 100644
--- a/p2v/conversion.c
+++ b/p2v/conversion.c
@@ -35,6 +35,8 @@
 
 #include <glib.h>
 
+#include <libxml/xmlwriter.h>
+
 #include "miniexpect.h"
 #include "p2v.h"
 
@@ -379,6 +381,64 @@ cleanup_data_conns (struct data_conn *data_conns, size_t nr)
   }
 }
 
+/* Macros "inspired" by src/launch-libvirt.c */
+/* <element */
+#define start_element(element)                                        \
+  if (xmlTextWriterStartElement (xo, BAD_CAST (element)) == -1) {     \
+    set_conversion_error ("xmlTextWriterStartElement: %m");           \
+    return NULL;                                                      \
+  }                                                                   \
+  do
+
+/* finish current </element> */
+#define end_element()                                                   \
+  while (0);                                                            \
+  do {                                                                  \
+    if (xmlTextWriterEndElement (xo) == -1) {                           \
+      set_conversion_error ("xmlTextWriterEndElement: %m");             \
+      return NULL;                                                      \
+    }                                                                   \
+  } while (0)
+
+/* <element/> */
+#define empty_element(element) start_element(element) {} end_element ()
+
+/* key=value attribute of the current element. */
+#define attribute(key,value)                                            \
+  if (xmlTextWriterWriteAttribute (xo, BAD_CAST (key), BAD_CAST (value)) == -1) { \
+    set_conversion_error ("xmlTextWriterWriteAttribute: %m");           \
+    return NULL;                                                        \
+  }
+
+/* key=value, but value is a printf-style format string. */
+#define attribute_format(key,fs,...)                                    \
+  if (xmlTextWriterWriteFormatAttribute (xo, BAD_CAST (key),            \
+                                           fs, ##__VA_ARGS__) == -1) {  \
+    set_conversion_error ("xmlTextWriterWriteFormatAttribute: %m");     \
+    return NULL;                                                        \
+  }
+
+/* A string, eg. within an element. */
+#define string(str)                                                     \
+  if (xmlTextWriterWriteString (xo, BAD_CAST (str)) == -1) {            \
+    set_conversion_error ("xmlTextWriterWriteString: %m");              \
+    return NULL;                                                        \
+  }
+
+/* A string, using printf-style formatting. */
+#define string_format(fs,...)                                           \
+  if (xmlTextWriterWriteFormatString (xo, fs, ##__VA_ARGS__) == -1) {   \
+    set_conversion_error ("xmlTextWriterWriteFormatString: %m");        \
+    return NULL;                                                        \
+  }
+
+/* An XML comment. */
+#define comment(str)                                                  \
+  if (xmlTextWriterWriteComment (xo, BAD_CAST (str)) == -1) {         \
+    set_conversion_error ("xmlTextWriterWriteComment: %m");           \
+    return NULL;                                                      \
+  }
+
 /* Write the libvirt XML for this physical machine.  Note this is not
  * actually input for libvirt.  It's input for virt-v2v on the
  * conversion server, and virt-v2v will (if necessary) generate the
@@ -388,126 +448,185 @@ static char *
 generate_libvirt_xml (struct config *config, struct data_conn *data_conns)
 {
   uint64_t memkb;
-  FILE *fp;
-  char *ret = NULL;
-  size_t len = 0;
+  char *ret;
+  CLEANUP_XMLBUFFERFREE xmlBufferPtr xb = NULL;
+  xmlOutputBufferPtr ob;
+  CLEANUP_XMLFREETEXTWRITER xmlTextWriterPtr xo = NULL;
   size_t i;
 
-  fp = open_memstream (&ret, &len);
-  if (fp == NULL) {
-    set_conversion_error ("open_memstream: %m");
+  xb = xmlBufferCreate ();
+  if (xb == NULL) {
+    set_conversion_error ("xmlBufferCreate: %m");
     return NULL;
   }
-
-  memkb = config->memory / 1024;
-
-  fprintf (fp,
-           "<!--\n"
-           "  NOTE!\n"
-           "\n"
-           "  This libvirt XML is generated by the virt-p2v front end, in\n"
-           "  order to communicate with the backend virt-v2v process running\n"
-           "  on the conversion server.  It is a minimal description of the\n"
-           "  physical machine.  If the target of the conversion is libvirt,\n"
-           "  then virt-v2v will generate the real target libvirt XML, which\n"
-           "  has only a little to do with the XML in this file.\n"
-           "\n"
-           "  For the code that generates this XML, see %s in the virt-p2v\n"
-           "  sources (in the libguestfs package).\n"
-           "-->\n"
-           "\n",
-           __FILE__);
-
-  /* XXX quoting needs to be improved here XXX */
-  fprintf (fp,
-           "<domain type='physical'>\n"
-           "  <name>%s</name>\n"
-           "  <memory unit='KiB'>%" PRIu64 "</memory>\n"
-           "  <currentMemory unit='KiB'>%" PRIu64 "</currentMemory>\n"
-           "  <vcpu>%d</vcpu>\n"
-           "  <os>\n"
-           "    <type arch='" host_cpu "'>hvm</type>\n"
-           "  </os>\n"
-           "  <features>%s%s%s</features>\n"
-           "  <devices>\n",
-           config->guestname,
-           memkb, memkb,
-           config->vcpus,
-           config->flags & FLAG_ACPI ? "<acpi/>" : "",
-           config->flags & FLAG_APIC ? "<apic/>" : "",
-           config->flags & FLAG_PAE  ? "<pae/>" : "");
-
-  for (i = 0; config->disks[i] != NULL; ++i) {
-    char target_dev[64];
-
-    if (config->disks[i][0] == '/') {
-    target_sd:
-      memcpy (target_dev, "sd", 2);
-      guestfs___drive_name (i, &target_dev[2]);
-    } else {
-      if (strlen (config->disks[i]) <= sizeof (target_dev) - 1)
-        strcpy (target_dev, config->disks[i]);
-      else
-        goto target_sd;
-    }
-
-    fprintf (fp,
-             "    <disk type='network' device='disk'>\n"
-             "      <driver name='qemu' type='raw'/>\n"
-             "      <source protocol='nbd'>\n"
-             "        <host name='localhost' port='%d'/>\n"
-             "      </source>\n"
-             "      <target dev='%s'/>\n"
-             "    </disk>\n",
-             data_conns[i].nbd_remote_port, target_dev);
+  ob = xmlOutputBufferCreateBuffer (xb, NULL);
+  if (ob == NULL) {
+    set_conversion_error ("xmlOutputBufferCreateBuffer: %m");
+    return NULL;
+  }
+  xo = xmlNewTextWriter (ob);
+  if (xo == NULL) {
+    set_conversion_error ("xmlNewTextWriter: %m");
+    return NULL;
   }
 
-  if (config->removable) {
-    for (i = 0; config->removable[i] != NULL; ++i) {
-      fprintf (fp,
-               "    <disk type='network' device='cdrom'>\n"
-               "      <driver name='qemu' type='raw'/>\n"
-               "      <target dev='%s'/>\n"
-               "    </disk>\n",
-               config->removable[i]);
-    }
+  if (xmlTextWriterSetIndent (xo, 1) == -1 ||
+      xmlTextWriterSetIndentString (xo, BAD_CAST "  ") == -1) {
+    set_conversion_error ("could not set XML indent: %m");
+    return NULL;
+  }
+  if (xmlTextWriterStartDocument (xo, NULL, NULL, NULL) == -1) {
+    set_conversion_error ("xmlTextWriterStartDocument: %m");
+    return NULL;
   }
 
-  if (config->interfaces) {
-    for (i = 0; config->interfaces[i] != NULL; ++i) {
-      CLEANUP_FREE char *mac_filename = NULL;
-      CLEANUP_FREE char *mac = NULL;
+  memkb = config->memory / 1024;
 
-      if (asprintf (&mac_filename, "/sys/class/net/%s/address",
-                    config->interfaces[i]) == -1) {
-        perror ("asprintf");
-        exit (EXIT_FAILURE);
+  comment
+    (" NOTE!\n"
+     "\n"
+     "  This libvirt XML is generated by the virt-p2v front end, in\n"
+     "  order to communicate with the backend virt-v2v process running\n"
+     "  on the conversion server.  It is a minimal description of the\n"
+     "  physical machine.  If the target of the conversion is libvirt,\n"
+     "  then virt-v2v will generate the real target libvirt XML, which\n"
+     "  has only a little to do with the XML in this file.\n"
+     "\n"
+     "  TL;DR: Don't try to load this XML into libvirt. ");
+
+  start_element ("domain") {
+    attribute ("type", "physical");
+
+    start_element ("name") {
+      string (config->guestname);
+    } end_element ();
+
+    start_element ("memory") {
+      attribute ("unit", "KiB");
+      string_format ("%" PRIu64, memkb);
+    } end_element ();
+
+    start_element ("currentMemory") {
+      attribute ("unit", "KiB");
+      string_format ("%" PRIu64, memkb);
+    } end_element ();
+
+    start_element ("vcpu") {
+      string_format ("%d", config->vcpus);
+    } end_element ();
+
+    start_element ("os") {
+      start_element ("type") {
+        attribute ("arch", host_cpu);
+        string ("hvm");
+      } end_element ();
+    } end_element ();
+
+    start_element ("features") {
+      if (config->flags & FLAG_ACPI) empty_element ("acpi");
+      if (config->flags & FLAG_APIC) empty_element ("apic");
+      if (config->flags & FLAG_PAE)  empty_element ("pae");
+    } end_element ();
+
+    start_element ("devices") {
+
+      for (i = 0; config->disks[i] != NULL; ++i) {
+        char target_dev[64];
+
+        if (config->disks[i][0] == '/') {
+        target_sd:
+          memcpy (target_dev, "sd", 2);
+          guestfs___drive_name (i, &target_dev[2]);
+        } else {
+          if (strlen (config->disks[i]) <= sizeof (target_dev) - 1)
+            strcpy (target_dev, config->disks[i]);
+          else
+            goto target_sd;
+        }
+
+        start_element ("disk") {
+          attribute ("type", "network");
+          attribute ("device", "disk");
+          start_element ("driver") {
+            attribute ("name", "qemu");
+            attribute ("type", "raw");
+          } end_element ();
+          start_element ("source") {
+            attribute ("protocol", "nbd");
+            start_element ("host") {
+              attribute ("name", "localhost");
+              attribute_format ("port", "%d", data_conns[i].nbd_remote_port);
+            } end_element ();
+          } end_element ();
+          start_element ("target") {
+            attribute ("dev", target_dev);
+          } end_element ();
+        } end_element ();
       }
-      if (g_file_get_contents (mac_filename, &mac, NULL, NULL)) {
-        size_t len = strlen (mac);
 
-        if (len > 0 && mac[len-1] == '\n')
-          mac[len-1] = '\0';
+      if (config->removable) {
+        for (i = 0; config->removable[i] != NULL; ++i) {
+          start_element ("disk") {
+            attribute ("type", "network");
+            attribute ("device", "cdrom");
+            start_element ("driver") {
+              attribute ("name", "qemu");
+              attribute ("type", "raw");
+            } end_element ();
+            start_element ("target") {
+              attribute ("dev", config->removable[i]);
+            } end_element ();
+          } end_element ();
+        }
       }
 
-      fprintf (fp,
-               "    <interface type='network'>\n"
-               "      <source network='default'/>\n"
-               "      <target dev='%s'/>\n",
-               config->interfaces[i]);
-      if (mac)
-        fprintf (fp, "      <mac address='%s'/>\n", mac);
-      fprintf (fp,
-               "    </interface>\n");
-    }
-  }
+      if (config->interfaces) {
+        for (i = 0; config->interfaces[i] != NULL; ++i) {
+          CLEANUP_FREE char *mac_filename = NULL;
+          CLEANUP_FREE char *mac = NULL;
+
+          if (asprintf (&mac_filename, "/sys/class/net/%s/address",
+                        config->interfaces[i]) == -1) {
+            perror ("asprintf");
+            exit (EXIT_FAILURE);
+          }
+          if (g_file_get_contents (mac_filename, &mac, NULL, NULL)) {
+            size_t len = strlen (mac);
+
+            if (len > 0 && mac[len-1] == '\n')
+              mac[len-1] = '\0';
+          }
+
+          start_element ("interface") {
+            attribute ("type", "network");
+            start_element ("source") {
+              attribute ("network", "default");
+            } end_element ();
+            start_element ("target") {
+              attribute ("dev", config->interfaces[i]);
+            } end_element ();
+            if (mac) {
+              start_element ("mac") {
+                attribute ("address", mac);
+              } end_element ();
+            }
+          } end_element ();
+        }
+      }
+
+    } end_element (); /* </devices> */
 
-  /* Old virt-p2v didn't try to model the graphics hardware. */
+  } end_element (); /* </domain> */
 
-  fprintf (fp,
-           "  </devices>\n"
-           "</domain>\n");
-  fclose (fp);
+  if (xmlTextWriterEndDocument (xo) == -1) {
+    set_conversion_error ("xmlTextWriterEndDocument: %m");
+    return NULL;
+  }
+  ret = (char *) xmlBufferDetach (xb); /* caller frees */
+  if (ret == NULL) {
+    set_conversion_error ("xmlBufferDetach: %m");
+    return NULL;
+  }
 
   return ret;
 }
diff --git a/v2v/TODO b/v2v/TODO
index a44e9fb..15ec931 100644
--- a/v2v/TODO
+++ b/v2v/TODO
@@ -13,7 +13,6 @@ p2v:
  - network dialog and network configuration
  - GUI controls for network mapping
  - why is the Back button insensitive?
- - use libxml2 in p2v/conversion.c to generate libvirt XML
 
 p2v/gui.c:  /* XXX It would be nice not to have to set this explicitly, but
 p2v/gui.c:  /* XXX Add list of input and output drivers to virt-v2v --machine-re

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-libvirt/libguestfs.git



More information about the Pkg-libvirt-commits mailing list