Libguestfs - Mar 2017 - [PATCH 0/4] Pass CPU vendor, model and topology from source to target.

This is tangentially related to:

  https://bugzilla.redhat.com/show_bug.cgi?id=1372668

The problem in that bug is that we didn't pass the source CPU model
(Sandybridge in that case) through to the target RHV hypervisor.  So
when the Windows guest booted on the target it gives an error about
CPU hardware being disconnected (although it otherwise boots and works
fine).

This patch series explores sending the CPU vendor, model and topology
from the source to the target, where supported.  It turns out that RHV
doesn't seem to support this anyway, but other target hypervisors do.

Rich.

In the fake <domain type='physical'> libvirt XML that we create to
describe the physical host, we did not accurately pass any information
about the host CPU except the number of cores (<vcpu/>).

This commit extracts detailed information about the vendor, model and
topology of the host CPU and adds that to the libvirt XML for
virt-v2v.  Conveniently we can use libvirt capabilities to get this
information without needing to parse /proc/cpuinfo or similar
techniques.

The libvirt XML looks like this:

  <domain type="physical">
  ...
    <cpu match="minimum">
      <vendor>Intel</vendor>
      <model fallback="allow">Broadwell</model>
      <topology sockets="1" cores="2"
threads="2"/>
    </cpu>
  ...
    <features>
      <acpi/>
      <apic/>
      <pae/>
    </features>
---
 p2v/Makefile.am     |  12 ++-
 p2v/config.c        |  18 +++-
 p2v/conversion.c    |  35 ++++++-
 p2v/cpuid.c         | 294 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 p2v/dependencies.m4 |   5 +
 p2v/main.c          |  56 +---------
 p2v/p2v.h           |  20 +++-
 7 files changed, 373 insertions(+), 67 deletions(-)
 create mode 100644 p2v/cpuid.c

diff --git a/p2v/Makefile.am b/p2v/Makefile.am
index 0152dc1..9751a07 100644
--- a/p2v/Makefile.am
+++ b/p2v/Makefile.am
@@ -74,6 +74,7 @@ virt_p2v_SOURCES = \
 	about-license.c \
 	config.c \
 	conversion.c \
+	cpuid.c \
 	gui.c \
 	gui-gtk2-compat.h \
 	gui-gtk3-compat.h \
@@ -97,6 +98,7 @@ virt_p2v_CPPFLAGS = \
 virt_p2v_CFLAGS = \
 	-pthread \
 	$(WARN_CFLAGS) $(WERROR_CFLAGS) \
+	$(LIBVIRT_CFLAGS) \
 	$(PCRE_CFLAGS) \
 	$(LIBXML2_CFLAGS) \
 	$(GTK_CFLAGS) \
@@ -105,6 +107,7 @@ virt_p2v_CFLAGS = \
 virt_p2v_LDADD = \
 	$(top_builddir)/common/utils/libutils.la \
 	$(top_builddir)/common/miniexpect/libminiexpect.la \
+	$(LIBVIRT_LIBS) \
 	$(PCRE_LIBS) \
 	$(LIBXML2_LIBS) \
 	$(GTK_LIBS) \
@@ -120,9 +123,16 @@ dependencies_files = \
 	dependencies.redhat \
 	dependencies.suse
 
+if HAVE_LIBVIRT
+dependencies_have_libvirt = -DHAVE_LIBVIRT=1
+endif
+
 $(dependencies_files): dependencies.m4
 	define=`echo $@ | $(SED)
's/dependencies.//;y/abcdefghijklmnopqrstuvwxyz/ABCDEFGHIJKLMNOPQRSTUVWXYZ/'`;
\
-	m4 -D$$define=1 -DGTK_VERSION=$(GTK_VERSION) $< > $@-t
+	m4 -D$$define=1 \
+	   -DGTK_VERSION=$(GTK_VERSION) \
+	   $(dependencies_have_libvirt) \
+	   $< > $@-t
 	mv $@-t $@
 
 # Support files needed by the virt-p2v-make-* scripts.
diff --git a/p2v/config.c b/p2v/config.c
index 054b07c..f9f610c 100644
--- a/p2v/config.c
+++ b/p2v/config.c
@@ -95,6 +95,8 @@ free_config (struct config *c)
   free (c->identity_url);
   free (c->identity_file);
   free (c->guestname);
+  free (c->cpu.vendor);
+  free (c->cpu.model);
   guestfs_int_free_string_list (c->disks);
   guestfs_int_free_string_list (c->removable);
   guestfs_int_free_string_list (c->interfaces);
@@ -132,10 +134,20 @@ print_config (struct config *config, FILE *fp)
            config->guestname ? config->guestname : "none");
   fprintf (fp, "vcpus  . . . . .   %d\n", config->vcpus);
   fprintf (fp, "memory . . . . .   %" PRIu64 "\n",
config->memory);
+  if (config->cpu.vendor)
+    fprintf (fp, "cpu vendor . . .   %s\n", config->cpu.vendor);
+  if (config->cpu.model)
+    fprintf (fp, "cpu model  . . .   %s\n", config->cpu.model);
+  if (config->cpu.sockets > 0)
+    fprintf (fp, "cpu sockets  . .   %u\n", config->cpu.sockets);
+  if (config->cpu.cores > 0)
+    fprintf (fp, "cpu cores  . . .   %u\n", config->cpu.cores);
+  if (config->cpu.threads > 0)
+    fprintf (fp, "cpu threads  . .   %u\n", config->cpu.threads);
   fprintf (fp, "flags  . . . . .  %s%s%s\n",
-           config->flags & FLAG_ACPI ? " acpi" : "",
-           config->flags & FLAG_APIC ? " apic" : "",
-           config->flags & FLAG_PAE  ? " pae"  :
"");
+           config->cpu.acpi ? " acpi" : "",
+           config->cpu.apic ? " apic" : "",
+           config->cpu.pae  ? " pae"  : "");
   fprintf (fp, "disks  . . . . .  ");
   if (config->disks != NULL) {
     for (i = 0; config->disks[i] != NULL; ++i)
diff --git a/p2v/conversion.c b/p2v/conversion.c
index 0c17ef2..55fbfb1 100644
--- a/p2v/conversion.c
+++ b/p2v/conversion.c
@@ -612,6 +612,35 @@ generate_libvirt_xml (struct config *config, struct
data_conn *data_conns,
       string_format ("%d", config->vcpus);
     } end_element ();
 
+    if (config->cpu.vendor || config->cpu.model ||
+        config->cpu.sockets || config->cpu.cores ||
config->cpu.threads) {
+      /* https://libvirt.org/formatdomain.html#elementsCPU */
+      start_element ("cpu") {
+        attribute ("match", "minimum");
+        if (config->cpu.vendor) {
+          start_element ("vendor") {
+            string (config->cpu.vendor);
+          } end_element ();
+        }
+        if (config->cpu.model) {
+          start_element ("model") {
+            attribute ("fallback", "allow");
+            string (config->cpu.model);
+          } end_element ();
+        }
+        if (config->cpu.sockets || config->cpu.cores ||
config->cpu.threads) {
+          start_element ("topology") {
+            if (config->cpu.sockets)
+              attribute_format ("sockets", "%u",
config->cpu.sockets);
+            if (config->cpu.cores)
+              attribute_format ("cores", "%u",
config->cpu.cores);
+            if (config->cpu.threads)
+              attribute_format ("threads", "%u",
config->cpu.threads);
+          } end_element ();
+        }
+      } end_element ();
+    }
+
     start_element ("os") {
       start_element ("type") {
         attribute ("arch", host_cpu);
@@ -620,9 +649,9 @@ generate_libvirt_xml (struct config *config, struct
data_conn *data_conns,
     } 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");
+      if (config->cpu.acpi) empty_element ("acpi");
+      if (config->cpu.apic) empty_element ("apic");
+      if (config->cpu.pae)  empty_element ("pae");
     } end_element ();
 
     start_element ("devices") {
diff --git a/p2v/cpuid.c b/p2v/cpuid.c
new file mode 100644
index 0000000..b7d4a4d
--- /dev/null
+++ b/p2v/cpuid.c
@@ -0,0 +1,294 @@
+/* virt-p2v
+ * Copyright (C) 2009-2017 Red Hat Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
USA.
+ */
+
+/**
+ * Process CPU capabilities into libvirt-compatible
C<E<lt>cpuE<gt>> data.
+ *
+ * If libvirt is available at compile time then this is quite
+ * simple - libvirt API C<virConnectGetCapabilities> provides
+ * a C<E<lt>hostE<ge>> element which has mostly what we need.
+ *
+ * Flags C<acpi>, C<apic>, C<pae> still have to be parsed out
of
+ * F</proc/cpuinfo> because these will not necessarily be present in
+ * the libvirt capabilities directly (they are implied by the
+ * processor model, requiring a complex lookup in the CPU map).
+ *
+ * Note that #vCPUs and amount of RAM is handled by F<main.c>.
+ *
+ * See: L<https://libvirt.org/formatdomain.html#elementsCPU>
+ */
+
+#include <config.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdarg.h>
+#include <string.h>
+#include <errno.h>
+#include <libintl.h>
+
+#ifdef HAVE_LIBVIRT
+#include <libvirt/libvirt.h>
+#include <libvirt/virterror.h>
+#endif
+
+#include <libxml/xpath.h>
+
+#include "getprogname.h"
+#include "ignore-value.h"
+
+#include "p2v.h"
+
+static void
+free_cpu_config (struct cpu_config *cpu)
+{
+  if (cpu->vendor)
+    free (cpu->vendor);
+  if (cpu->model)
+    free (cpu->model);
+  memset (cpu, 0, sizeof *cpu);
+}
+
+/**
+ * Read flags from F</proc/cpuinfo>.
+ */
+static void
+cpuinfo_flags (struct cpu_config *cpu)
+{
+  const char *cmd;
+  CLEANUP_PCLOSE FILE *fp = NULL;
+  CLEANUP_FREE char *flag = NULL;
+  ssize_t len;
+  size_t buflen = 0;
+
+  /* Get the flags, one per line. */
+  cmd = "< /proc/cpuinfo "
+#if defined(__arm__)
+    "grep ^Features"
+#else
+    "grep ^flags"
+#endif
+    " | awk '{ for (i = 3; i <= NF; ++i) { print $i }; exit
}'";
+
+  fp = popen (cmd, "re");
+  if (fp == NULL) {
+    perror ("/proc/cpuinfo");
+    return;
+  }
+
+  while (errno = 0, (len = getline (&flag, &buflen, fp)) != -1) {
+    if (len > 0 && flag[len-1] == '\n')
+      flag[len-1] = '\0';
+
+    if (STREQ (flag, "acpi"))
+      cpu->acpi = 1;
+    else if (STREQ (flag, "apic"))
+      cpu->apic = 1;
+    else if (STREQ (flag, "pae"))
+      cpu->pae = 1;
+  }
+
+  if (errno) {
+    perror ("getline");
+    return;
+  }
+}
+
+#ifdef HAVE_LIBVIRT
+
+static void
+ignore_errors (void *ignore, virErrorPtr ignore2)
+{
+  /* empty */
+}
+
+static void libvirt_error (const char *fs, ...) __attribute__((format
(printf,1,2)));
+
+static void
+libvirt_error (const char *fs, ...)
+{
+  va_list args;
+  CLEANUP_FREE char *msg = NULL;
+  int len;
+  virErrorPtr err;
+
+  va_start (args, fs);
+  len = vasprintf (&msg, fs, args);
+  va_end (args);
+
+  if (len < 0) goto fallback;
+
+  /* In all recent libvirt, this retrieves the thread-local error. */
+  err = virGetLastError ();
+  if (err)
+    fprintf (stderr,
+             "%s: %s: %s [code=%d int1=%d]\n",
+             getprogname (), msg, err->message, err->code, err->int1);
+  else
+  fallback:
+    fprintf (stderr, "%s: %s\n", getprogname (), msg);
+}
+
+/**
+ * Read the capabilities from libvirt and parse out the fields
+ * we care about.
+ */
+static void
+libvirt_capabilities (struct cpu_config *cpu)
+{
+  virConnectPtr conn;
+  CLEANUP_FREE char *capabilities_xml = NULL;
+  CLEANUP_XMLFREEDOC xmlDocPtr doc = NULL;
+  CLEANUP_XMLXPATHFREECONTEXT xmlXPathContextPtr xpathCtx = NULL;
+  CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xpathObj = NULL;
+  xmlNodeSetPtr nodes;
+  size_t nr_nodes, i;
+  xmlNodePtr node;
+
+  /* Connect to libvirt and get the capabilities XML. */
+  conn = virConnectOpenReadOnly (NULL);
+  if (!conn) {
+    libvirt_error (_("could not connect to libvirt"));
+    return;
+  }
+
+  /* Suppress default behaviour of printing errors to stderr.  Note
+   * you can't set this to NULL to ignore errors; setting it to NULL
+   * restores the default error handler ...
+   */
+  virConnSetErrorFunc (conn, NULL, ignore_errors);
+
+  capabilities_xml = virConnectGetCapabilities (conn);
+  if (!capabilities_xml) {
+    libvirt_error (_("could not get libvirt capabilities"));
+    virConnectClose (conn);
+    return;
+  }
+
+  /* Parse the capabilities XML with libxml2. */
+  doc = xmlReadMemory (capabilities_xml, strlen (capabilities_xml),
+                       NULL, NULL, XML_PARSE_NONET);
+  if (doc == NULL) {
+    fprintf (stderr,
+             _("%s: unable to parse capabilities XML returned by
libvirt\n"),
+             getprogname ());
+    return;
+  }
+
+  xpathCtx = xmlXPathNewContext (doc);
+  if (xpathCtx == NULL) {
+    fprintf (stderr, _("%s: unable to create new XPath context\n"),
+             getprogname ());
+    return;
+  }
+
+  /* Get the CPU vendor. */
+  xpathObj +    xmlXPathEvalExpression (BAD_CAST
"/capabilities/host/cpu/vendor/text()",
+                            xpathCtx);
+  if (xpathObj == NULL) {
+    fprintf (stderr, _("%s: %s: %d: unable to evaluate xpath
expression\n"),
+             getprogname (), __FILE__, __LINE__);
+    return;
+  }
+  nodes = xpathObj->nodesetval;
+  nr_nodes = nodes->nodeNr;
+  if (nr_nodes > 0) {
+    node = nodes->nodeTab[0];
+    cpu->vendor = (char *) xmlNodeGetContent (node);
+  }
+
+  /* Get the CPU model. */
+  xmlXPathFreeObject (xpathObj);
+  xpathObj +    xmlXPathEvalExpression (BAD_CAST
"/capabilities/host/cpu/model/text()",
+                            xpathCtx);
+  if (xpathObj == NULL) {
+    fprintf (stderr, _("%s: %s: %d: unable to evaluate xpath
expression\n"),
+             getprogname (), __FILE__, __LINE__);
+    return;
+  }
+  nodes = xpathObj->nodesetval;
+  nr_nodes = nodes->nodeNr;
+  if (nr_nodes > 0) {
+    node = nodes->nodeTab[0];
+    cpu->model = (char *) xmlNodeGetContent (node);
+  }
+
+  /* Get the topology.  Note the XPath expression returns all
+   * attributes of the <topology> node.
+   */
+  xmlXPathFreeObject (xpathObj);
+  xpathObj +    xmlXPathEvalExpression (BAD_CAST
"/capabilities/host/cpu/topology/@*",
+                            xpathCtx);
+  if (xpathObj == NULL) {
+    fprintf (stderr, _("%s: %s: %d: unable to evaluate xpath
expression\n"),
+             getprogname (), __FILE__, __LINE__);
+    return;
+  }
+  nodes = xpathObj->nodesetval;
+  nr_nodes = nodes->nodeNr;
+  /* Iterate over the attributes of the <topology> node. */
+  for (i = 0; i < nr_nodes; ++i) {
+    node = nodes->nodeTab[i];
+
+    if (node->type == XML_ATTRIBUTE_NODE) {
+      xmlAttrPtr attr = (xmlAttrPtr) node;
+      CLEANUP_FREE char *content = NULL;
+      unsigned *up;
+
+      if (STREQ ((const char *) attr->name, "sockets")) {
+        up = &cpu->sockets;
+      parse_attr:
+        *up = 0;
+        content = (char *) xmlNodeListGetString (doc, attr->children, 1);
+        if (content)
+          ignore_value (sscanf (content, "%u", up));
+      }
+      else if (STREQ ((const char *) attr->name, "cores")) {
+        up = &cpu->cores;
+        goto parse_attr;
+      }
+      else if (STREQ ((const char *) attr->name, "threads")) {
+        up = &cpu->threads;
+        goto parse_attr;
+      }
+    }
+  }
+}
+
+#else /* !HAVE_LIBVIRT */
+
+static void
+libvirt_capabilities (struct cpu_config *cpu)
+{
+  fprintf (stderr,
+           _("%s: program was compiled without libvirt support\n"),
+           getprogname ());
+}
+
+#endif /* !HAVE_LIBVIRT */
+
+void
+get_cpu_config (struct cpu_config *cpu)
+{
+  free_cpu_config (cpu);
+  libvirt_capabilities (cpu);
+  cpuinfo_flags (cpu);
+}
diff --git a/p2v/dependencies.m4 b/p2v/dependencies.m4
index 21541b4..adbac26 100644
--- a/p2v/dependencies.m4
+++ b/p2v/dependencies.m4
@@ -25,6 +25,8 @@ ifelse(REDHAT,1,
   libxml2
   gtk`'GTK_VERSION
   dbus-libs
+  dnl libvirt is optional, used just to parse the host CPU capabilities.
+  ifdef(`HAVE_LIBVIRT', `libvirt-libs')
 
   dnl Run as external programs by the p2v binary.
   /usr/bin/ssh
@@ -79,6 +81,7 @@ ifelse(DEBIAN,1,
   libxml2
   libgtk`'GTK_VERSION`'.0-0
   libdbus-1-3
+  ifdef(`HAVE_LIBVIRT', `libvirt0')
   openssh-client
   qemu-utils
   gawk
@@ -112,6 +115,7 @@ ifelse(ARCHLINUX,1,
   libxml2
   gtk`'GTK_VERSION
   dbus
+  ifdef(`HAVE_LIBVIRT', `libvirt')
   openssh
   qemu
   gawk
@@ -146,6 +150,7 @@ ifelse(SUSE,1,
   libxml2
   gtk`'GTK_VERSION
   libdbus-1-3
+  ifdef(`HAVE_LIBVIRT', `libvirt-libs')
   qemu-tools
   openssh
   gawk
diff --git a/p2v/main.c b/p2v/main.c
index af14240..e1a7550 100644
--- a/p2v/main.c
+++ b/p2v/main.c
@@ -65,7 +65,6 @@ static void udevadm_settle (void);
 static void set_config_defaults (struct config *config);
 static void find_all_disks (void);
 static void find_all_interfaces (void);
-static int cpuinfo_flags (void);
 
 enum { HELP_OPTION = CHAR_MAX + 1 };
 static const char options[] = "Vv";
@@ -276,7 +275,6 @@ set_config_defaults (struct config *config)
 {
   long i;
   char hostname[257];
-  int flags;
 
   /* Default guest name is derived from the source hostname.  If we
    * assume that the p2v ISO gets its IP address and hostname from
@@ -340,11 +338,7 @@ set_config_defaults (struct config *config)
   config->memory |= config->memory >> 32;
   config->memory++;
 
-  flags = cpuinfo_flags ();
-  if (flags >= 0)
-    config->flags = flags;
-  else
-    config->flags = 0;
+  get_cpu_config (&config->cpu);
 
   /* Find all block devices in the system. */
   if (!test_disk)
@@ -585,51 +579,3 @@ find_all_interfaces (void)
   if (all_interfaces)
     qsort (all_interfaces, nr_interfaces, sizeof (char *), compare);
 }
-
-/**
- * Read the list of flags from F</proc/cpuinfo>.
- */
-static int
-cpuinfo_flags (void)
-{
-  const char *cmd;
-  CLEANUP_PCLOSE FILE *fp = NULL;
-  CLEANUP_FREE char *flag = NULL;
-  ssize_t len;
-  size_t buflen = 0;
-  int ret = 0;
-
-  /* Get the flags, one per line. */
-  cmd = "< /proc/cpuinfo "
-#if defined(__arm__)
-    "grep ^Features"
-#else
-    "grep ^flags"
-#endif
-    " | awk '{ for (i = 3; i <= NF; ++i) { print $i }; exit
}'";
-
-  fp = popen (cmd, "re");
-  if (fp == NULL) {
-    perror ("/proc/cpuinfo");
-    return -1;
-  }
-
-  while (errno = 0, (len = getline (&flag, &buflen, fp)) != -1) {
-    if (len > 0 && flag[len-1] == '\n')
-      flag[len-1] = '\0';
-
-    if (STREQ (flag, "acpi"))
-      ret |= FLAG_ACPI;
-    else if (STREQ (flag, "apic"))
-      ret |= FLAG_APIC;
-    else if (STREQ (flag, "pae"))
-      ret |= FLAG_PAE;
-  }
-
-  if (errno) {
-    perror ("getline");
-    return -1;
-  }
-
-  return ret;
-}
diff --git a/p2v/p2v.h b/p2v/p2v.h
index 5223aa2..69ed35c 100644
--- a/p2v/p2v.h
+++ b/p2v/p2v.h
@@ -59,6 +59,17 @@ extern int feature_colours_option;
 extern int force_colour;
 
 /* config.c */
+struct cpu_config {
+  char *vendor;                 /* eg. "Intel" */
+  char *model;                  /* eg. "Broadwell" */
+  unsigned sockets;             /* number of sockets */
+  unsigned cores;               /* number of cores per socket */
+  unsigned threads;             /* number of hyperthreads per core */
+  int acpi;
+  int apic;
+  int pae;
+};
+
 struct config {
   char *server;
   int port;
@@ -71,7 +82,7 @@ struct config {
   char *guestname;
   int vcpus;
   uint64_t memory;
-  int flags;
+  struct cpu_config cpu;
   char **disks;
   char **removable;
   char **interfaces;
@@ -83,10 +94,6 @@ struct config {
   char *output_storage;
 };
 
-#define FLAG_ACPI 1
-#define FLAG_APIC 2
-#define FLAG_PAE  4
-
 #define OUTPUT_ALLOCATION_NONE         0
 #define OUTPUT_ALLOCATION_SPARSE       1
 #define OUTPUT_ALLOCATION_PREALLOCATED 2
@@ -96,6 +103,9 @@ extern struct config *copy_config (struct config *);
 extern void free_config (struct config *);
 extern void print_config (struct config *, FILE *);
 
+/* cpuid.c */
+extern void get_cpu_config (struct cpu_config *);
+
 /* kernel-cmdline.c */
 extern char **parse_cmdline_string (const char *cmdline);
 extern char **parse_proc_cmdline (void);
-- 
2.10.2

This rearranges the input files for the test of virt-v2v
--print-source, but does not change its semantics.
---
 v2v/test-v2v-print-source.expected | 14 ++++++++++++++
 v2v/test-v2v-print-source.sh       | 26 +++-----------------------
 v2v/test-v2v-print-source.xml      | 23 +++++++++++++++++++++++
 3 files changed, 40 insertions(+), 23 deletions(-)
 create mode 100644 v2v/test-v2v-print-source.expected
 create mode 100644 v2v/test-v2v-print-source.xml

diff --git a/v2v/test-v2v-print-source.expected
b/v2v/test-v2v-print-source.expected
new file mode 100644
index 0000000..b947927
--- /dev/null
+++ b/v2v/test-v2v-print-source.expected
@@ -0,0 +1,14 @@
+    source name: windows
+hypervisor type: kvm
+         memory: 1073741824 (bytes)
+       nr vCPUs: 1
+   CPU features: 
+       firmware: unknown
+        display: 
+          video: qxl
+          sound: 
+disks:
+	windows.img (raw) [virtio-blk]
+removable media:
+NICs:
+	Network "default" mac: 00:11:22:33:44:55 [virtio]
diff --git a/v2v/test-v2v-print-source.sh b/v2v/test-v2v-print-source.sh
index 62899e6..c6f00a2 100755
--- a/v2v/test-v2v-print-source.sh
+++ b/v2v/test-v2v-print-source.sh
@@ -24,15 +24,12 @@ $TEST_FUNCTIONS
 skip_if_skipped
 skip_unless_phony_guest windows.img
 
-libvirt_uri="test://$abs_top_builddir/test-data/phony-guests/guests.xml"
-f=$top_builddir/test-data/phony-guests/windows.img
-
 d=test-v2v-print-source.d
 rm -rf $d
 mkdir $d
 
 $VG virt-v2v --debug-gc \
-    -i libvirt -ic "$libvirt_uri" windows \
+    -i libvirtxml test-v2v-print-source.xml \
     -o local -os $d \
     --print-source > $d/output
 
@@ -40,27 +37,10 @@ mv $d/output $d/output.orig
 < $d/output.orig \
 grep -v 'Opening the source' |
 grep -v 'Source guest information' |
-sed -e 's,/.*/,/,' |
+sed -e 's,/.*/windows.img,windows.img,' |
 grep -v '^$' \
 > $d/output
 
-if [ "$(cat $d/output)" != "    source name: windows
-hypervisor type: test
-         memory: 1073741824 (bytes)
-       nr vCPUs: 1
-   CPU features: 
-       firmware: unknown
-        display: 
-          video: qxl
-          sound: 
-disks:
-	/windows.img (raw) [virtio-blk]
-removable media:
-NICs:
-	Network \"default\" mac: 00:11:22:33:44:55 [virtio]" ]; then
-    echo "$0: unexpected output from test:"
-    cat $d/output.orig
-    exit 1
-fi
+diff -u $d/output test-v2v-print-source.expected
 
 rm -r $d
diff --git a/v2v/test-v2v-print-source.xml b/v2v/test-v2v-print-source.xml
new file mode 100644
index 0000000..0667f2e
--- /dev/null
+++ b/v2v/test-v2v-print-source.xml
@@ -0,0 +1,23 @@
+<domain type='kvm'>
+  <name>windows</name>
+  <memory>1048576</memory>
+  <os>
+    <type>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <devices>
+    <disk type='file' device='disk'>
+      <driver name='qemu' type='raw'/>
+      <source file='../test-data/phony-guests/windows.img'/>
+      <target dev='vda' bus='virtio'/>
+    </disk>
+    <interface type='network'>
+      <mac address='00:11:22:33:44:55'/>
+      <source network='default'/>
+      <model type='virtio'/>
+    </interface>
+    <video>
+      <model type='qxl' ram='65536' vram='65536'
vgamem='16384' heads='1'/>
+    </video>
+  </devices>
+</domain>
-- 
2.10.2

Just refactoring, no change in semantics.
---
 v2v/create_libvirt_xml.ml | 49 +++++++++++++++++++++++++++++++----------------
 1 file changed, 32 insertions(+), 17 deletions(-)

diff --git a/v2v/create_libvirt_xml.ml b/v2v/create_libvirt_xml.ml
index de3ff39..7830bc3 100644
--- a/v2v/create_libvirt_xml.ml
+++ b/v2v/create_libvirt_xml.ml
@@ -32,6 +32,21 @@ let string_set_of_list  
 let create_libvirt_xml ?pool source target_buses guestcaps
                        target_features target_firmware +  (* The main body of
the libvirt XML document. *)
+  let body = ref [] in
+
+  append body [
+    Comment generated_by;
+    e "name" [] [PCData source.s_name];
+  ];
+
+  let memory_k = source.s_memory /^ 1024L in
+  append body [
+    e "memory" ["unit", "KiB"] [PCData
(Int64.to_string memory_k)];
+    e "currentMemory" ["unit", "KiB"] [PCData
(Int64.to_string memory_k)];
+    e "vcpu" [] [PCData (string_of_int source.s_vcpu)]
+  ];
+
   let uefi_firmware      match target_firmware with
     | TargetBIOS -> None
@@ -47,8 +62,6 @@ let create_libvirt_xml ?pool source target_buses guestcaps
   let machine_q35 = secure_boot_required in
   let smm = secure_boot_required in
 
-  let memory_k = source.s_memory /^ 1024L in
-
   (* We have the machine features of the guest when it was on the
    * source hypervisor (source.s_features).  We have the acpi flag
    * which tells us whether acpi is required by this guest
@@ -91,6 +104,10 @@ let create_libvirt_xml ?pool source target_buses guestcaps
 
   let features = List.sort compare (StringSet.elements features) in
 
+  append body [
+    e "features" [] (List.map (fun s -> e s [] []) features);
+  ];
+
   (* The <os> section subelements. *)
   let os_section      let machine = if machine_q35 then [ "machine",
"q35" ] else [] in
@@ -108,6 +125,14 @@ let create_libvirt_xml ?pool source target_buses guestcaps
     (e "type" (["arch", guestcaps.gcaps_arch] @ machine)
[PCData "hvm"])
     :: loader in
 
+  append body [
+    e "os" [] os_section;
+
+    e "on_poweroff" [] [PCData "destroy"];
+    e "on_reboot" [] [PCData "restart"];
+    e "on_crash" [] [PCData "restart"];
+  ];
+
   (* The devices. *)
   let devices = ref [] in
 
@@ -284,23 +309,13 @@ let create_libvirt_xml ?pool source target_buses guestcaps
     e "console" ["type", "pty"] [];
   ];
 
+  append body [
+    e "devices" [] !devices;
+  ];
+
   let doc : doc      doc "domain" [
       "type", "kvm";                (* Always assume target
is kvm? *)
-    ] [
-      Comment generated_by;
-      e "name" [] [PCData source.s_name];
-      e "memory" ["unit", "KiB"] [PCData
(Int64.to_string memory_k)];
-      e "currentMemory" ["unit", "KiB"] [PCData
(Int64.to_string memory_k)];
-      e "vcpu" [] [PCData (string_of_int source.s_vcpu)];
-      e "os" [] os_section;
-      e "features" [] (List.map (fun s -> e s [] []) features);
-
-      e "on_poweroff" [] [PCData "destroy"];
-      e "on_reboot" [] [PCData "restart"];
-      e "on_crash" [] [PCData "restart"];
-
-      e "devices" [] !devices;
-    ] in
+    ] !body in
 
   doc
-- 
2.10.2

Where supported, pass the source CPU vendor, model and topology to the
target hypervisor.

For -i ova, we can get just cores per socket via a proprietary VMware
extension to OVF.

For -i libvirt and from virt-p2v, we can get all of these fields from
the libvirt XML.

For -o libvirt/local, we can preserve all of the information in the
target XML.

For -o glance, as far as I can tell from the documentation, Glance
does not support anything like this.

For -o rhv/vdsm, it looks from the code like this could be supported,
but I could not work out how to enable it in the OVF.

For -o qemu, we preserve the topology only because versions of qemu
vary widely in their support for CPU models.
---
 v2v/create_libvirt_xml.ml              | 36 ++++++++++++++++++++++++++++++++++
 v2v/input_disk.ml                      |  5 +++++
 v2v/input_ova.ml                       |  8 +++++++-
 v2v/output_qemu.ml                     | 24 +++++++++++++++++++++--
 v2v/parse_libvirt_xml.ml               | 11 +++++++++++
 v2v/parse_ovf_from_ova.ml              | 22 ++++++++++++++++++++-
 v2v/parse_ovf_from_ova.mli             |  5 +++--
 v2v/test-v2v-i-ova-formats.expected    |  3 +++
 v2v/test-v2v-i-ova-gz.expected         |  3 +++
 v2v/test-v2v-i-ova-subfolders.expected |  3 +++
 v2v/test-v2v-i-ova-tar.expected        |  3 +++
 v2v/test-v2v-i-ova-two-disks.expected  |  3 +++
 v2v/test-v2v-print-source.expected     |  5 ++++-
 v2v/test-v2v-print-source.xml          | 10 ++++++++++
 v2v/types.ml                           | 13 ++++++++++++
 v2v/types.mli                          |  5 +++++
 v2v/v2v.ml                             | 27 +++++++++++++++++++++++++
 17 files changed, 179 insertions(+), 7 deletions(-)

diff --git a/v2v/create_libvirt_xml.ml b/v2v/create_libvirt_xml.ml
index 7830bc3..fc71965 100644
--- a/v2v/create_libvirt_xml.ml
+++ b/v2v/create_libvirt_xml.ml
@@ -47,6 +47,42 @@ let create_libvirt_xml ?pool source target_buses guestcaps
     e "vcpu" [] [PCData (string_of_int source.s_vcpu)]
   ];
 
+  if source.s_cpu_vendor <> None || source.s_cpu_model <> None ||
+     source.s_cpu_sockets <> None || source.s_cpu_cores <> None ||
+     source.s_cpu_threads <> None then (
+    let cpu = ref [] in
+
+    (match source.s_cpu_vendor with
+     | None -> ()
+     | Some vendor ->
+        push_back cpu (e "vendor" [] [PCData vendor])
+    );
+    (match source.s_cpu_model with
+     | None -> ()
+     | Some model ->
+        push_back cpu (e "model" ["fallback",
"allow"] [PCData model])
+    );
+    if source.s_cpu_sockets <> None || source.s_cpu_cores <> None
||
+       source.s_cpu_threads <> None then (
+      let topology_attrs = ref [] in
+      (match source.s_cpu_sockets with
+       | None -> ()
+       | Some v -> push_back topology_attrs ("sockets",
string_of_int v)
+      );
+      (match source.s_cpu_cores with
+       | None -> ()
+       | Some v -> push_back topology_attrs ("cores",
string_of_int v)
+      );
+      (match source.s_cpu_threads with
+       | None -> ()
+       | Some v -> push_back topology_attrs ("threads",
string_of_int v)
+      );
+      push_back cpu (e "topology" !topology_attrs [])
+    );
+
+    append body [ e "cpu" [ "match", "minimum" ]
!cpu ]
+  );
+
   let uefi_firmware      match target_firmware with
     | TargetBIOS -> None
diff --git a/v2v/input_disk.ml b/v2v/input_disk.ml
index 27f8553..d28f45e 100644
--- a/v2v/input_disk.ml
+++ b/v2v/input_disk.ml
@@ -80,6 +80,11 @@ class input_disk input_format disk = object
       s_name = name; s_orig_name = name;
       s_memory = 2048L *^ 1024L *^ 1024L; (* 2048 MB *)
       s_vcpu = 1;                         (* 1 vCPU is a safe default *)
+      s_cpu_vendor = None;
+      s_cpu_model = None;
+      s_cpu_sockets = None;
+      s_cpu_cores = None;
+      s_cpu_threads = None;
       s_features = [ "acpi"; "apic"; "pae" ];
       s_firmware = UnknownFirmware;       (* causes virt-v2v to autodetect *)
       s_display diff --git a/v2v/input_ova.ml b/v2v/input_ova.ml
index e80ec82..b82862f 100644
--- a/v2v/input_ova.ml
+++ b/v2v/input_ova.ml
@@ -222,7 +222,8 @@ object
     let ovf_folder = Filename.dirname ovf in
 
     (* Parse the ovf file. *)
-    let name, memory, vcpu, firmware, disks, removables, nics +    let name,
memory, vcpu, cpu_sockets, cpu_cores, firmware,
+        disks, removables, nics        parse_ovf_from_ova ovf in
 
     let name @@ -314,6 +315,11 @@ object
       s_orig_name = name;
       s_memory = memory;
       s_vcpu = vcpu;
+      s_cpu_vendor = None;
+      s_cpu_model = None;
+      s_cpu_sockets = cpu_sockets;
+      s_cpu_cores = cpu_cores;
+      s_cpu_threads = None; (* XXX *)
       s_features = []; (* XXX *)
       s_firmware = firmware;
       s_display = None; (* XXX *)
diff --git a/v2v/output_qemu.ml b/v2v/output_qemu.ml
index 84efd45..b3115f9 100644
--- a/v2v/output_qemu.ml
+++ b/v2v/output_qemu.ml
@@ -96,8 +96,28 @@ object
     );
 
     arg "-m" (Int64.to_string (source.s_memory /^ 1024L /^ 1024L));
-    if source.s_vcpu > 1 then
-      arg "-smp" (string_of_int source.s_vcpu);
+    if source.s_vcpu > 1 then (
+      if source.s_cpu_sockets <> None || source.s_cpu_cores <> None
||
+         source.s_cpu_threads <> None then (
+        let a = ref [] in
+        push_back a (sprintf "cpus=%d" source.s_vcpu);
+        (match source.s_cpu_sockets with
+         | None -> ()
+         | Some v -> push_back a (sprintf "sockets=%d" v)
+        );
+        (match source.s_cpu_cores with
+         | None -> ()
+         | Some v -> push_back a (sprintf "cores=%d" v)
+        );
+        (match source.s_cpu_threads with
+         | None -> ()
+         | Some v -> push_back a (sprintf "threads=%d" v)
+        );
+        commas "-smp" !a
+      )
+      else
+        arg "-smp" (string_of_int source.s_vcpu);
+    );
 
     let make_disk if_name i = function
     | BusSlotEmpty -> ()
diff --git a/v2v/parse_libvirt_xml.ml b/v2v/parse_libvirt_xml.ml
index edffd20..6032c31 100644
--- a/v2v/parse_libvirt_xml.ml
+++ b/v2v/parse_libvirt_xml.ml
@@ -67,6 +67,12 @@ let parse_libvirt_xml ?conn xml    let memory = memory *^
1024L in
   let vcpu = xpath_int_default "/domain/vcpu/text()" 1 in
 
+  let cpu_vendor = xpath_string "/domain/cpu/vendor/text()" in
+  let cpu_model = xpath_string "/domain/cpu/model/text()" in
+  let cpu_sockets = xpath_int "/domain/cpu/topology/@sockets" in
+  let cpu_cores = xpath_int "/domain/cpu/topology/@cores" in
+  let cpu_threads = xpath_int "/domain/cpu/topology/@threads" in
+
   let features      let features = ref [] in
     let obj = Xml.xpath_eval_expression xpathctx "/domain/features/*"
in
@@ -410,6 +416,11 @@ let parse_libvirt_xml ?conn xml      s_name = name;
s_orig_name = name;
     s_memory = memory;
     s_vcpu = vcpu;
+    s_cpu_vendor = cpu_vendor;
+    s_cpu_model = cpu_model;
+    s_cpu_sockets = cpu_sockets;
+    s_cpu_cores = cpu_cores;
+    s_cpu_threads = cpu_threads;
     s_features = features;
     s_firmware = UnknownFirmware; (* XXX until RHBZ#1217444 is fixed *)
     s_display = display;
diff --git a/v2v/parse_ovf_from_ova.ml b/v2v/parse_ovf_from_ova.ml
index 989483e..2a37527 100644
--- a/v2v/parse_ovf_from_ova.ml
+++ b/v2v/parse_ovf_from_ova.ml
@@ -69,6 +69,26 @@ let parse_ovf_from_ova ovf_filename      (* Search for number
of vCPUs. *)
     let vcpu = xpath_int_default
"/ovf:Envelope/ovf:VirtualSystem/ovf:VirtualHardwareSection/ovf:Item[rasd:ResourceType/text()=3]/rasd:VirtualQuantity/text()"
1 in
 
+    (* CPU topology.  coresPerSocket is a VMware proprietary extension.
+     * I couldn't find out how hyperthreads is specified in the OVF.
+     *)
+    let cores_per_socket = xpath_int
"/ovf:Envelope/ovf:VirtualSystem/ovf:VirtualHardwareSection/ovf:Item[rasd:ResourceType/text()=3]/vmw:CoresPerSocket/text()"
in
+    let cpu_sockets, cpu_cores +      match cores_per_socket with
+      | None -> None, None
+      | Some cores_per_socket when cores_per_socket <= 0 ->
+         warning (f_"invalid vmw:CoresPerSocket (%d) ignored")
+                 cores_per_socket;
+         None, None
+      | Some cores_per_socket ->
+         let sockets = vcpu / cores_per_socket in
+         if sockets <= 0 then (
+           warning (f_"invalid vmw:CoresPerSocket < number of
cores");
+           None, None
+         )
+         else
+           Some sockets, Some cores_per_socket in
+
     (* BIOS or EFI firmware? *)
     let firmware = xpath_string_default
"/ovf:Envelope/ovf:VirtualSystem/ovf:VirtualHardwareSection/vmw:Config[@vmw:key=\"firmware\"]/@vmw:value"
"bios" in
     let firmware @@ -78,7 +98,7 @@ let parse_ovf_from_ova ovf_filename        |
s ->
          error (f_"unknown Config:firmware value %s (expected
\"bios\" or \"efi\")") s in
 
-    name, memory, vcpu, firmware,
+    name, memory, vcpu, cpu_sockets, cpu_cores, firmware,
     parse_disks (), parse_removables (), parse_nics ()
 
   (* Helper function to return the parent controller of a disk. *)
diff --git a/v2v/parse_ovf_from_ova.mli b/v2v/parse_ovf_from_ova.mli
index 3f60abc..54cdcf2 100644
--- a/v2v/parse_ovf_from_ova.mli
+++ b/v2v/parse_ovf_from_ova.mli
@@ -29,8 +29,9 @@ type ovf_disk = {
 }
 (** A VMDK disk from a parsed OVF. *)
 
-val parse_ovf_from_ova : string -> string option * int64 * int *
Types.source_firmware * ovf_disk list * Types.source_removable list *
Types.source_nic list
+val parse_ovf_from_ova : string -> string option * int64 * int * int option
* int option * Types.source_firmware * ovf_disk list * Types.source_removable
list * Types.source_nic list
 (** Parse an OVF file.
 
     The returned tuple is
-    [name, memory, vcpu, firmware, disks, removables, nics] *)
+    [name, memory, vcpu, cpu_sockets, cpu_cores, firmware,
+    disks, removables, nics] *)
diff --git a/v2v/test-v2v-i-ova-formats.expected
b/v2v/test-v2v-i-ova-formats.expected
index 7049aee..11b24e0 100644
--- a/v2v/test-v2v-i-ova-formats.expected
+++ b/v2v/test-v2v-i-ova-formats.expected
@@ -4,6 +4,9 @@ Source guest information (--print-source option):
 hypervisor type: vmware
          memory: 1073741824 (bytes)
        nr vCPUs: 1
+     CPU vendor: 
+      CPU model: 
+   CPU topology: sockets: - cores/socket: - threads/core: -
    CPU features: 
        firmware: uefi
         display: 
diff --git a/v2v/test-v2v-i-ova-gz.expected b/v2v/test-v2v-i-ova-gz.expected
index 50ba746..11db2a3 100644
--- a/v2v/test-v2v-i-ova-gz.expected
+++ b/v2v/test-v2v-i-ova-gz.expected
@@ -4,6 +4,9 @@ Source guest information (--print-source option):
 hypervisor type: vmware
          memory: 1073741824 (bytes)
        nr vCPUs: 1
+     CPU vendor: 
+      CPU model: 
+   CPU topology: sockets: - cores/socket: - threads/core: -
    CPU features: 
        firmware: bios
         display: 
diff --git a/v2v/test-v2v-i-ova-subfolders.expected
b/v2v/test-v2v-i-ova-subfolders.expected
index b6fdb07..4ef8b4b 100644
--- a/v2v/test-v2v-i-ova-subfolders.expected
+++ b/v2v/test-v2v-i-ova-subfolders.expected
@@ -4,6 +4,9 @@ Source guest information (--print-source option):
 hypervisor type: vmware
          memory: 1073741824 (bytes)
        nr vCPUs: 1
+     CPU vendor: 
+      CPU model: 
+   CPU topology: sockets: - cores/socket: - threads/core: -
    CPU features: 
        firmware: uefi
         display: 
diff --git a/v2v/test-v2v-i-ova-tar.expected b/v2v/test-v2v-i-ova-tar.expected
index 7049aee..11b24e0 100644
--- a/v2v/test-v2v-i-ova-tar.expected
+++ b/v2v/test-v2v-i-ova-tar.expected
@@ -4,6 +4,9 @@ Source guest information (--print-source option):
 hypervisor type: vmware
          memory: 1073741824 (bytes)
        nr vCPUs: 1
+     CPU vendor: 
+      CPU model: 
+   CPU topology: sockets: - cores/socket: - threads/core: -
    CPU features: 
        firmware: uefi
         display: 
diff --git a/v2v/test-v2v-i-ova-two-disks.expected
b/v2v/test-v2v-i-ova-two-disks.expected
index cc850a7..b0bb3ef 100644
--- a/v2v/test-v2v-i-ova-two-disks.expected
+++ b/v2v/test-v2v-i-ova-two-disks.expected
@@ -4,6 +4,9 @@ Source guest information (--print-source option):
 hypervisor type: vmware
          memory: 1073741824 (bytes)
        nr vCPUs: 1
+     CPU vendor: 
+      CPU model: 
+   CPU topology: sockets: - cores/socket: - threads/core: -
    CPU features: 
        firmware: bios
         display: 
diff --git a/v2v/test-v2v-print-source.expected
b/v2v/test-v2v-print-source.expected
index b947927..6e78aad 100644
--- a/v2v/test-v2v-print-source.expected
+++ b/v2v/test-v2v-print-source.expected
@@ -2,7 +2,10 @@
 hypervisor type: kvm
          memory: 1073741824 (bytes)
        nr vCPUs: 1
-   CPU features: 
+     CPU vendor: Intel
+      CPU model: Broadwell
+   CPU topology: sockets: 4 cores/socket: 8 threads/core: 2
+   CPU features: pae,apic,acpi
        firmware: unknown
         display: 
           video: qxl
diff --git a/v2v/test-v2v-print-source.xml b/v2v/test-v2v-print-source.xml
index 0667f2e..3768caf 100644
--- a/v2v/test-v2v-print-source.xml
+++ b/v2v/test-v2v-print-source.xml
@@ -1,6 +1,16 @@
 <domain type='kvm'>
   <name>windows</name>
   <memory>1048576</memory>
+  <cpu match="minimum">
+    <vendor>Intel</vendor>
+    <model fallback="allow">Broadwell</model>
+    <topology sockets="4" cores="8"
threads="2"/>
+  </cpu>
+  <features>
+    <acpi/>
+    <apic/>
+    <pae/>
+  </features>
   <os>
     <type>hvm</type>
     <boot dev='hd'/>
diff --git a/v2v/types.ml b/v2v/types.ml
index d802e19..31cbbd2 100644
--- a/v2v/types.ml
+++ b/v2v/types.ml
@@ -29,6 +29,11 @@ type source = {
   s_orig_name : string;
   s_memory : int64;
   s_vcpu : int;
+  s_cpu_vendor : string option;
+  s_cpu_model : string option;
+  s_cpu_sockets : int option;
+  s_cpu_cores : int option;
+  s_cpu_threads : int option;
   s_features : string list;
   s_firmware : source_firmware;
   s_display : source_display option;
@@ -102,6 +107,9 @@ let rec string_of_source s  hypervisor type: %s
          memory: %Ld (bytes)
        nr vCPUs: %d
+     CPU vendor: %s
+      CPU model: %s
+   CPU topology: sockets: %s cores/socket: %s threads/core: %s
    CPU features: %s
        firmware: %s
         display: %s
@@ -118,6 +126,11 @@ NICs:
     (string_of_source_hypervisor s.s_hypervisor)
     s.s_memory
     s.s_vcpu
+    (match s.s_cpu_vendor with None -> "" | Some v -> v)
+    (match s.s_cpu_model with None -> "" | Some v -> v)
+    (match s.s_cpu_sockets with None -> "-" | Some v ->
string_of_int v)
+    (match s.s_cpu_cores with None -> "-" | Some v ->
string_of_int v)
+    (match s.s_cpu_threads with None -> "-" | Some v ->
string_of_int v)
     (String.concat "," s.s_features)
     (string_of_source_firmware s.s_firmware)
     (match s.s_display with
diff --git a/v2v/types.mli b/v2v/types.mli
index 31a974a..c902b7a 100644
--- a/v2v/types.mli
+++ b/v2v/types.mli
@@ -64,6 +64,11 @@ type source = {
                                             still saved here). *)
   s_memory : int64;                     (** Memory size (bytes). *)
   s_vcpu : int;                         (** Number of CPUs. *)
+  s_cpu_vendor : string option;         (** Source CPU vendor. *)
+  s_cpu_model : string option;          (** Source CPU model. *)
+  s_cpu_sockets : int option;           (** Number of sockets. *)
+  s_cpu_cores : int option;             (** Number of cores per socket. *)
+  s_cpu_threads : int option;           (** Number of threads per core. *)
   s_features : string list;             (** Machine features. *)
   s_firmware : source_firmware;         (** Firmware (BIOS or EFI). *)
   s_display : source_display option;    (** Guest display. *)
diff --git a/v2v/v2v.ml b/v2v/v2v.ml
index 551524d..bd3a413 100644
--- a/v2v/v2v.ml
+++ b/v2v/v2v.ml
@@ -181,7 +181,34 @@ and open_source cmdline input  
   assert (source.s_name <> "");
   assert (source.s_memory > 0L);
+
   assert (source.s_vcpu >= 1);
+  assert (source.s_cpu_vendor <> Some "");
+  assert (source.s_cpu_model <> Some "");
+  (match source.s_cpu_sockets with
+   | None -> ()
+   | Some i when i > 0 -> ()
+   | _ -> assert false);
+  (match source.s_cpu_cores with
+   | None -> ()
+   | Some i when i > 0 -> ()
+   | _ -> assert false);
+  (match source.s_cpu_threads with
+   | None -> ()
+   | Some i when i > 0 -> ()
+   | _ -> assert false);
+  (match source.s_cpu_sockets, source.s_cpu_cores, source.s_cpu_threads with
+   | None, None, None -> () (* no topology specified *)
+   | sockets, cores, threads ->
+      let sockets = match sockets with None -> 1 | Some v -> v in
+      let cores = match cores with None -> 1 | Some v -> v in
+      let threads = match threads with None -> 1 | Some v -> v in
+      let expected_vcpu = sockets * cores * threads in
+      if expected_vcpu <> source.s_vcpu then
+        warning (f_"source sockets * cores * threads <> number of
vCPUs.\nSockets %d * cores per socket %d * threads %d = %d, but number of vCPUs
= %d.\n\nThis is a problem with either the source metadata or the virt-v2v input
module.  In some circumstances this could stop the guest from booting on the
target.")
+                sockets cores threads expected_vcpu source.s_vcpu
+  );
+
   if source.s_disks = [] then
     error (f_"source has no hard disks!");
   List.iter (
-- 
2.10.2

On Thursday, 16 March 2017 19:57:12 CET Richard W.M. Jones
wrote:
> In the fake <domain type='physical'> libvirt XML that we
create to
> describe the physical host, we did not accurately pass any information
> about the host CPU except the number of cores (<vcpu/>).
> 
> This commit extracts detailed information about the vendor, model and
> topology of the host CPU and adds that to the libvirt XML for
> virt-v2v.  Conveniently we can use libvirt capabilities to get this
> information without needing to parse /proc/cpuinfo or similar
> techniques.
> 
> The libvirt XML looks like this:
> 
>   <domain type="physical">
>   ...
>     <cpu match="minimum">
>       <vendor>Intel</vendor>
>       <model fallback="allow">Broadwell</model>
>       <topology sockets="1" cores="2"
threads="2"/>
>     </cpu>
>   ...
>     <features>
>       <acpi/>
>       <apic/>
>       <pae/>
>     </features>
> ---
Mostly LGTM, two notes below.
> +  /* Get the CPU vendor. */
> +  xpathObj > +    xmlXPathEvalExpression (BAD_CAST
"/capabilities/host/cpu/vendor/text()",
> +                            xpathCtx);
> +  if (xpathObj == NULL) {
> +    fprintf (stderr, _("%s: %s: %d: unable to evaluate xpath
expression\n"),
> +             getprogname (), __FILE__, __LINE__);
More than __FILE__ and __LINE__ (which can change, and requires you to
inspect the patched sources to find out the exact location), I'd just
print the failed XPath expression.  (Same in the two below.)
> diff --git a/p2v/p2v.h b/p2v/p2v.h
> index 5223aa2..69ed35c 100644
> --- a/p2v/p2v.h
> +++ b/p2v/p2v.h
> @@ -59,6 +59,17 @@ extern int feature_colours_option;
>  extern int force_colour;
>  
>  /* config.c */
> +struct cpu_config {
> +  char *vendor;                 /* eg. "Intel" */
> +  char *model;                  /* eg. "Broadwell" */
> +  unsigned sockets;             /* number of sockets */
> +  unsigned cores;               /* number of cores per socket */
> +  unsigned threads;             /* number of hyperthreads per core */
> +  int acpi;
> +  int apic;
> +  int pae;
bool for the above three, so it's clear they are single switches and not
flags/bitfields.

Thanks,
-- 
Pino Toscano

On Thursday, 16 March 2017 19:57:15 CET Richard W.M. Jones
wrote:
> Where supported, pass the source CPU vendor, model and topology to the
> target hypervisor.
> 
> For -i ova, we can get just cores per socket via a proprietary VMware
> extension to OVF.
> 
> For -i libvirt and from virt-p2v, we can get all of these fields from
> the libvirt XML.
> 
> For -o libvirt/local, we can preserve all of the information in the
> target XML.
> 
> For -o glance, as far as I can tell from the documentation, Glance
> does not support anything like this.
It looks like there are properties for this:
https://docs.openstack.org/cli-reference/glance-property-keys.html
see hw_cpu_sockets, hw_cpu_cores, hw_cpu_threads, and hw_machine_type.

The rest of the patch LGTM, just a pity OCaml does not have unsigned
types in the stdlib...

Thanks,
-- 
Pino Toscano

On Thursday, 16 March 2017 19:57:11 CET Richard W.M. Jones
wrote:
> This is tangentially related to:
> 
>   https://bugzilla.redhat.com/show_bug.cgi?id=1372668
> 
> The problem in that bug is that we didn't pass the source CPU model
> (Sandybridge in that case) through to the target RHV hypervisor.  So
> when the Windows guest booted on the target it gives an error about
> CPU hardware being disconnected (although it otherwise boots and works
> fine).
> 
> This patch series explores sending the CPU vendor, model and topology
> from the source to the target, where supported.  It turns out that RHV
> doesn't seem to support this anyway, but other target hypervisors do.
Except where noted, LGTM.

Thanks,
-- 
Pino Toscano