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@xxxxxxxxxx>
and push.
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
