[Libguestfs] [PATCH 1/4] p2v: Pass host CPU details to virt-v2v.

[Pino Toscano]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20170317/c194c866/attachment.sig>