[libvirt] [PATCH v3 3/3] qemu: Enable NUMA node tag in pci-root for PPC64

[Andrea Bolognani]

On Fri, 2017-07-21 at 13:33 +0530, Shivaprasad G Bhat wrote:
[...]
> +++ b/docs/formatdomain.html.in
> @@ -3778,7 +3778,9 @@
>        </dd>
>        <dt><code>node</code></dt>
>        <dd>
> -        pci-expander-bus controllers can have an
> +        Some PCI controllers (pci-expander-bus for the pc machine
> +        type, pcie-expander-bus for the q35 machine type and
> +        pci-root for the pseries machine type) can have an

Controller names could have been wrapped in <code> tags
for nicer formatting. A "Since" notice would also have
been nice.

[...]
> +++ b/src/conf/domain_conf.c
> @@ -9457,8 +9457,15 @@ virDomainControllerDefParseXML(xmlNodePtr node,
>                  goto error;
>              }
>          }
> -        if (numaNode >= 0)
> +        if (numaNode >= 0) {
>              def->opts.pciopts.numaNode = numaNode;
> +            if (def->idx == 0) {
> +                virReportError(VIR_ERR_XML_ERROR, "%s",
> +                               _("The PCI controller with index=0 can't "
> +                                 "be associated with a NUMA node."));
> +                goto error;
> +            }
> +        }

The check should be *before* setting the value. Not that
it matters from a functional point of view, since you're
going to jump either way, but it looks nicer ;)

[...]
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-phb-numa-node.xml
> @@ -0,0 +1,41 @@
> +<domain type='qemu'>
> +  <name>QEMUGuest1</name>
> +  <uuid>87eedafe-eedc-4336-8130-ed9fe5dc90c8</uuid>
> +  <memory unit='KiB'>2097152</memory>
> +  <vcpu placement='static'>8</vcpu>

The number of vCPUs here...

> +  <numatune>
> +    <memnode cellid="0" mode="strict" nodeset="1"/>
> +    <memnode cellid="1" mode="strict" nodeset="2"/>
> +  </numatune>
> +  <cpu>
> +    <topology sockets='3' cores='1' threads='8'/>

... doesn't match the topology here.

The test case won't fail because you don't enable the
QEMU_CAPS_QUERY_HOTPLUGGABLE_CPUS capability, but it's
confusing to have it there.

I'll fix up these small issues, add

  Reviewed-by: Andrea Bolognani <abologna at redhat.com>

and push.

-- 
Andrea Bolognani / Red Hat / Virtualization