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@redhat.com> and push. -- Andrea Bolognani / Red Hat / Virtualization