On 10/12/2017 01:31 PM, Wim Ten Have wrote:

From: Wim ten Have <wim.ten.have@oracle.com>

Add support for describing NUMA distances in a domain's <numa> <cell> XML description.

Below is an example of a 4 node setup:

<cpu> <numa> <cell id='0' cpus='0-3' memory='2097152' unit='KiB'> <distances> <sibling id='0' value='10'/> <sibling id='1' value='21'/> <sibling id='2' value='31'/> <sibling id='3' value='21'/> </distances> </cell> <cell id='1' cpus='4-7' memory='2097152' unit='KiB'> <distances> <sibling id='0' value='21'/> <sibling id='1' value='10'/> <sibling id='2' value='21'/> <sibling id='3' value='31'/> </distances> </cell> <cell id='2' cpus='8-11' memory='2097152' unit='KiB'> <distances> <sibling id='0' value='31'/> <sibling id='1' value='21'/> <sibling id='2' value='10'/> <sibling id='3' value='21'/> </distances> <cell id='3' cpus='12-15' memory='2097152' unit='KiB'> <distances> <sibling id='0' value='21'/> <sibling id='1' value='31'/> <sibling id='2' value='21'/> <sibling id='3' value='10'/> </distances> </cell> </numa> </cpu>

A <cell> defines a NUMA node. <distances> describes the NUMA distance from the <cell> to the other NUMA nodes (the <sibling>s). For example, in above XML description, the distance between NUMA node0 <cell id='0' ...> and NUMA node2 <sibling id='2' ...> is 31.

Valid distance value are '10 <= value <= 255'. A distance value of 10 represents the distance to the node itself. A distance value of 20 represents the default value for remote nodes but other values are possible depending on the physical topology of the system.

When distances are not fully described, any missing sibling distance values will default to 10 for local nodes and 20 for remote nodes.

Signed-off-by: Wim ten Have <wim.ten.have@oracle.com> --- Changes on v1: - Add changes to docs/formatdomain.html.in describing schema update. Changes on v2: - Automatically apply distance symmetry maintaining cell <-> sibling. - Check for maximum '255' on numaDistanceValue. - Automatically complete empty distance ranges. - Check that sibling_id's are in range with cell identifiers. - Allow non-contiguous ranges, starting from any node id. - Respect parameters as ATTRIBUTE_NONNULL fix functions and callers. - Add and apply topology for LOCAL_DISTANCE=10 and REMOTE_DISTANCE=20. Changes on v3 - Add UNREACHABLE if one locality is unreachable from another. - Add code cleanup aligning function naming in a separated patch. - Add numa related driver code in a separated patch. - Remove <choice> from numaDistanceValue schema/basictypes.rng - Correct doc changes. Changes on v4 - Fix symmetry error under virDomainNumaDefNodeDistanceParseXML() --- docs/formatdomain.html.in | 63 ++++++++++++- docs/schemas/basictypes.rng | 7 ++ docs/schemas/cputypes.rng | 18 ++++ src/conf/numa_conf.c | 221 +++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 305 insertions(+), 4 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index c0e3c2221..ab2a89c6a 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1529,7 +1529,68 @@ </p>

<p> - This guest NUMA specification is currently available only for QEMU/KVM. + This guest NUMA specification is currently available only for + QEMU/KVM and Xen. Whereas Xen driver also allows for a distinct + description of NUMA arranged <code>sibling</code> <code>cell</code> + <code>distances</code> <span class="since">Since 3.8.0</span>.

Now 3.9.0.

+ </p> + + <p> + Under NUMA h/w architecture, distinct resources such as memory

For the website docs, I think it is better to write "hardware" instead of "h/w".

+ create a designated distance between <code>cell</code> and + <code>siblings</code> that now can be described with the help of + <code>distances</code>. A detailed description can be found within + the ACPI (Advanced Configuration and Power Interface Specification) + within the chapter explaining the system's SLIT (System Locality + Distance Information Table). + </p> + +<pre> +... +<cpu> + ... + <numa> + <cell id='0' cpus='0,4-7' memory='512000' unit='KiB'> + <distances> + <sibling id='0' value='10'/> + <sibling id='1' value='21'/> + <sibling id='2' value='31'/> + <sibling id='3' value='41'/> + </distances> + </cell> + <cell id='1' cpus='1,8-10,12-15' memory='512000' unit='KiB' memAccess='shared'> + <distances> + <sibling id='0' value='21'/> + <sibling id='1' value='10'/> + <sibling id='2' value='21'/> + <sibling id='3' value='31'/> + </distances> + </cell> + <cell id='2' cpus='2,11' memory='512000' unit='KiB' memAccess='shared'> + <distances> + <sibling id='0' value='31'/> + <sibling id='1' value='21'/> + <sibling id='2' value='10'/> + <sibling id='3' value='21'/> + </distances> + </cell> + <cell id='3' cpus='3' memory='512000' unit='KiB'> + <distances> + <sibling id='0' value='41'/> + <sibling id='1' value='31'/> + <sibling id='2' value='21'/> + <sibling id='3' value='10'/> + </distances> + </cell> + </numa> + ... +</cpu> +...</pre> + + <p> + Under Xen driver, if no <code>distances</code> are given to describe + the SLIT data between different cells, it will default to a scheme + using 10 for local and 20 for remote distances. </p>

<h3><a id="elementsEvents">Events configuration</a></h3> diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng index 1ea667cdf..1a18cd31b 100644 --- a/docs/schemas/basictypes.rng +++ b/docs/schemas/basictypes.rng @@ -77,6 +77,13 @@ </choice> </define>

+ <define name="numaDistanceValue"> + <data type="unsignedInt"> + <param name="minInclusive">10</param> + <param name="maxInclusive">255</param> + </data> + </define> + <define name="pciaddress"> <optional> <attribute name="domain"> diff --git a/docs/schemas/cputypes.rng b/docs/schemas/cputypes.rng index 3eef16abc..c45b6dfb2 100644 --- a/docs/schemas/cputypes.rng +++ b/docs/schemas/cputypes.rng @@ -129,6 +129,24 @@ </choice> </attribute> </optional> + <optional> + <element name="distances"> + <oneOrMore> + <ref name="numaDistance"/> + </oneOrMore> + </element> + </optional> + </element> + </define> + + <define name="numaDistance"> + <element name="sibling"> + <attribute name="id"> + <ref name="unsignedInt"/> + </attribute> + <attribute name="value"> + <ref name="numaDistanceValue"/> + </attribute> </element> </define>

diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c index b71dc012c..00a3343fb 100644 --- a/src/conf/numa_conf.c +++ b/src/conf/numa_conf.c @@ -29,6 +29,15 @@ #include "virnuma.h" #include "virstring.h"

+/* + * Distance definitions defined Conform ACPI 2.0 SLIT. + * See include/linux/topology.h + */ +#define LOCAL_DISTANCE 10 +#define REMOTE_DISTANCE 20 +/* SLIT entry value is a one-byte unsigned integer. */ +#define UNREACHABLE 255 + #define VIR_FROM_THIS VIR_FROM_DOMAIN

VIR_ENUM_IMPL(virDomainNumatuneMemMode, @@ -48,6 +57,8 @@ VIR_ENUM_IMPL(virDomainMemoryAccess, VIR_DOMAIN_MEMORY_ACCESS_LAST, "shared", "private")

+typedef struct _virDomainNumaDistance virDomainNumaDistance; +typedef virDomainNumaDistance *virDomainNumaDistancePtr;

typedef struct _virDomainNumaNode virDomainNumaNode; typedef virDomainNumaNode *virDomainNumaNodePtr; @@ -66,6 +77,12 @@ struct _virDomainNuma { virBitmapPtr nodeset; /* host memory nodes where this guest node resides */ virDomainNumatuneMemMode mode; /* memory mode selection */ virDomainMemoryAccess memAccess; /* shared memory access configuration */ + + struct _virDomainNumaDistance { + unsigned int value; /* locality value for node i->j or j->i */ + unsigned int cellid; + } *distances; /* remote node distances */ + size_t ndistances; } *mem_nodes; /* guest node configuration */ size_t nmem_nodes;

@@ -686,6 +703,174 @@ virDomainNumatuneNodesetIsAvailable(virDomainNumaPtr numatune, }

+static int +virDomainNumaDefNodeDistanceParseXML(virDomainNumaPtr def, + xmlXPathContextPtr ctxt, + unsigned int cur_cell) +{ + int ret = -1; + int sibling; + char *tmp = NULL; + xmlNodePtr *nodes = NULL; + size_t i, ndistances = def->nmem_nodes; + + if (!ndistances) + return 0; + + /* check if NUMA distances definition is present */ + if (!virXPathNode("./distances[1]", ctxt)) + return 0; + + if ((sibling = virXPathNodeSet("./distances[1]/sibling", ctxt, &nodes)) <= 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("NUMA distances defined without siblings")); + goto cleanup; + } + + for (i = 0; i < sibling; i++) { + virDomainNumaDistancePtr ldist, rdist; + unsigned int sibling_id, sibling_value; + + /* siblings are in order of parsing or explicitly numbered */ + if (!(tmp = virXMLPropString(nodes[i], "id"))) { + virReportError(VIR_ERR_XML_ERROR, + _("Missing 'id' attribute in NUMA " + "distances under 'cell id %d'"), + cur_cell); + goto cleanup; + } + + /* The "id" needs to be applicable */ + if (virStrToLong_uip(tmp, NULL, 10, &sibling_id) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid 'id' attribute in NUMA " + "distances for sibling: '%s'"), + tmp); + goto cleanup; + } + VIR_FREE(tmp); + + /* The "id" needs to be within numa/cell range */ + if (sibling_id >= ndistances) { + virReportError(VIR_ERR_XML_ERROR, + _("There is no cell administrated matching " + "'sibling_id %d' under NUMA 'cell id %d' "), + sibling_id, cur_cell);

I think this is a little awkward and could be problematic for translation. A better wording might be 'sibling_id %d' does not refer to a valid cell within NUMA 'cell id %d'

+ goto cleanup; + } + + /* We need a locality value. Check and correct + * distance to local and distance to remote node. + */ + if (!(tmp = virXMLPropString(nodes[i], "value"))) { + virReportError(VIR_ERR_XML_ERROR, + _("Missing 'value' attribute in NUMA distances " + "under 'cell id %d' for 'sibling id %d'"), + cur_cell, sibling_id); + goto cleanup; + } + + /* The "value" needs to be applicable */ + if (virStrToLong_uip(tmp, NULL, 10, &sibling_value) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid 'value' attribute in NUMA " + "distances for value: '%s'"),

I think it would be better if this error message was similar to the next two. E.g. "'value %s' is invalid for 'sibling id %d' under NUMA 'cell id %d'". What do you think?

+ tmp); + goto cleanup; + } + VIR_FREE(tmp); + + /* LOCAL_DISTANCE <= "value" <= UNREACHABLE */ + if (sibling_value < LOCAL_DISTANCE || + sibling_value > UNREACHABLE) { + virReportError(VIR_ERR_XML_ERROR, + _("Out of range value '%d' set for " + "'sibling id %d' under NUMA 'cell id %d' "),

Extra space at the end of this error message.

+ sibling_value, sibling_id, cur_cell); + goto cleanup; + } + + /* Assure correct LOCAL_DISTANCE setting if given */ + if (sibling_id == cur_cell && + sibling_value != LOCAL_DISTANCE) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid value '%d !LOCAL_DISTANCE' set for " + "'sibling id %d' under NUMA 'cell id %d' "),

Extra space at the end of this one too. Also, I think we can drop the '!LOCAL_DISTANCE'.

+ sibling_value, sibling_id, cur_cell); + goto cleanup; + } + + /* Assure correct LOCAL_DISTANCE setting if given */ + if (sibling_id != cur_cell && + sibling_value == LOCAL_DISTANCE) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid value '%d LOCAL_DISTANCE' set for " + "'sibling id %d' under NUMA 'cell id %d' "),

Same comments here.

+ sibling_value, sibling_id, cur_cell); + goto cleanup; + } + + /* Apply the local / remote distance */ + ldist = def->mem_nodes[cur_cell].distances; + if (!ldist) { + if (def->mem_nodes[cur_cell].ndistances) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid 'ndistances' set in NUMA " + "distances for sibling id: '%d'"), + cur_cell); + goto cleanup; + }

A value in def->mem_nodes[cur_cell].ndistances seems more like a coding error. It's definitely not an XML error since ndistances isn't in the schema.

+ + if (VIR_ALLOC_N(ldist, ndistances) < 0) + goto cleanup; + + ldist[cur_cell].value = LOCAL_DISTANCE; + ldist[cur_cell].cellid = cur_cell; + def->mem_nodes[cur_cell].ndistances = ndistances; + } + + ldist[sibling_id].cellid = sibling_id; + ldist[sibling_id].value = sibling_value; + def->mem_nodes[cur_cell].distances = ldist; + + /* Apply symmetry if none given */ + rdist = def->mem_nodes[sibling_id].distances; + if (!rdist) { + if (def->mem_nodes[sibling_id].ndistances) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid 'ndistances' set in NUMA " + "distances for sibling id: '%d'"), + sibling_id); + goto cleanup; + }

Same comment/question here.

+ + if (VIR_ALLOC_N(rdist, ndistances) < 0) + goto cleanup; + + rdist[sibling_id].value = LOCAL_DISTANCE; + rdist[sibling_id].cellid = sibling_id; + def->mem_nodes[sibling_id].ndistances = ndistances; + } + + rdist[cur_cell].cellid = cur_cell; + if (!rdist[cur_cell].value) + rdist[cur_cell].value = sibling_value; + def->mem_nodes[sibling_id].distances = rdist; + } + + ret = 0; + + cleanup: + if (ret) { + for (i = 0; i < ndistances; i++) + VIR_FREE(def->mem_nodes[i].distances); + } + VIR_FREE(nodes); + VIR_FREE(tmp); + + return ret; +} + int virDomainNumaDefCPUParseXML(virDomainNumaPtr def, xmlXPathContextPtr ctxt) @@ -694,7 +879,7 @@ virDomainNumaDefCPUParseXML(virDomainNumaPtr def, xmlNodePtr oldNode = ctxt->node; char *tmp = NULL; int n; - size_t i; + size_t i, j; int ret = -1;

/* check if NUMA definition is present */ @@ -712,7 +897,6 @@ virDomainNumaDefCPUParseXML(virDomainNumaPtr def, def->nmem_nodes = n;

for (i = 0; i < n; i++) { - size_t j; int rc; unsigned int cur_cell = i;

@@ -788,6 +972,10 @@ virDomainNumaDefCPUParseXML(virDomainNumaPtr def, def->mem_nodes[cur_cell].memAccess = rc; VIR_FREE(tmp); } + + /* Parse NUMA distances info */ + if (virDomainNumaDefNodeDistanceParseXML(def, ctxt, cur_cell) < 0) + goto cleanup; }

ret = 0; @@ -815,6 +1003,8 @@ virDomainNumaDefCPUFormatXML(virBufferPtr buf, virBufferAddLit(buf, "<numa>\n"); virBufferAdjustIndent(buf, 2); for (i = 0; i < ncells; i++) { + int ndistances; + memAccess = virDomainNumaGetNodeMemoryAccessMode(def, i);

if (!(cpustr = virBitmapFormat(virDomainNumaGetNodeCpumask(def, i)))) @@ -829,7 +1019,32 @@ virDomainNumaDefCPUFormatXML(virBufferPtr buf, if (memAccess) virBufferAsprintf(buf, " memAccess='%s'", virDomainMemoryAccessTypeToString(memAccess)); - virBufferAddLit(buf, "/>\n"); + + ndistances = def->mem_nodes[i].ndistances; + if (!ndistances) { + virBufferAddLit(buf, "/>\n"); + } else { + size_t j; + virDomainNumaDistancePtr distances = def->mem_nodes[i].distances; + + virBufferAddLit(buf, ">\n"); + virBufferAdjustIndent(buf, 2); + virBufferAddLit(buf, "<distances>\n"); + virBufferAdjustIndent(buf, 2); + for (j = 0; j < ndistances; j++) { + if (distances[j].value) { + virBufferAddLit(buf, "<sibling"); + virBufferAsprintf(buf, " id='%d'", distances[j].cellid); + virBufferAsprintf(buf, " value='%d'", distances[j].value); + virBufferAddLit(buf, "/>\n"); + } + } + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</distances>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</cell>\n"); + } + VIR_FREE(cpustr); } virBufferAdjustIndent(buf, -2);

I could have missed them, but I didn't notice any existing tests for all this NUMA parsing/formatting. If I did miss them, then your new settings should to be added to those tests. If no tests exist, we should consider adding some as a follow-up. Regards, Jim

On 10/12/2017 01:31 PM, Wim Ten Have wrote:

From: Wim ten Have <wim.ten.have@oracle.com>

This patch converts NUMA configurations between the Xen libxl configuration file format and libvirt's XML format.

XML HVM domain on a 4 node (2 cores/socket) configuration:

<cpu> <numa> <cell id='0' cpus='0-1' memory='2097152' unit='KiB'> <distances> <sibling id='0' value='10'/> <sibling id='1' value='21'/> <sibling id='2' value='31'/> <sibling id='3' value='21'/> </distances> </cell> <cell id='1' cpus='2-3' memory='2097152' unit='KiB'> <distances> <sibling id='0' value='21'/> <sibling id='1' value='10'/> <sibling id='2' value='21'/> <sibling id='3' value='31'/> </distances> </cell> <cell id='2' cpus='3-4' memory='2097152' unit='KiB'> <distances> <sibling id='0' value='31'/> <sibling id='1' value='21'/> <sibling id='2' value='10'/> <sibling id='3' value='21'/> </distances> </cell> <cell id='3' cpus='5-6' memory='2097152' unit='KiB'> <distances> <sibling id='0' value='21'/> <sibling id='1' value='31'/> <sibling id='2' value='21'/> <sibling id='3' value='10'/> </distances> </cell> </numa> </cpu>

Xen xl.cfg domain configuration:

vnuma = [["pnode=0","size=2048","vcpus=0-1","vdistances=10,21,31,21"], ["pnode=1","size=2048","vcpus=2-3","vdistances=21,10,21,31"], ["pnode=2","size=2048","vcpus=4-5","vdistances=31,21,10,21"], ["pnode=3","size=2048","vcpus=6-7","vdistances=21,31,21,10"]]

If there is no XML <distances> description amongst the <cell> data the conversion schema from xml to native will generate 10 for local and 20 for all remote instances.

Does xl have the same behavior? E.g. with vnuma = [["pnode=0","size=2048","vcpus=0-1"], ["pnode=1","size=2048","vcpus=2-3"], ["pnode=2","size=2048","vcpus=4-5"], ["pnode=3","size=2048","vcpus=6-7"]] will distances be 10 for local and 20 for remote nodes?

Signed-off-by: Wim ten Have <wim.ten.have@oracle.com> --- Changes on v2: - Reduce the indentation level under xenParseXLVnuma(). Changes on v3: - Add the Numa core split functions required to interface. --- src/conf/numa_conf.c | 138 ++++++++++++++++++++ src/conf/numa_conf.h | 20 +++ src/libvirt_private.syms | 5 + src/xenconfig/xen_xl.c | 333 +++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 496 insertions(+)

diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c index 00a3343fb..b513b1de1 100644 --- a/src/conf/numa_conf.c +++ b/src/conf/numa_conf.c @@ -1144,6 +1144,133 @@ virDomainNumaGetNodeCount(virDomainNumaPtr numa) }

+size_t +virDomainNumaSetNodeCount(virDomainNumaPtr numa, size_t nmem_nodes) +{ + if (!nmem_nodes) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot set an empty mem_nodes set")); + return 0; + } + + if (numa->mem_nodes) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot alter an existing mem_nodes set")); + return 0; + } + + if (VIR_ALLOC_N(numa->mem_nodes, nmem_nodes) < 0) + return 0; + + numa->nmem_nodes = nmem_nodes; + + return numa->nmem_nodes; +} + +size_t +virDomainNumaGetNodeDistance(virDomainNumaPtr numa, + size_t node, + size_t cellid) +{ + virDomainNumaDistancePtr distances = NULL; + + if (node < numa->nmem_nodes) + distances = numa->mem_nodes[node].distances; + + /* + * Present the configured distance value. If + * out of range or not available set the platform + * defined default for local and remote nodes. + */ + if (!distances || + !distances[cellid].value || + !numa->mem_nodes[node].ndistances) + return (node == cellid) ? \ + LOCAL_DISTANCE : REMOTE_DISTANCE;

This return statement will fit within 80 columns on a single line. (Side note: lines <= 80 columns is not strictly enforced in libvirt.)

+ + return distances[cellid].value; +} + + +int +virDomainNumaSetNodeDistance(virDomainNumaPtr numa, + size_t node, + size_t cellid, + unsigned int value) +{ + virDomainNumaDistancePtr distances; + + if (node >= numa->nmem_nodes) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Argument 'node' %zu outranges " + "defined number of NUMA nodes"), + node); + return -1; + } + + distances = numa->mem_nodes[node].distances; + if (!distances || + cellid >= numa->mem_nodes[node].ndistances) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Arguments under memnode element do not " + "correspond with existing guest's NUMA cell")); + return -1; + } + + /* + * Advanced Configuration and Power Interface + * Specification version 6.1. Chapter 5.2.17 + * System Locality Distance Information Table + * ... Distance values of 0-9 are reserved. + */ + if (value < LOCAL_DISTANCE || + value > UNREACHABLE) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Distance value of %d is in 0-9 reserved range"),

Should the '0-9' be dropped? E.g. value could be > UNREACHABLE.

+ value); + return -1; + } + + if (value == LOCAL_DISTANCE && node != cellid) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Distance value %d under node %zu seems "

s/seems/is/ ?

+ "LOCAL_DISTANCE and should be set to 10"), + value, node); + return -1; + } + + distances[cellid].cellid = cellid; + distances[cellid].value = value; + + return distances[cellid].value; +} + + +size_t +virDomainNumaSetNodeDistanceCount(virDomainNumaPtr numa, + size_t node, + size_t ndistances) +{ + virDomainNumaDistancePtr distances; + + distances = numa->mem_nodes[node].distances; + if (distances) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Cannot alter an existing nmem_nodes distances set for node: %zu"), + node); + return 0; + } + + if (VIR_ALLOC_N(distances, ndistances) < 0) + return 0; + + numa->mem_nodes[node].distances = distances; + numa->mem_nodes[node].ndistances = ndistances; + + return numa->mem_nodes[node].ndistances; +} + + virBitmapPtr virDomainNumaGetNodeCpumask(virDomainNumaPtr numa, size_t node) @@ -1152,6 +1279,17 @@ virDomainNumaGetNodeCpumask(virDomainNumaPtr numa, }

+virBitmapPtr +virDomainNumaSetNodeCpumask(virDomainNumaPtr numa, + size_t node, + virBitmapPtr cpumask) +{ + numa->mem_nodes[node].cpumask = cpumask; + + return numa->mem_nodes[node].cpumask; +} + + virDomainMemoryAccess virDomainNumaGetNodeMemoryAccessMode(virDomainNumaPtr numa, size_t node) diff --git a/src/conf/numa_conf.h b/src/conf/numa_conf.h index 378b772e4..8184b528b 100644 --- a/src/conf/numa_conf.h +++ b/src/conf/numa_conf.h @@ -87,12 +87,32 @@ int virDomainNumatuneMaybeGetNodeset(virDomainNumaPtr numatune,

size_t virDomainNumaGetNodeCount(virDomainNumaPtr numa);

+size_t virDomainNumaSetNodeCount(virDomainNumaPtr numa, + size_t nmem_nodes) + ATTRIBUTE_NONNULL(1); +size_t virDomainNumaGetNodeDistance(virDomainNumaPtr numa, + size_t node, + size_t sibling) + ATTRIBUTE_NONNULL(1); +int virDomainNumaSetNodeDistance(virDomainNumaPtr numa, + size_t node, + size_t sibling, + unsigned int value) + ATTRIBUTE_NONNULL(1); +size_t virDomainNumaSetNodeDistanceCount(virDomainNumaPtr numa, + size_t node, + size_t ndistances) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3);

Is ATTRIBUTE_NONNULL needed for ndistances?

virBitmapPtr virDomainNumaGetNodeCpumask(virDomainNumaPtr numa, size_t node) ATTRIBUTE_NONNULL(1); virDomainMemoryAccess virDomainNumaGetNodeMemoryAccessMode(virDomainNumaPtr numa, size_t node) ATTRIBUTE_NONNULL(1); +virBitmapPtr virDomainNumaSetNodeCpumask(virDomainNumaPtr numa, + size_t node, + virBitmapPtr cpumask) + ATTRIBUTE_NONNULL(1);

This file contains sections for the function types. E.g. all the getters are in 'Getters' section, setters in the 'Setters' section, etc.

unsigned long long virDomainNumaGetNodeMemorySize(virDomainNumaPtr numa, size_t node) ATTRIBUTE_NONNULL(1); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 26c5ddb40..861660b94 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -713,9 +713,14 @@ virDomainNumaGetMaxCPUID; virDomainNumaGetMemorySize; virDomainNumaGetNodeCount; virDomainNumaGetNodeCpumask; +virDomainNumaGetNodeDistance; virDomainNumaGetNodeMemoryAccessMode; virDomainNumaGetNodeMemorySize; virDomainNumaNew; +virDomainNumaSetNodeCount; +virDomainNumaSetNodeCpumask; +virDomainNumaSetNodeDistance; +virDomainNumaSetNodeDistanceCount; virDomainNumaSetNodeMemorySize; virDomainNumatuneFormatNodeset; virDomainNumatuneFormatXML; diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c index 8acbfe3f6..54b2ac650 100644 --- a/src/xenconfig/xen_xl.c +++ b/src/xenconfig/xen_xl.c @@ -309,6 +309,205 @@ xenParseXLSpice(virConfPtr conf, virDomainDefPtr def) return -1; }

+#ifdef LIBXL_HAVE_VNUMA +static int +xenParseXLVnuma(virConfPtr conf, virDomainDefPtr def) +{ + int ret = -1; + char *tmp = NULL; + char **token = NULL; + size_t vcpus = 0; + size_t nr_nodes = 0; + size_t vnodeCnt = 0; + virCPUDefPtr cpu = NULL; + virConfValuePtr list; + virConfValuePtr vnode; + virDomainNumaPtr numa; + + numa = def->numa; + if (numa == NULL) + return -1; + + list = virConfGetValue(conf, "vnuma"); + if (!list || list->type != VIR_CONF_LIST) + return 0; + + vnode = list->list; + while (vnode && vnode->type == VIR_CONF_LIST) { + vnode = vnode->next; + nr_nodes++; + } + + if (!virDomainNumaSetNodeCount(numa, nr_nodes)) + goto cleanup; + + if (VIR_ALLOC(cpu) < 0) + goto cleanup; + + list = list->list; + while (list) { + int pnode = -1; + virBitmapPtr cpumask = NULL; + unsigned long long kbsize = 0; + + /* Is there a sublist (vnode)? */ + if (list && list->type == VIR_CONF_LIST) { + vnode = list->list; + + while (vnode && vnode->type == VIR_CONF_STRING) { + const char *data; + const char *str = vnode->str; + + if (!str || + !(data = strrchr(str, '='))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("vnuma vnode invalid format '%s'"), + str); + goto cleanup; + } + data++; + + if (*data) { + size_t len; + char vtoken[64]; + + if (STRPREFIX(str, "pnode")) { + unsigned int cellid; + + len = strlen(data); + if (!virStrncpy(vtoken, data, + len, sizeof(vtoken))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("vnuma vnode %zu pnode '%s' too long for destination"), + vnodeCnt, data); + goto cleanup; + } + + if ((virStrToLong_ui(vtoken, NULL, 10, &cellid) < 0) || + (cellid >= nr_nodes)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("vnuma vnode %zu invalid value pnode '%s'"),

Maybe better worded as "vnuma vnode %zu contains invalid pnode value '%s'"?

+ vnodeCnt, data); + goto cleanup; + } + pnode = cellid; + } else if (STRPREFIX(str, "size")) { + len = strlen(data); + if (!virStrncpy(vtoken, data, + len, sizeof(vtoken))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("vnuma vnode %zu size '%s' too long for destination"), + vnodeCnt, data); + goto cleanup; + } + + if (virStrToLong_ull(vtoken, NULL, 10, &kbsize) < 0) + goto cleanup; + + virDomainNumaSetNodeMemorySize(numa, vnodeCnt, (kbsize * 1024)); + + } else if (STRPREFIX(str, "vcpus")) { + len = strlen(data); + if (!virStrncpy(vtoken, data, + len, sizeof(vtoken))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("vnuma vnode %zu vcpus '%s' too long for destination"), + vnodeCnt, data); + goto cleanup; + } + + if ((virBitmapParse(vtoken, &cpumask, VIR_DOMAIN_CPUMASK_LEN) < 0) || + (virDomainNumaSetNodeCpumask(numa, vnodeCnt, cpumask) == NULL)) + goto cleanup; + + vcpus += virBitmapCountBits(cpumask); + + } else if (STRPREFIX(str, "vdistances")) { + size_t i, ndistances; + unsigned int value; + + len = strlen(data); + if (!virStrncpy(vtoken, data, + len, sizeof(vtoken))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("vnuma vnode %zu vdistances '%s' too long for destination"), + vnodeCnt, data); + goto cleanup; + } + + if (VIR_STRDUP(tmp, vtoken) < 0) + goto cleanup; + + if (!(token = virStringSplitCount(tmp, ",", 0, &ndistances))) + goto cleanup; + + if (ndistances != nr_nodes) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("vnuma pnode %d configured '%s' (count %zu) doesn't fit the number of specified vnodes %zu"), + pnode, str, ndistances, nr_nodes); + goto cleanup; + } + + if (virDomainNumaSetNodeDistanceCount(numa, vnodeCnt, ndistances) != ndistances) + goto cleanup; + + for (i = 0; i < ndistances; i++) { + if ((virStrToLong_ui(token[i], NULL, 10, &value) < 0) || + (virDomainNumaSetNodeDistance(numa, vnodeCnt, i, value) != value)) + goto cleanup; + } + + } else { + virReportError(VIR_ERR_CONF_SYNTAX, + _("vnuma vnode %zu invalid token '%s'"),

How about "Invalid vnuma configuration for vnode %zu"?

+ vnodeCnt, str); + goto cleanup; + } + } + vnode = vnode->next; + } + } + + if ((pnode < 0) || + (cpumask == NULL) || + (kbsize == 0)) { + virReportError(VIR_ERR_CONF_SYNTAX, + _("vnuma vnode %zu incomplete token. Missing%s%s%s"),

This could be tough for translation too. Maybe the message I suggested above is sufficient for here too.

+ vnodeCnt, + (pnode < 0) ? " \'pnode\'":"", + (cpumask == NULL) ? " \'vcpus\'":"", + (kbsize == 0) ? " \'size\'":""); + goto cleanup; + } + + list = list->next; + vnodeCnt++; + } + + if (def->maxvcpus == 0) + def->maxvcpus = vcpus; + + if (def->maxvcpus < vcpus) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("vnuma requested vcpus %zu fails available maxvcpus %zu"),

How about "vnuma configuration contains %zu vcpus, which is greater than %zu maxvcpus"?

+ vcpus, def->maxvcpus); + goto cleanup; + } + + cpu->type = VIR_CPU_TYPE_GUEST; + def->cpu = cpu; + + ret = 0; + + cleanup: + if (ret) + VIR_FREE(cpu); + virStringListFree(token); + VIR_FREE(tmp); + + return ret; +} +#endif

Wow, that's a hairy function :-). I probably missed something that coverity will complain about once this code hits libvirt.git.

static int xenParseXLDiskSrc(virDomainDiskDefPtr disk, char *srcstr) @@ -863,6 +1062,11 @@ xenParseXL(virConfPtr conf, if (xenParseXLOS(conf, def, caps) < 0) goto cleanup;

+#ifdef LIBXL_HAVE_VNUMA + if (xenParseXLVnuma(conf, def) < 0) + goto cleanup; +#endif + if (xenParseXLDisk(conf, def) < 0) goto cleanup;

@@ -1005,6 +1209,130 @@ xenFormatXLOS(virConfPtr conf, virDomainDefPtr def) return 0; }

+#ifdef LIBXL_HAVE_VNUMA +static int +xenFormatXLVnode(virConfValuePtr list, virBufferPtr buf) +{ + int ret = -1; + virConfValuePtr numaPnode, tmp; + + if (virBufferCheckError(buf) < 0) + goto cleanup; + + if (VIR_ALLOC(numaPnode) < 0) + goto cleanup; + + /* Place VNODE directive */ + numaPnode->type = VIR_CONF_STRING; + numaPnode->str = virBufferContentAndReset(buf); + + tmp = list->list; + while (tmp && tmp->next) + tmp = tmp->next; + if (tmp) + tmp->next = numaPnode; + else + list->list = numaPnode; + ret = 0; + + cleanup: + virBufferFreeAndReset(buf); + return ret; +} + +static int ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2)

I don't think these are needed on this internal function.

+xenFormatXLVnuma(virConfValuePtr list, + virDomainNumaPtr numa, size_t node, size_t nr_nodes)

One parameter per line.

+{ + int ret = -1; + size_t i; + + virBuffer buf = VIR_BUFFER_INITIALIZER; + virConfValuePtr numaVnode, tmp; + + size_t nodeSize = virDomainNumaGetNodeMemorySize(numa, node) / 1024; + char *nodeVcpus = virBitmapFormat(virDomainNumaGetNodeCpumask(numa, node)); + + if (VIR_ALLOC(numaVnode) < 0) + goto cleanup; + + numaVnode->type = VIR_CONF_LIST; + numaVnode->list = NULL; + + /* pnode */ + virBufferAsprintf(&buf, "pnode=%ld", node); + xenFormatXLVnode(numaVnode, &buf); + + /* size */ + virBufferAsprintf(&buf, "size=%ld", nodeSize); + xenFormatXLVnode(numaVnode, &buf); + + /* vcpus */ + virBufferAsprintf(&buf, "vcpus=%s", nodeVcpus); + xenFormatXLVnode(numaVnode, &buf); + + /* distances */ + virBufferAddLit(&buf, "vdistances="); + for (i = 0; i < nr_nodes; i++) { + virBufferAsprintf(&buf, "%zu", + virDomainNumaGetNodeDistance(numa, node, i)); + if ((nr_nodes - i) > 1) + virBufferAddLit(&buf, ","); + } + xenFormatXLVnode(numaVnode, &buf); + + tmp = list->list; + while (tmp && tmp->next) + tmp = tmp->next; + if (tmp) + tmp->next = numaVnode; + else + list->list = numaVnode; + ret = 0; + + cleanup: + VIR_FREE(nodeVcpus); + return ret; +} + +static int +xenFormatXLDomainVnuma(virConfPtr conf, virDomainDefPtr def) +{ + virDomainNumaPtr numa = def->numa; + virConfValuePtr vnumaVal; + size_t i; + size_t nr_nodes; + + if (numa == NULL) + return -1; + + if (VIR_ALLOC(vnumaVal) < 0) + return -1; + + vnumaVal->type = VIR_CONF_LIST; + vnumaVal->list = NULL; + + nr_nodes = virDomainNumaGetNodeCount(numa); + for (i = 0; i < nr_nodes; i++) { + if (xenFormatXLVnuma(vnumaVal, numa, i, nr_nodes) < 0) + goto cleanup; + } + + if (vnumaVal->list != NULL) { + int ret = virConfSetValue(conf, "vnuma", vnumaVal); + vnumaVal = NULL; + if (ret < 0) + return -1; + } + VIR_FREE(vnumaVal); + + return 0; + + cleanup: + virConfFreeValue(vnumaVal); + return -1; +} +#endif

the "formaters" are easier to read :-).

static char * xenFormatXLDiskSrcNet(virStorageSourcePtr src) @@ -1642,6 +1970,11 @@ xenFormatXL(virDomainDefPtr def, virConnectPtr conn) if (xenFormatXLOS(conf, def) < 0) goto cleanup;

+#ifdef LIBXL_HAVE_VNUMA + if (xenFormatXLDomainVnuma(conf, def) < 0) + goto cleanup; +#endif + if (xenFormatXLDomainDisks(conf, def) < 0) goto cleanup;

Other than the nits, looks good. Regards, Jim

On Thu, 26 Oct 2017 17:40:37 -0600 Jim Fehlig <jfehlig@suse.com> wrote:

On 10/12/2017 01:31 PM, Wim Ten Have wrote:

...

From: Wim ten Have <wim.ten.have@oracle.com>

This patch generates a NUMA distance-aware libxl description from the information extracted from a NUMA distance-aware libvirt XML file. ... + /* pnode */ + p->pnode = simulate ? 0 : i;

So any time the number of vnuma nodes exceeds the number of physical nodes, all vnuma nodes are confined to physical node 0?

Yes.

Does xl behave this way too?

Yes.

...

+ + /* memory size */ + p->memkb = virDomainNumaGetNodeMemorySize(numa, i); + + /* vcpus */ + bitmap = virDomainNumaGetNodeCpumask(numa, i); + if (bitmap == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("vnuma sibling %zu missing vcpus set"), i); + goto cleanup; + } + + if ((cpu = virBitmapNextSetBit(bitmap, -1)) < 0) + goto cleanup; + + libxl_bitmap_init(&vcpu_bitmap); + if (libxl_cpu_bitmap_alloc(ctx, &vcpu_bitmap, b_info->max_vcpus)) { + virReportOOMError(); + goto cleanup; + } + + do { + libxl_bitmap_set(&vcpu_bitmap, cpu); + } while ((cpu = virBitmapNextSetBit(bitmap, cpu)) >= 0); + + libxl_bitmap_copy_alloc(ctx, &p->vcpus, &vcpu_bitmap); + libxl_bitmap_dispose(&vcpu_bitmap); + + /* vdistances */ + if (VIR_ALLOC_N(p->distances, num_vnuma) < 0) + goto cleanup; + p->num_distances = num_vnuma; + + for (j = 0; j < num_vnuma; j++) + p->distances[j] = virDomainNumaGetNodeDistance(numa, i, j); + } + + b_info->vnuma_nodes = vnuma_nodes; + b_info->num_vnuma_nodes = num_vnuma; + + ret = 0; + + cleanup: + if (ret) { + for (i = 0; i < num_vnuma; i++) { + libxl_vnode_info *p = &vnuma_nodes[i]; + + VIR_FREE(p->distances); + } + VIR_FREE(vnuma_nodes); + } + + return ret; +} +#endif + static int libxlDiskSetDiscard(libxl_device_disk *x_disk, int discard) { @@ -2209,6 +2325,11 @@ libxlBuildDomainConfig(virPortAllocatorPtr graphicsports, if (libxlMakeDomBuildInfo(def, ctx, caps, d_config) < 0) return -1;

+#ifdef LIBXL_HAVE_VNUMA + if (libxlMakeVnumaList(def, ctx, d_config) < 0) + return -1; +#endif + if (libxlMakeDiskList(def, d_config) < 0) return -1;

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 8483d6ecf..656f4a82d 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -2788,7 +2788,8 @@ libxlDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flag virCheckFlags(VIR_DOMAIN_DEFINE_VALIDATE, NULL);

if (flags & VIR_DOMAIN_DEFINE_VALIDATE) - parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE_SCHEMA; + parse_flags |= (VIR_DOMAIN_DEF_PARSE_VALIDATE_SCHEMA | + VIR_DOMAIN_DEF_PARSE_ABI_UPDATE);

Why is this change needed? In its current form, the patch doesn't touch any code in the domain def post parse callback.

I remove this. I added this to forcibly recompute memory size in case total_memory held a value not matching the numa cells setting. (!= 0) See virDomainDefPostParseMemory() /* Attempt to infer the initial memory size from the sum NUMA memory sizes * in case ABI updates are allowed or the <memory> element wasn't specified */ if (def->mem.total_memory == 0 || parseFlags & VIR_DOMAIN_DEF_PARSE_ABI_UPDATE || parseFlags & VIR_DOMAIN_DEF_PARSE_ABI_UPDATE_MIGRATION) numaMemory = virDomainNumaGetMemorySize(def->numa); Will address all other brought comments under v6. Regards, - Wim.

On 10/12/2017 01:31 PM, Wim Ten Have wrote:

From: Wim ten Have <wim.ten.have@oracle.com>

Test a bidirectional xen-xl domxml to and from native for numa support administration as brought under this patch series.

Signed-off-by: Wim ten Have <wim.ten.have@oracle.com> --- .../test-fullvirt-vnuma-autocomplete.cfg | 26 +++++++ .../test-fullvirt-vnuma-autocomplete.xml | 85 ++++++++++++++++++++++ .../test-fullvirt-vnuma-nodistances.cfg | 26 +++++++ .../test-fullvirt-vnuma-nodistances.xml | 53 ++++++++++++++ .../test-fullvirt-vnuma-partialdist.cfg | 26 +++++++ .../test-fullvirt-vnuma-partialdist.xml | 60 +++++++++++++++ tests/xlconfigdata/test-fullvirt-vnuma.cfg | 26 +++++++ tests/xlconfigdata/test-fullvirt-vnuma.xml | 81 +++++++++++++++++++++ tests/xlconfigtest.c | 6 ++

Cool! Thanks for the various configurations to test all the hairy parsing code :-). Reviewed-by: Jim Fehlig <jfehlig@suse.com> BTW I should have mentioned it while reviewing 3/4, but we should also have tests for the libxl_domain_config generator, now that we have a test suite for that. See tests/libxlxml2domconfigtest.c. Regards, Jim

9 files changed, 389 insertions(+) create mode 100644 tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.cfg create mode 100644 tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.xml create mode 100644 tests/xlconfigdata/test-fullvirt-vnuma-nodistances.cfg create mode 100644 tests/xlconfigdata/test-fullvirt-vnuma-nodistances.xml create mode 100644 tests/xlconfigdata/test-fullvirt-vnuma-partialdist.cfg create mode 100644 tests/xlconfigdata/test-fullvirt-vnuma-partialdist.xml create mode 100644 tests/xlconfigdata/test-fullvirt-vnuma.cfg create mode 100644 tests/xlconfigdata/test-fullvirt-vnuma.xml

diff --git a/tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.cfg b/tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.cfg new file mode 100644 index 000000000..edba69a17 --- /dev/null +++ b/tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.cfg @@ -0,0 +1,26 @@ +name = "XenGuest2" +uuid = "c7a5fdb2-cdaf-9455-926a-d65c16db1809" +maxmem = 8192 +memory = 8192 +vcpus = 12 +pae = 1 +acpi = 1 +apic = 1 +viridian = 0 +rtc_timeoffset = 0 +localtime = 0 +on_poweroff = "destroy" +on_reboot = "restart" +on_crash = "restart" +device_model = "/usr/lib/xen/bin/qemu-system-i386" +sdl = 0 +vnc = 1 +vncunused = 1 +vnclisten = "127.0.0.1" +vif = [ "mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000" ] +parallel = "none" +serial = "none" +builder = "hvm" +boot = "d" +vnuma = [ [ "pnode=0", "size=2048", "vcpus=0,11", "vdistances=10,21,31,41,51,61" ], [ "pnode=1", "size=2048", "vcpus=1,10", "vdistances=21,10,21,31,41,51" ], [ "pnode=2", "size=2048", "vcpus=2,9", "vdistances=31,21,10,21,31,41" ], [ "pnode=3", "size=2048", "vcpus=3,8", "vdistances=41,31,21,10,21,31" ], [ "pnode=4", "size=2048", "vcpus=4,7", "vdistances=51,41,31,21,10,21" ], [ "pnode=5", "size=2048", "vcpus=5-6", "vdistances=61,51,41,31,21,10" ] ] +disk = [ "format=raw,vdev=hda,access=rw,backendtype=phy,target=/dev/HostVG/XenGuest2" ] diff --git a/tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.xml b/tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.xml new file mode 100644 index 000000000..e3639eb04 --- /dev/null +++ b/tests/xlconfigdata/test-fullvirt-vnuma-autocomplete.xml @@ -0,0 +1,85 @@ +<domain type='xen'> + <name>XenGuest2</name> + <uuid>c7a5fdb2-cdaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>8388608</memory> + <currentMemory unit='KiB'>8388608</currentMemory> + <vcpu placement='static'>12</vcpu> + <os> + <type arch='x86_64' machine='xenfv'>hvm</type> + <loader type='rom'>/usr/lib/xen/boot/hvmloader</loader> + <boot dev='cdrom'/> + </os> + <features> + <acpi/> + <apic/> + <pae/> + </features> + <cpu> + <numa> + <cell id='0' cpus='0,11' memory='2097152' unit='KiB'> + <distances> + <sibling id='1' value='21'/> + <sibling id='2' value='31'/> + <sibling id='3' value='41'/> + <sibling id='4' value='51'/> + <sibling id='5' value='61'/> + </distances> + </cell> + <cell id='1' cpus='1,10' memory='2097152' unit='KiB'> + <distances> + <sibling id='2' value='21'/> + <sibling id='3' value='31'/> + <sibling id='4' value='41'/> + <sibling id='5' value='51'/> + </distances> + </cell> + <cell id='2' cpus='2,9' memory='2097152' unit='KiB'> + <distances> + <sibling id='3' value='21'/> + <sibling id='4' value='31'/> + <sibling id='5' value='41'/> + </distances> + </cell> + <cell id='3' cpus='3,8' memory='2097152' unit='KiB'> + <distances> + <sibling id='4' value='21'/> + <sibling id='5' value='31'/> + </distances> + </cell> + <cell id='4' cpus='4,7' memory='2097152' unit='KiB'> + <distances> + <sibling id='5' value='21'/> + </distances> + </cell> + <cell id='5' cpus='5-6' memory='2097152' unit='KiB'/> + </numa> + </cpu> + <clock offset='variable' adjustment='0' basis='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <devices> + <emulator>/usr/lib/xen/bin/qemu-system-i386</emulator> + <disk type='block' device='disk'> + <driver name='phy' type='raw'/> + <source dev='/dev/HostVG/XenGuest2'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='ide' index='0'/> + <interface type='bridge'> + <mac address='00:16:3e:66:92:9c'/> + <source bridge='xenbr1'/> + <script path='vif-bridge'/> + <model type='e1000'/> + </interface> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1'> + <listen type='address' address='127.0.0.1'/> + </graphics> + <video> + <model type='cirrus' vram='8192' heads='1' primary='yes'/> + </video> + </devices> +</domain> diff --git a/tests/xlconfigdata/test-fullvirt-vnuma-nodistances.cfg b/tests/xlconfigdata/test-fullvirt-vnuma-nodistances.cfg new file mode 100644 index 000000000..8186edfee --- /dev/null +++ b/tests/xlconfigdata/test-fullvirt-vnuma-nodistances.cfg @@ -0,0 +1,26 @@ +name = "XenGuest2" +uuid = "c7a5fdb2-cdaf-9455-926a-d65c16db1809" +maxmem = 8192 +memory = 8192 +vcpus = 8 +pae = 1 +acpi = 1 +apic = 1 +viridian = 0 +rtc_timeoffset = 0 +localtime = 0 +on_poweroff = "destroy" +on_reboot = "restart" +on_crash = "restart" +device_model = "/usr/lib/xen/bin/qemu-system-i386" +sdl = 0 +vnc = 1 +vncunused = 1 +vnclisten = "127.0.0.1" +vif = [ "mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000" ] +parallel = "none" +serial = "none" +builder = "hvm" +boot = "d" +vnuma = [ [ "pnode=0", "size=2048", "vcpus=0-1", "vdistances=10,20,20,20" ], [ "pnode=1", "size=2048", "vcpus=2-3", "vdistances=20,10,20,20" ], [ "pnode=2", "size=2048", "vcpus=4-5", "vdistances=20,20,10,20" ], [ "pnode=3", "size=2048", "vcpus=6-7", "vdistances=20,20,20,10" ] ] +disk = [ "format=raw,vdev=hda,access=rw,backendtype=phy,target=/dev/HostVG/XenGuest2" ] diff --git a/tests/xlconfigdata/test-fullvirt-vnuma-nodistances.xml b/tests/xlconfigdata/test-fullvirt-vnuma-nodistances.xml new file mode 100644 index 000000000..9cab3ca6d --- /dev/null +++ b/tests/xlconfigdata/test-fullvirt-vnuma-nodistances.xml @@ -0,0 +1,53 @@ +<domain type='xen'> + <name>XenGuest2</name> + <uuid>c7a5fdb2-cdaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>8388608</memory> + <currentMemory unit='KiB'>8388608</currentMemory> + <vcpu placement='static'>8</vcpu> + <os> + <type arch='x86_64' machine='xenfv'>hvm</type> + <loader type='rom'>/usr/lib/xen/boot/hvmloader</loader> + <boot dev='cdrom'/> + </os> + <features> + <acpi/> + <apic/> + <pae/> + </features> + <cpu> + <numa> + <cell id='0' cpus='0-1' memory='2097152' unit='KiB'/> + <cell id='1' cpus='2-3' memory='2097152' unit='KiB'/> + <cell id='2' cpus='4-5' memory='2097152' unit='KiB'/> + <cell id='3' cpus='6-7' memory='2097152' unit='KiB'/> + </numa> + </cpu> + <clock offset='variable' adjustment='0' basis='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <devices> + <emulator>/usr/lib/xen/bin/qemu-system-i386</emulator> + <disk type='block' device='disk'> + <driver name='phy' type='raw'/> + <source dev='/dev/HostVG/XenGuest2'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='ide' index='0'/> + <interface type='bridge'> + <mac address='00:16:3e:66:92:9c'/> + <source bridge='xenbr1'/> + <script path='vif-bridge'/> + <model type='e1000'/> + </interface> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1'> + <listen type='address' address='127.0.0.1'/> + </graphics> + <video> + <model type='cirrus' vram='8192' heads='1' primary='yes'/> + </video> + </devices> +</domain> diff --git a/tests/xlconfigdata/test-fullvirt-vnuma-partialdist.cfg b/tests/xlconfigdata/test-fullvirt-vnuma-partialdist.cfg new file mode 100644 index 000000000..861a50e76 --- /dev/null +++ b/tests/xlconfigdata/test-fullvirt-vnuma-partialdist.cfg @@ -0,0 +1,26 @@ +name = "XenGuest2" +uuid = "c7a5fdb2-cdaf-9455-926a-d65c16db1809" +maxmem = 8192 +memory = 8192 +vcpus = 8 +pae = 1 +acpi = 1 +apic = 1 +viridian = 0 +rtc_timeoffset = 0 +localtime = 0 +on_poweroff = "destroy" +on_reboot = "restart" +on_crash = "restart" +device_model = "/usr/lib/xen/bin/qemu-system-i386" +sdl = 0 +vnc = 1 +vncunused = 1 +vnclisten = "127.0.0.1" +vif = [ "mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000" ] +parallel = "none" +serial = "none" +builder = "hvm" +boot = "d" +vnuma = [ [ "pnode=0", "size=2048", "vcpus=0-1", "vdistances=10,21,31,41" ], [ "pnode=1", "size=2048", "vcpus=2-3", "vdistances=21,10,20,20" ], [ "pnode=2", "size=2048", "vcpus=4-5", "vdistances=31,20,10,20" ], [ "pnode=3", "size=2048", "vcpus=6-7", "vdistances=41,20,20,10" ] ] +disk = [ "format=raw,vdev=hda,access=rw,backendtype=phy,target=/dev/HostVG/XenGuest2" ] diff --git a/tests/xlconfigdata/test-fullvirt-vnuma-partialdist.xml b/tests/xlconfigdata/test-fullvirt-vnuma-partialdist.xml new file mode 100644 index 000000000..084b8893c --- /dev/null +++ b/tests/xlconfigdata/test-fullvirt-vnuma-partialdist.xml @@ -0,0 +1,60 @@ +<domain type='xen'> + <name>XenGuest2</name> + <uuid>c7a5fdb2-cdaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>8388608</memory> + <currentMemory unit='KiB'>8388608</currentMemory> + <vcpu placement='static'>8</vcpu> + <os> + <type arch='x86_64' machine='xenfv'>hvm</type> + <loader type='rom'>/usr/lib/xen/boot/hvmloader</loader> + <boot dev='cdrom'/> + </os> + <features> + <acpi/> + <apic/> + <pae/> + </features> + <cpu> + <numa> + <cell id='0' cpus='0-1' memory='2097152' unit='KiB'> + <distances> + <sibling id='0' value='10'/> + <sibling id='1' value='21'/> + <sibling id='2' value='31'/> + <sibling id='3' value='41'/> + </distances> + </cell> + <cell id='1' cpus='2-3' memory='2097152' unit='KiB'/> + <cell id='2' cpus='4-5' memory='2097152' unit='KiB'/> + <cell id='3' cpus='6-7' memory='2097152' unit='KiB'/> + </numa> + </cpu> + <clock offset='variable' adjustment='0' basis='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <devices> + <emulator>/usr/lib/xen/bin/qemu-system-i386</emulator> + <disk type='block' device='disk'> + <driver name='phy' type='raw'/> + <source dev='/dev/HostVG/XenGuest2'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='ide' index='0'/> + <interface type='bridge'> + <mac address='00:16:3e:66:92:9c'/> + <source bridge='xenbr1'/> + <script path='vif-bridge'/> + <model type='e1000'/> + </interface> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1'> + <listen type='address' address='127.0.0.1'/> + </graphics> + <video> + <model type='cirrus' vram='8192' heads='1' primary='yes'/> + </video> + </devices> +</domain> diff --git a/tests/xlconfigdata/test-fullvirt-vnuma.cfg b/tests/xlconfigdata/test-fullvirt-vnuma.cfg new file mode 100644 index 000000000..91e233ac2 --- /dev/null +++ b/tests/xlconfigdata/test-fullvirt-vnuma.cfg @@ -0,0 +1,26 @@ +name = "XenGuest2" +uuid = "c7a5fdb2-cdaf-9455-926a-d65c16db1809" +maxmem = 8192 +memory = 8192 +vcpus = 8 +pae = 1 +acpi = 1 +apic = 1 +viridian = 0 +rtc_timeoffset = 0 +localtime = 0 +on_poweroff = "destroy" +on_reboot = "restart" +on_crash = "restart" +device_model = "/usr/lib/xen/bin/qemu-system-i386" +sdl = 0 +vnc = 1 +vncunused = 1 +vnclisten = "127.0.0.1" +vif = [ "mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000" ] +parallel = "none" +serial = "none" +builder = "hvm" +boot = "d" +vnuma = [ [ "pnode=0", "size=2048", "vcpus=0-1", "vdistances=10,21,31,41" ], [ "pnode=1", "size=2048", "vcpus=2-3", "vdistances=21,10,21,31" ], [ "pnode=2", "size=2048", "vcpus=4-5", "vdistances=31,21,10,21" ], [ "pnode=3", "size=2048", "vcpus=6-7", "vdistances=41,31,21,10" ] ] +disk = [ "format=raw,vdev=hda,access=rw,backendtype=phy,target=/dev/HostVG/XenGuest2" ] diff --git a/tests/xlconfigdata/test-fullvirt-vnuma.xml b/tests/xlconfigdata/test-fullvirt-vnuma.xml new file mode 100644 index 000000000..5368b0d9c --- /dev/null +++ b/tests/xlconfigdata/test-fullvirt-vnuma.xml @@ -0,0 +1,81 @@ +<domain type='xen'> + <name>XenGuest2</name> + <uuid>c7a5fdb2-cdaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>8388608</memory> + <currentMemory unit='KiB'>8388608</currentMemory> + <vcpu placement='static'>8</vcpu> + <os> + <type arch='x86_64' machine='xenfv'>hvm</type> + <loader type='rom'>/usr/lib/xen/boot/hvmloader</loader> + <boot dev='cdrom'/> + </os> + <features> + <acpi/> + <apic/> + <pae/> + </features> + <cpu> + <numa> + <cell id='0' cpus='0-1' memory='2097152' unit='KiB'> + <distances> + <sibling id='0' value='10'/> + <sibling id='1' value='21'/> + <sibling id='2' value='31'/> + <sibling id='3' value='41'/> + </distances> + </cell> + <cell id='1' cpus='2-3' memory='2097152' unit='KiB'> + <distances> + <sibling id='0' value='21'/> + <sibling id='1' value='10'/> + <sibling id='2' value='21'/> + <sibling id='3' value='31'/> + </distances> + </cell> + <cell id='2' cpus='4-5' memory='2097152' unit='KiB'> + <distances> + <sibling id='0' value='31'/> + <sibling id='1' value='21'/> + <sibling id='2' value='10'/> + <sibling id='3' value='21'/> + </distances> + </cell> + <cell id='3' cpus='6-7' memory='2097152' unit='KiB'> + <distances> + <sibling id='0' value='41'/> + <sibling id='1' value='31'/> + <sibling id='2' value='21'/> + <sibling id='3' value='10'/> + </distances> + </cell> + </numa> + </cpu> + <clock offset='variable' adjustment='0' basis='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <devices> + <emulator>/usr/lib/xen/bin/qemu-system-i386</emulator> + <disk type='block' device='disk'> + <driver name='phy' type='raw'/> + <source dev='/dev/HostVG/XenGuest2'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='ide' index='0'/> + <interface type='bridge'> + <mac address='00:16:3e:66:92:9c'/> + <source bridge='xenbr1'/> + <script path='vif-bridge'/> + <model type='e1000'/> + </interface> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1'> + <listen type='address' address='127.0.0.1'/> + </graphics> + <video> + <model type='cirrus' vram='8192' heads='1' primary='yes'/> + </video> + </devices> +</domain> diff --git a/tests/xlconfigtest.c b/tests/xlconfigtest.c index 30468c905..f2b1cd66d 100644 --- a/tests/xlconfigtest.c +++ b/tests/xlconfigtest.c @@ -270,6 +270,12 @@ mymain(void) DO_TEST("fullvirt-multi-timer"); DO_TEST("fullvirt-nestedhvm"); DO_TEST("fullvirt-nestedhvm-disabled"); +#ifdef LIBXL_HAVE_VNUMA + DO_TEST("fullvirt-vnuma"); + DO_TEST_PARSE("fullvirt-vnuma-autocomplete", false); + DO_TEST_PARSE("fullvirt-vnuma-nodistances", false); + DO_TEST_PARSE("fullvirt-vnuma-partialdist", false); +#endif

DO_TEST("paravirt-cmdline"); DO_TEST_FORMAT("paravirt-cmdline-extra-root", false);