[libvirt] [PATCHv2 6/7] capabilities: Add additional data to the NUMA topology info
Peter Krempa
pkrempa at redhat.com
Wed Jan 23 10:12:35 UTC 2013
On 01/23/13 11:04, Daniel P. Berrange wrote:
> On Tue, Jan 22, 2013 at 10:30:25PM +0100, Peter Krempa wrote:
>> 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.
>> ---
>> src/conf/capabilities.c | 31 ++++++++++++++++++++++-----
>> src/nodeinfo.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++---
>> src/test/test_driver.c | 3 +++
>> src/xen/xend_internal.c | 1 +
>> 4 files changed, 84 insertions(+), 8 deletions(-)
>>
>> diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
>> index 14d3498..c9036f4 100644
>> --- a/src/conf/capabilities.c
>> +++ b/src/conf/capabilities.c
>> @@ -686,27 +686,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;
>> }
>>
>> /**
>> @@ -782,9 +802,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 5febcfb..dc6771a 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,52 @@ 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, ',', &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;
>> +}
>> +
>> +static int
>> +virNodeCapsFillCPUInfo(int cpu_id, virCapsHostNUMACellCPUPtr cpu)
>> +{
>> + cpu->id = cpu_id;
>> + cpu->socket_id = virNodeGetCpuValue(SYSFS_CPU_PATH, cpu_id,
>> + "topology/physical_package_id", -1);
>> + cpu->core_id = virNodeGetCpuValue(SYSFS_CPU_PATH, cpu_id,
>> + "topology/core_id", -1);
>> +
>> + if (cpu->socket_id == -1 || cpu->core_id == -1)
>> + return 0;
>> +
>> + if (!(cpu->siblings = virNodeGetSiblingsList(SYSFS_CPU_PATH, cpu_id)))
>> + return -1;
>> +
>> + return 0;
>> +}
>> +
>> int
>> nodeCapsInitNUMA(virCapsPtr caps)
>> {
>> @@ -1516,9 +1564,12 @@ 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)
>> + goto cleanup;
>> + }
>> + }
>>
>> if (virCapabilitiesAddHostNUMACell(caps, n, ncpus, cpus) < 0)
>> goto cleanup;
>> diff --git a/src/test/test_driver.c b/src/test/test_driver.c
>> index 6909fa4..038f627 100644
>> --- a/src/test/test_driver.c
>> +++ b/src/test/test_driver.c
>> @@ -559,6 +559,9 @@ static int testOpenDefault(virConnectPtr conn) {
>> }
>> for (u = 0 ; u < 16 ; u++) {
>> privconn->cells[u % 2].cpus[(u / 2)].id = u;
>> + privconn->cells[u % 2].cpus[(u / 2)].socket_id = -1;
>> + privconn->cells[u % 2].cpus[(u / 2)].core_id = -1;
>> + privconn->cells[u % 2].cpus[(u / 2)].siblings = NULL;
>> }
>
> This is wrong because these fields are unsigned int.
Hm, yeah, I forgot to update this. Anyways, the data doesn't pose
problem here as the output isn't enhanced until siblings is non-NULL.
This code gets fixed in 7/7.
>
>> diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
>> index 57d8325..434f558 100644
>> --- a/src/xen/xend_internal.c
>> +++ b/src/xen/xend_internal.c
>> @@ -1161,6 +1161,7 @@ sexpr_to_xend_topology(const struct sexpr *root,
>> ignore_value(virBitmapGetBit(cpuset, cpu, &used));
>> if (used) {
>> cpuInfo[n].id = cpu;
>> + cpuInfo[n].siblings = NULL;
>
> As mentioned before, this should be initializing based on the nodeinfo.
> Here you've allowed socket_id + core_id to all initialize to 0 which
> is wrong.
Also here, the siblings are NULL so the new output isn't used at all. I
added the condition so that the new code could be avoided until I
prepare means to test the XEN support.
>
> Daniel
>
More information about the libvir-list
mailing list