[libvirt] [PATCHv3 0/6] Add additional data to NUMA topology info
Another spin of the series. Difference to previous version: - remove redundant init of some structures - add graceful behavior if topology cannot be discovered - fix incorrect usage of virBitmapParse I'm working on adding more docs for the capabilities XML and the support for the xend driver. Peter Krempa (6): schema: Make the cpuset type reusable across schema files schemas: Add schemas for more CPU topology information in the caps XML conf: Split out NUMA topology formatting to simplify access to data capabilities: Switch CPU data in NUMA topology to a struct capabilities: Add additional data to the NUMA topology info test: Add support for thread and core information for the test driver docs/schemas/basictypes.rng | 6 +++ docs/schemas/capability.rng | 11 +++++ docs/schemas/domaincommon.rng | 5 --- src/conf/capabilities.c | 94 ++++++++++++++++++++++++++++++------------- src/conf/capabilities.h | 16 +++++++- src/libvirt_private.syms | 1 + src/nodeinfo.c | 85 ++++++++++++++++++++++++++++++++------ src/qemu/qemu_process.c | 2 +- src/test/test_driver.c | 24 +++++++++-- src/xen/xend_internal.c | 21 +++++----- 10 files changed, 204 insertions(+), 61 deletions(-) -- 1.8.1.1
--- docs/schemas/basictypes.rng | 6 ++++++ docs/schemas/domaincommon.rng | 5 ----- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng index 38cab16..8e44e8d 100644 --- a/docs/schemas/basictypes.rng +++ b/docs/schemas/basictypes.rng @@ -219,4 +219,10 @@ </data> </define> + <define name="cpuset"> + <data type="string"> + <param name="pattern">([0-9]+(-[0-9]+)?|\^[0-9]+)(,([0-9]+(-[0-9]+)?|\^[0-9]+))*</param> + </data> + </define> + </grammar> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 7f3320e..1a7d6b5 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3684,11 +3684,6 @@ <!-- Type library --> - <define name="cpuset"> - <data type="string"> - <param name="pattern">([0-9]+(-[0-9]+)?|\^[0-9]+)(,([0-9]+(-[0-9]+)?|\^[0-9]+))*</param> - </data> - </define> <define name="countCPU"> <data type="unsignedShort"> <param name="pattern">[0-9]+</param> -- 1.8.1.1
This patch adds RNG schemas for adding more information in the topology output of the NUMA section in the capabilities XML. The added elements are designed to provide more information about the placement and topology of the processors in the system to management applications. A demonstration of supported XML added by this patch: <capabilities> <host> <topology> <cells num='3'> <cell id='0'> <cpus num='4'> <!-- this is node with Hyperthreading --> <cpu id='0' socket_id='0' core_id='0' siblings='0-1'/> <cpu id='1' socket_id='0' core_id='0' siblings='0-1'/> <cpu id='2' socket_id='0' core_id='1' siblings='2-3'/> <cpu id='3' socket_id='0' core_id='1' siblings='2-3'/> </cpus> </cell> <cell id='1'> <cpus num='4'> <!-- this is node with modules (Bulldozer) --> <cpu id='4' socket_id='0' core_id='2' siblings='4-5'/> <cpu id='5' socket_id='0' core_id='3' siblings='4-5'/> <cpu id='6' socket_id='0' core_id='4' siblings='6-7'/> <cpu id='7' socket_id='0' core_id='5' siblings='6-7'/> </cpus> </cell> <cell id='2'> <cpus num='4'> <!-- this is a normal multi-core node --> <cpu id='8' socket_id='1' core_id='0' siblings='8'/> <cpu id='9' socket_id='1' core_id='1' siblings='9'/> <cpu id='10' socket_id='1' core_id='2' siblings='10'/> <cpu id='11' socket_id='1' core_id='3' siblings='11'/> </cpus> </cell> </cells> </topology> </host> </capabilities> The socket_id field represents identification of the physical socket the CPU is plugged in. This ID may not be identical to the physical socket ID reported by the kernel. The core_id identifies a core within a socket. Also this field may not accurately represent physical ID's. The core_id is guaranteed to be unique within a cell and a socket. There may be duplicates between sockets. Only cores sharing core_id within one cell and one socket can be considered as threads. Cores sharing core_id within sparate cells are distinct cores. The siblings field is a list of CPU id's the cpu id's the CPU is sibling with - thus a thread. The list is in the cpuset format. --- docs/schemas/capability.rng | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng index 593d340..53fb04a 100644 --- a/docs/schemas/capability.rng +++ b/docs/schemas/capability.rng @@ -194,6 +194,17 @@ <attribute name='id'> <ref name='unsignedInt'/> </attribute> + <optional> + <attribute name='socket_id'> + <ref name='unsignedInt'/> + </attribute> + <attribute name='core_id'> + <ref name='unsignedInt'/> + </attribute> + <attribute name='siblings'> + <ref name='cpuset'/> + </attribute> + </optional> </element> </define> -- 1.8.1.1
--- src/conf/capabilities.c | 43 +++++++++++++++++++++++++------------------ 1 file changed, 25 insertions(+), 18 deletions(-) diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index 365c511..0d2512e 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -678,6 +678,28 @@ virCapabilitiesDefaultGuestEmulator(virCapsPtr caps, return NULL; } +static void +virCapabilitiesFormatNUMATopology(virBufferPtr xml, + size_t ncells, + virCapsHostNUMACellPtr *cells) +{ + int i; + int j; + + virBufferAddLit(xml, " <topology>\n"); + virBufferAsprintf(xml, " <cells num='%zu'>\n", ncells); + for (i = 0; i < ncells; i++) { + virBufferAsprintf(xml, " <cell id='%d'>\n", cells[i]->num); + virBufferAsprintf(xml, " <cpus num='%d'>\n", cells[i]->ncpus); + for (j = 0; j < cells[i]->ncpus; j++) + virBufferAsprintf(xml, " <cpu id='%d'/>\n", + cells[i]->cpus[j]); + virBufferAddLit(xml, " </cpus>\n"); + virBufferAddLit(xml, " </cell>\n"); + } + virBufferAddLit(xml, " </cells>\n"); + virBufferAddLit(xml, " </topology>\n"); +} /** * virCapabilitiesFormatXML: @@ -752,24 +774,9 @@ virCapabilitiesFormatXML(virCapsPtr caps) virBufferAddLit(&xml, " </migration_features>\n"); } - if (caps->host.nnumaCell) { - virBufferAddLit(&xml, " <topology>\n"); - virBufferAsprintf(&xml, " <cells num='%zu'>\n", - caps->host.nnumaCell); - for (i = 0 ; i < caps->host.nnumaCell ; i++) { - virBufferAsprintf(&xml, " <cell id='%d'>\n", - caps->host.numaCell[i]->num); - virBufferAsprintf(&xml, " <cpus num='%d'>\n", - caps->host.numaCell[i]->ncpus); - for (j = 0 ; j < caps->host.numaCell[i]->ncpus ; j++) - virBufferAsprintf(&xml, " <cpu id='%d'/>\n", - caps->host.numaCell[i]->cpus[j]); - virBufferAddLit(&xml, " </cpus>\n"); - virBufferAddLit(&xml, " </cell>\n"); - } - virBufferAddLit(&xml, " </cells>\n"); - virBufferAddLit(&xml, " </topology>\n"); - } + if (caps->host.nnumaCell) + virCapabilitiesFormatNUMATopology(&xml, caps->host.nnumaCell, + caps->host.numaCell); for (i = 0; i < caps->host.nsecModels; i++) { virBufferAddLit(&xml, " <secmodel>\n"); -- 1.8.1.1
This will allow storing additional topology data in the NUMA topology definition. This patch changes the storage type and fixes fallout of the change across the drivers using it. This patch also changes semantics of adding new NUMA cell information. Until now the data were re-allocated and copied to the topology definition. This patch changes the addition function to steal the pointer to a pre-allocated structure to simplify the code. --- src/conf/capabilities.c | 32 +++++++++++++++++++++----------- src/conf/capabilities.h | 16 ++++++++++++++-- src/libvirt_private.syms | 1 + src/nodeinfo.c | 15 ++++++--------- src/qemu/qemu_process.c | 2 +- src/test/test_driver.c | 15 ++++++++++++--- src/xen/xend_internal.c | 21 +++++++++++---------- 7 files changed, 66 insertions(+), 36 deletions(-) diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index 0d2512e..575981c 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -65,12 +65,29 @@ virCapabilitiesNew(virArch hostarch, return caps; } +void +virCapabilitiesClearHostNUMACellCPUTopology(virCapsHostNUMACellCPUPtr cpus, + size_t ncpus) +{ + size_t i; + + if (!cpus) + return; + + for (i = 0; i < ncpus; i++) { + virBitmapFree(cpus[i].siblings); + cpus[i].siblings = NULL; + } +} + static void virCapabilitiesFreeHostNUMACell(virCapsHostNUMACellPtr cell) { if (cell == NULL) return; + virCapabilitiesClearHostNUMACellCPUTopology(cell->cpus, cell->ncpus); + VIR_FREE(cell->cpus); VIR_FREE(cell); } @@ -236,7 +253,7 @@ virCapabilitiesAddHostMigrateTransport(virCapsPtr caps, * @caps: capabilities to extend * @num: ID number of NUMA cell * @ncpus: number of CPUs in cell - * @cpus: array of CPU ID numbers for cell + * @cpus: array of CPU definition structures, the pointer is stolen * * Registers a new NUMA cell for a host, passing in a * array of CPU IDs belonging to the cell @@ -245,7 +262,7 @@ int virCapabilitiesAddHostNUMACell(virCapsPtr caps, int num, int ncpus, - const int *cpus) + virCapsHostNUMACellCPUPtr cpus) { virCapsHostNUMACellPtr cell; @@ -256,16 +273,9 @@ virCapabilitiesAddHostNUMACell(virCapsPtr caps, if (VIR_ALLOC(cell) < 0) return -1; - if (VIR_ALLOC_N(cell->cpus, ncpus) < 0) { - VIR_FREE(cell); - return -1; - } - memcpy(cell->cpus, - cpus, - ncpus * sizeof(*cpus)); - cell->ncpus = ncpus; cell->num = num; + cell->cpus = cpus; caps->host.numaCell[caps->host.nnumaCell++] = cell; @@ -693,7 +703,7 @@ virCapabilitiesFormatNUMATopology(virBufferPtr xml, virBufferAsprintf(xml, " <cpus num='%d'>\n", cells[i]->ncpus); for (j = 0; j < cells[i]->ncpus; j++) virBufferAsprintf(xml, " <cpu id='%d'/>\n", - cells[i]->cpus[j]); + cells[i]->cpus[j].id); virBufferAddLit(xml, " </cpus>\n"); virBufferAddLit(xml, " </cell>\n"); } diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h index 19b99c6..f5a5c48 100644 --- a/src/conf/capabilities.h +++ b/src/conf/capabilities.h @@ -84,12 +84,21 @@ struct _virCapsGuest { virCapsGuestFeaturePtr *features; }; +typedef struct _virCapsHostNUMACellCPU virCapsHostNUMACellCPU; +typedef virCapsHostNUMACellCPU *virCapsHostNUMACellCPUPtr; +struct _virCapsHostNUMACellCPU { + unsigned int id; + unsigned int socket_id; + unsigned int core_id; + virBitmapPtr siblings; +}; + typedef struct _virCapsHostNUMACell virCapsHostNUMACell; typedef virCapsHostNUMACell *virCapsHostNUMACellPtr; struct _virCapsHostNUMACell { int num; int ncpus; - int *cpus; + virCapsHostNUMACellCPUPtr cpus; }; typedef struct _virCapsHostSecModel virCapsHostSecModel; @@ -201,7 +210,7 @@ extern int virCapabilitiesAddHostNUMACell(virCapsPtr caps, int num, int ncpus, - const int *cpus); + virCapsHostNUMACellCPUPtr cpus); extern int @@ -250,6 +259,9 @@ virCapabilitiesSupportsGuestOSTypeArch(virCapsPtr caps, const char *ostype, virArch arch); +void +virCapabilitiesClearHostNUMACellCPUTopology(virCapsHostNUMACellCPUPtr cpu, + size_t ncpus); extern virArch virCapabilitiesDefaultGuestArch(virCapsPtr caps, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index fc23adc..42eae3e 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -56,6 +56,7 @@ virCapabilitiesAddHostFeature; virCapabilitiesAddHostMigrateTransport; virCapabilitiesAddHostNUMACell; virCapabilitiesAllocMachines; +virCapabilitiesClearHostNUMACellCPUTopology; virCapabilitiesDefaultGuestArch; virCapabilitiesDefaultGuestEmulator; virCapabilitiesDefaultGuestMachine; diff --git a/src/nodeinfo.c b/src/nodeinfo.c index 2cfd16c..c29769c 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -1479,9 +1479,10 @@ nodeCapsInitNUMA(virCapsPtr caps) int n; unsigned long *mask = NULL; unsigned long *allonesmask = NULL; - int *cpus = NULL; + virCapsHostNUMACellCPUPtr cpus = NULL; int ret = -1; int max_n_cpus = NUMA_MAX_N_CPUS; + int ncpus = 0; if (numa_available() < 0) return 0; @@ -1495,7 +1496,6 @@ nodeCapsInitNUMA(virCapsPtr caps) for (n = 0 ; n <= numa_max_node() ; n++) { int i; - int ncpus; /* The first time this returns -1, ENOENT if node doesn't exist... */ if (numa_node_to_cpus(n, mask, mask_n_bytes) < 0) { VIR_WARN("NUMA topology for cell %d of %d not available, ignoring", @@ -1518,20 +1518,17 @@ nodeCapsInitNUMA(virCapsPtr caps) for (ncpus = 0, i = 0 ; i < max_n_cpus ; i++) if (MASK_CPU_ISSET(mask, i)) - cpus[ncpus++] = i; + cpus[ncpus++].id = i; - if (virCapabilitiesAddHostNUMACell(caps, - n, - ncpus, - cpus) < 0) + if (virCapabilitiesAddHostNUMACell(caps, n, ncpus, cpus) < 0) goto cleanup; - - VIR_FREE(cpus); + cpus = NULL; } ret = 0; cleanup: + virCapabilitiesClearHostNUMACellCPUTopology(cpus, ncpus); VIR_FREE(cpus); VIR_FREE(mask); VIR_FREE(allonesmask); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 55d00e3..a2ce007 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2032,7 +2032,7 @@ qemuPrepareCpumap(virQEMUDriverPtr driver, if (result) { for (j = 0; j < cur_ncpus; j++) ignore_value(virBitmapSetBit(cpumap, - driver->caps->host.numaCell[i]->cpus[j])); + driver->caps->host.numaCell[i]->cpus[j].id)); } } } diff --git a/src/test/test_driver.c b/src/test/test_driver.c index a10a377..6909fa4 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -69,7 +69,7 @@ typedef struct _testDomainObjPrivate *testDomainObjPrivatePtr; struct _testCell { unsigned long mem; int numCpus; - int cpus[MAX_CPUS]; + virCapsHostNUMACellCPU cpus[MAX_CPUS]; }; typedef struct _testCell testCell; typedef struct _testCell *testCellPtr; @@ -174,8 +174,17 @@ testBuildCapabilities(virConnectPtr conn) { goto no_memory; for (i = 0; i < privconn->numCells; i++) { + virCapsHostNUMACellCPUPtr cpu_cells; + + if (VIR_ALLOC_N(cpu_cells, privconn->cells[i].numCpus) < 0) + goto no_memory; + + memcpy(cpu_cells, privconn->cells[i].cpus, + sizeof(*cpu_cells) * privconn->cells[i].numCpus); + + if (virCapabilitiesAddHostNUMACell(caps, i, privconn->cells[i].numCpus, - privconn->cells[i].cpus) < 0) + cpu_cells) < 0) goto no_memory; } @@ -549,7 +558,7 @@ static int testOpenDefault(virConnectPtr conn) { privconn->cells[u].mem = (u + 1) * 2048 * 1024; } for (u = 0 ; u < 16 ; u++) { - privconn->cells[u % 2].cpus[(u / 2)] = u; + privconn->cells[u % 2].cpus[(u / 2)].id = u; } if (!(privconn->caps = testBuildCapabilities(conn))) diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index 00d1c9b..d0e54a8 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -1112,7 +1112,7 @@ sexpr_to_xend_topology(const struct sexpr *root, { const char *nodeToCpu; const char *cur; - int *cpuNums = NULL; + virCapsHostNUMACellCPUPtr cpuInfo = NULL; int cell, cpu, nb_cpus; int n = 0; int numCpus; @@ -1124,9 +1124,6 @@ sexpr_to_xend_topology(const struct sexpr *root, numCpus = sexpr_int(root, "node/nr_cpus"); - if (VIR_ALLOC_N(cpuNums, numCpus) < 0) - goto memory_error; - cur = nodeToCpu; while (*cur != 0) { virBitmapPtr cpuset = NULL; @@ -1155,31 +1152,35 @@ sexpr_to_xend_topology(const struct sexpr *root, goto error; } + if (VIR_ALLOC_N(cpuInfo, numCpus) < 0) + goto memory_error; + for (n = 0, cpu = 0; cpu < numCpus; cpu++) { bool used; ignore_value(virBitmapGetBit(cpuset, cpu, &used)); if (used) - cpuNums[n++] = cpu; + cpuInfo[n++].id = cpu; } virBitmapFree(cpuset); - if (virCapabilitiesAddHostNUMACell(caps, cell, nb_cpus, cpuNums) < 0) + if (virCapabilitiesAddHostNUMACell(caps, cell, nb_cpus, cpuInfo) < 0) goto memory_error; + cpuInfo = NULL; } - VIR_FREE(cpuNums); + return 0; parse_error: virReportError(VIR_ERR_XEN_CALL, "%s", _("topology syntax error")); error: - VIR_FREE(cpuNums); + virCapabilitiesClearHostNUMACellCPUTopology(cpuInfo, nb_cpus); + VIR_FREE(cpuInfo); return -1; memory_error: - VIR_FREE(cpuNums); virReportOOMError(); - return -1; + goto error; } -- 1.8.1.1
This patch adds data gathering to the NUMA gathering files and adds support for outputting the data. The test driver and xend driver need to be adapted to fill sensible data to the structure in a future patch. --- src/conf/capabilities.c | 31 ++++++++++++++++---- src/nodeinfo.c | 76 +++++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 96 insertions(+), 11 deletions(-) diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index 575981c..4897b9a 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -688,27 +688,47 @@ virCapabilitiesDefaultGuestEmulator(virCapsPtr caps, return NULL; } -static void +static int virCapabilitiesFormatNUMATopology(virBufferPtr xml, size_t ncells, virCapsHostNUMACellPtr *cells) { int i; int j; + char *siblings; virBufferAddLit(xml, " <topology>\n"); virBufferAsprintf(xml, " <cells num='%zu'>\n", ncells); for (i = 0; i < ncells; i++) { virBufferAsprintf(xml, " <cell id='%d'>\n", cells[i]->num); virBufferAsprintf(xml, " <cpus num='%d'>\n", cells[i]->ncpus); - for (j = 0; j < cells[i]->ncpus; j++) - virBufferAsprintf(xml, " <cpu id='%d'/>\n", + for (j = 0; j < cells[i]->ncpus; j++) { + virBufferAsprintf(xml, " <cpu id='%d'", cells[i]->cpus[j].id); + + if (cells[i]->cpus[j].siblings) { + if (!(siblings = virBitmapFormat(cells[i]->cpus[j].siblings))) { + virReportOOMError(); + return -1; + } + + virBufferAsprintf(xml, + " socket_id='%d' core_id='%d' siblings='%s'", + cells[i]->cpus[j].socket_id, + cells[i]->cpus[j].core_id, + siblings); + VIR_FREE(siblings); + } + virBufferAddLit(xml, "/>\n"); + } + virBufferAddLit(xml, " </cpus>\n"); virBufferAddLit(xml, " </cell>\n"); } virBufferAddLit(xml, " </cells>\n"); virBufferAddLit(xml, " </topology>\n"); + + return 0; } /** @@ -784,9 +804,10 @@ virCapabilitiesFormatXML(virCapsPtr caps) virBufferAddLit(&xml, " </migration_features>\n"); } - if (caps->host.nnumaCell) + if (caps->host.nnumaCell && virCapabilitiesFormatNUMATopology(&xml, caps->host.nnumaCell, - caps->host.numaCell); + caps->host.numaCell) < 0) + return NULL; for (i = 0; i < caps->host.nsecModels; i++) { virBufferAddLit(&xml, " <secmodel>\n"); diff --git a/src/nodeinfo.c b/src/nodeinfo.c index c29769c..5ce7d4f 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -79,9 +79,11 @@ freebsdNodeGetCPUCount(void) #ifdef __linux__ # define CPUINFO_PATH "/proc/cpuinfo" # define SYSFS_SYSTEM_PATH "/sys/devices/system" +# define SYSFS_CPU_PATH SYSFS_SYSTEM_PATH"/cpu" # define PROCSTAT_PATH "/proc/stat" # define MEMINFO_PATH "/proc/meminfo" # define SYSFS_MEMORY_SHARED_PATH "/sys/kernel/mm/ksm" +# define SYSFS_THREAD_SIBLINGS_LIST_LENGHT_MAX 1024 # define LINUX_NB_CPU_STATS 4 # define LINUX_NB_MEMORY_STATS_ALL 4 @@ -1473,6 +1475,59 @@ cleanup: # define MASK_CPU_ISSET(mask, cpu) \ (((mask)[((cpu) / n_bits(*(mask)))] >> ((cpu) % n_bits(*(mask)))) & 1) +static virBitmapPtr +virNodeGetSiblingsList(const char *dir, int cpu_id) +{ + char *path = NULL; + char *buf = NULL; + virBitmapPtr ret = NULL; + + if (virAsprintf(&path, "%s/cpu%u/topology/thread_siblings_list", + dir, cpu_id) < 0) { + virReportOOMError(); + goto cleanup; + } + + if (virFileReadAll(path, SYSFS_THREAD_SIBLINGS_LIST_LENGHT_MAX, &buf) < 0) + goto cleanup; + + if (virBitmapParse(buf, 0, &ret, NUMA_MAX_N_CPUS) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Failed to parse thread siblings")); + goto cleanup; + } + +cleanup: + VIR_FREE(buf); + VIR_FREE(path); + return ret; +} + +/* returns 1 on success, 0 if the detection failed and -1 on hard error */ +static int +virNodeCapsFillCPUInfo(int cpu_id, virCapsHostNUMACellCPUPtr cpu) +{ + int tmp; + cpu->id = cpu_id; + + if ((tmp = virNodeGetCpuValue(SYSFS_CPU_PATH, cpu_id, + "topology/physical_package_id", -1)) < 0) + return 0; + + cpu->socket_id = tmp; + + if ((tmp = virNodeGetCpuValue(SYSFS_CPU_PATH, cpu_id, + "topology/core_id", -1)) < 0) + return 0; + + cpu->core_id = tmp; + + if (!(cpu->siblings = virNodeGetSiblingsList(SYSFS_CPU_PATH, cpu_id))) + return -1; + + return 0; +} + int nodeCapsInitNUMA(virCapsPtr caps) { @@ -1483,6 +1538,7 @@ nodeCapsInitNUMA(virCapsPtr caps) int ret = -1; int max_n_cpus = NUMA_MAX_N_CPUS; int ncpus = 0; + bool topology_failed = false; if (numa_available() < 0) return 0; @@ -1516,20 +1572,28 @@ nodeCapsInitNUMA(virCapsPtr caps) if (VIR_ALLOC_N(cpus, ncpus) < 0) goto cleanup; - for (ncpus = 0, i = 0 ; i < max_n_cpus ; i++) - if (MASK_CPU_ISSET(mask, i)) - cpus[ncpus++].id = i; + for (ncpus = 0, i = 0 ; i < max_n_cpus ; i++) { + if (MASK_CPU_ISSET(mask, i)) { + if (virNodeCapsFillCPUInfo(i, cpus + ncpus++) < 0) { + topology_failed = true; + virResetLastError(); + } + } + } if (virCapabilitiesAddHostNUMACell(caps, n, ncpus, cpus) < 0) goto cleanup; - cpus = NULL; } ret = 0; cleanup: - virCapabilitiesClearHostNUMACellCPUTopology(cpus, ncpus); - VIR_FREE(cpus); + if (topology_failed || ret < 0) + virCapabilitiesClearHostNUMACellCPUTopology(cpus, ncpus); + + if (ret < 0) + VIR_FREE(cpus); + VIR_FREE(mask); VIR_FREE(allonesmask); return ret; -- 1.8.1.1
On Thu, Jan 24, 2013 at 10:57:33AM +0100, Peter Krempa wrote:
diff --git a/src/nodeinfo.c b/src/nodeinfo.c index c29769c..5ce7d4f 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -79,9 +79,11 @@ freebsdNodeGetCPUCount(void) #ifdef __linux__ # define CPUINFO_PATH "/proc/cpuinfo" # define SYSFS_SYSTEM_PATH "/sys/devices/system" +# define SYSFS_CPU_PATH SYSFS_SYSTEM_PATH"/cpu" # define PROCSTAT_PATH "/proc/stat" # define MEMINFO_PATH "/proc/meminfo" # define SYSFS_MEMORY_SHARED_PATH "/sys/kernel/mm/ksm" +# define SYSFS_THREAD_SIBLINGS_LIST_LENGHT_MAX 1024
s/LENGHT_MAX/LENGTH_MAX/ and where it is used later too. ACK with the typo fixed Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
This patch adds demo processor topology information for the test driver. --- src/test/test_driver.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 6909fa4..ddc4110 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -558,7 +558,16 @@ static int testOpenDefault(virConnectPtr conn) { privconn->cells[u].mem = (u + 1) * 2048 * 1024; } for (u = 0 ; u < 16 ; u++) { - privconn->cells[u % 2].cpus[(u / 2)].id = u; + virBitmapPtr siblings = virBitmapNew(16); + if (!siblings) { + virReportOOMError(); + goto error; + } + ignore_value(virBitmapSetBit(siblings, u)); + privconn->cells[u / 8].cpus[(u % 8)].id = u; + privconn->cells[u / 8].cpus[(u % 8)].socket_id = u / 8; + privconn->cells[u / 8].cpus[(u % 8)].core_id = u % 8; + privconn->cells[u / 8].cpus[(u % 8)].siblings = siblings; } if (!(privconn->caps = testBuildCapabilities(conn))) -- 1.8.1.1
On Thu, Jan 24, 2013 at 10:57:28AM +0100, Peter Krempa wrote:
Another spin of the series. Difference to previous version: - remove redundant init of some structures - add graceful behavior if topology cannot be discovered - fix incorrect usage of virBitmapParse
I'm working on adding more docs for the capabilities XML and the support for the xend driver.
Peter Krempa (6): schema: Make the cpuset type reusable across schema files schemas: Add schemas for more CPU topology information in the caps XML conf: Split out NUMA topology formatting to simplify access to data capabilities: Switch CPU data in NUMA topology to a struct capabilities: Add additional data to the NUMA topology info test: Add support for thread and core information for the test driver
ACK to all. For future reference, if you have a series and the early patches in the series have already been ACKed, there's no need to keep reposting the ACKd patches. Just merge the first ACKs ones in the series, and repost what is left over. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 01/24/13 11:05, Daniel P. Berrange wrote:
On Thu, Jan 24, 2013 at 10:57:28AM +0100, Peter Krempa wrote:
Another spin of the series. Difference to previous version: - remove redundant init of some structures - add graceful behavior if topology cannot be discovered - fix incorrect usage of virBitmapParse
I'm working on adding more docs for the capabilities XML and the support for the xend driver.
Peter Krempa (6): schema: Make the cpuset type reusable across schema files schemas: Add schemas for more CPU topology information in the caps XML conf: Split out NUMA topology formatting to simplify access to data capabilities: Switch CPU data in NUMA topology to a struct capabilities: Add additional data to the NUMA topology info test: Add support for thread and core information for the test driver
ACK to all.
For future reference, if you have a series and the early patches in the series have already been ACKed, there's no need to keep reposting the ACKd patches. Just merge the first ACKs ones in the series, and repost what is left over.
Hm, yeah, those were pretty self-contained. I will keep that in mind for the next time. Thanks for the review. Peter
Daniel
participants (2)
-
Daniel P. Berrange -
Peter Krempa