Re: [libvirt] [PATCH v3 3/3] qemu: Enable NUMA node tag in pci-root for PPC64
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(a)redhat.com>
and push.
--
Andrea Bolognani / Red Hat / Virtualization