8:45 a.m.
https://bugzilla.redhat.com/show_bug.cgi?id=888503
The values added by this patch help avoid management applications the dodgy
approach of multiplying values in the nodeinfo structure to gather data needed
for their management decisions.
Peter Krempa (2):
nodeinfo: Gather total number of physical cores and fix tests
capabilities: Add total number of cores and threads to the XML
docs/formatcaps.html.in | 2 +-
docs/schemas/capability.rng | 8 +++++++
src/conf/cpu_conf.c | 4 ++++
src/conf/cpu_conf.h | 2 ++
src/libvirt_private.syms | 1 +
src/nodeinfo.c | 37 ++++++++++++++++++++++-------
src/nodeinfo.h | 2 ++
src/qemu/qemu_capabilities.c | 3 ++-
tests/nodeinfodata/linux-x86-test1.expected | 2 +-
tests/nodeinfodata/linux-x86-test2.expected | 2 +-
tests/nodeinfodata/linux-x86-test3.expected | 2 +-
tests/nodeinfodata/linux-x86-test4.expected | 2 +-
tests/nodeinfodata/linux-x86-test5.expected | 2 +-
tests/nodeinfodata/linux-x86-test6.expected | 2 +-
tests/nodeinfodata/linux-x86-test7.expected | 2 +-
tests/nodeinfodata/linux-x86-test8.expected | 2 +-
tests/nodeinfotest.c | 11 +++++----
tests/testutilsqemu.c | 2 ++
18 files changed, 66 insertions(+), 22 deletions(-)
--
1.8.1
8:45 a.m.
New subject: [libvirt] [PATCH 1/2] nodeinfo: Gather total number of physical cores and fix tests
Gather the total number of physical cores needed for the next patch and
add the data to the tests.
The incorrect value in test 7 is expected as the test is based on data
from a machine that has incorrect NUMA information.
---
src/libvirt_private.syms | 1 +
src/nodeinfo.c | 37 ++++++++++++++++++++++-------
src/nodeinfo.h | 2 ++
tests/nodeinfodata/linux-x86-test1.expected | 2 +-
tests/nodeinfodata/linux-x86-test2.expected | 2 +-
tests/nodeinfodata/linux-x86-test3.expected | 2 +-
tests/nodeinfodata/linux-x86-test4.expected | 2 +-
tests/nodeinfodata/linux-x86-test5.expected | 2 +-
tests/nodeinfodata/linux-x86-test6.expected | 2 +-
tests/nodeinfodata/linux-x86-test7.expected | 2 +-
tests/nodeinfodata/linux-x86-test8.expected | 2 +-
tests/nodeinfotest.c | 11 +++++----
12 files changed, 47 insertions(+), 20 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 7be58ee..031937f 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -918,6 +918,7 @@ nodeGetCPUMap;
nodeGetCPUStats;
nodeGetFreeMemory;
nodeGetInfo;
+nodeGetInfoCores;
nodeGetMemoryParameters;
nodeGetMemoryStats;
nodeSetMemoryParameters;
diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 477104f..b21fc3a 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -90,7 +90,8 @@ freebsdNodeGetCPUCount(void)
/* NB, this is not static as we need to call it from the testsuite */
int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
const char *sysfs_dir,
- virNodeInfoPtr nodeinfo);
+ virNodeInfoPtr nodeinfo,
+ unsigned int *totalcores);
static int linuxNodeGetCPUStats(FILE *procstat,
int cpuNum,
@@ -228,12 +229,13 @@ CPU_COUNT(cpu_set_t *set)
static int
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2)
ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4)
-ATTRIBUTE_NONNULL(5)
+ATTRIBUTE_NONNULL(5) ATTRIBUTE_NONNULL(6)
virNodeParseNode(const char *node,
int *sockets,
int *cores,
int *threads,
- int *offline)
+ int *offline,
+ unsigned int *totalcores)
{
int ret = -1;
int processors = 0;
@@ -357,6 +359,7 @@ virNodeParseNode(const char *node,
continue;
core = CPU_COUNT(&core_maps[i]);
+ *totalcores += core;
if (core > *cores)
*cores = core;
}
@@ -376,7 +379,8 @@ cleanup:
int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
const char *sysfs_dir,
- virNodeInfoPtr nodeinfo)
+ virNodeInfoPtr nodeinfo,
+ unsigned int *totalcoresinfo)
{
char line[1024];
DIR *nodedir = NULL;
@@ -386,6 +390,7 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
int ret = -1;
char *sysfs_nodedir = NULL;
char *sysfs_cpudir = NULL;
+ unsigned int totalcores = 0;
nodeinfo->cpus = 0;
nodeinfo->mhz = 0;
@@ -503,7 +508,7 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
}
if ((cpus = virNodeParseNode(sysfs_cpudir, &socks, &cores,
- &threads, &offline)) < 0)
+ &threads, &offline, &totalcores)) <
0)
goto cleanup;
VIR_FREE(sysfs_cpudir);
@@ -538,8 +543,9 @@ fallback:
goto cleanup;
}
+ totalcores = 0;
if ((cpus = virNodeParseNode(sysfs_cpudir, &socks, &cores,
- &threads, &offline)) < 0)
+ &threads, &offline, &totalcores)) < 0)
goto cleanup;
nodeinfo->nodes = 1;
@@ -582,6 +588,9 @@ done:
nodeinfo->threads = 1;
}
+ if (totalcoresinfo)
+ *totalcoresinfo = totalcores;
+
ret = 0;
cleanup:
@@ -864,7 +873,10 @@ error:
}
#endif
-int nodeGetInfo(virConnectPtr conn ATTRIBUTE_UNUSED, virNodeInfoPtr nodeinfo)
+
+int
+nodeGetInfoCores(virNodeInfoPtr nodeinfo,
+ unsigned int *totalcores)
{
virArch hostarch = virArchFromHost();
@@ -881,7 +893,8 @@ int nodeGetInfo(virConnectPtr conn ATTRIBUTE_UNUSED, virNodeInfoPtr
nodeinfo)
return -1;
}
- ret = linuxNodeInfoCPUPopulate(cpuinfo, SYSFS_SYSTEM_PATH, nodeinfo);
+ ret = linuxNodeInfoCPUPopulate(cpuinfo, SYSFS_SYSTEM_PATH,
+ nodeinfo, totalcores);
if (ret < 0)
goto cleanup;
@@ -936,6 +949,14 @@ cleanup:
#endif
}
+
+int
+nodeGetInfo(virConnectPtr conn ATTRIBUTE_UNUSED, virNodeInfoPtr nodeinfo)
+{
+ return nodeGetInfoCores(nodeinfo, NULL);
+}
+
+
int nodeGetCPUStats(virConnectPtr conn ATTRIBUTE_UNUSED,
int cpuNum ATTRIBUTE_UNUSED,
virNodeCPUStatsPtr params ATTRIBUTE_UNUSED,
diff --git a/src/nodeinfo.h b/src/nodeinfo.h
index 350f3c3..02b136d 100644
--- a/src/nodeinfo.h
+++ b/src/nodeinfo.h
@@ -28,6 +28,8 @@
# include "capabilities.h"
int nodeGetInfo(virConnectPtr conn, virNodeInfoPtr nodeinfo);
+int nodeGetInfoCores(virNodeInfoPtr nodeinfo, unsigned int *totalcores);
+
int nodeCapsInitNUMA(virCapsPtr caps);
int nodeGetCPUStats(virConnectPtr conn,
diff --git a/tests/nodeinfodata/linux-x86-test1.expected
b/tests/nodeinfodata/linux-x86-test1.expected
index 4c86824..3136949 100644
--- a/tests/nodeinfodata/linux-x86-test1.expected
+++ b/tests/nodeinfodata/linux-x86-test1.expected
@@ -1 +1 @@
-CPUs: 2/2, MHz: 2800, Nodes: 1, Sockets: 1, Cores: 2, Threads: 1
+CPUs: 2/2, MHz: 2800, Nodes: 1, Sockets: 1, Cores: 2, Threads: 1, Total cores: 2
diff --git a/tests/nodeinfodata/linux-x86-test2.expected
b/tests/nodeinfodata/linux-x86-test2.expected
index 33bfbf3..34ab5f7 100644
--- a/tests/nodeinfodata/linux-x86-test2.expected
+++ b/tests/nodeinfodata/linux-x86-test2.expected
@@ -1 +1 @@
-CPUs: 2/2, MHz: 800, Nodes: 1, Sockets: 1, Cores: 2, Threads: 1
+CPUs: 2/2, MHz: 800, Nodes: 1, Sockets: 1, Cores: 2, Threads: 1, Total cores: 2
diff --git a/tests/nodeinfodata/linux-x86-test3.expected
b/tests/nodeinfodata/linux-x86-test3.expected
index 0306f86..85ca9e0 100644
--- a/tests/nodeinfodata/linux-x86-test3.expected
+++ b/tests/nodeinfodata/linux-x86-test3.expected
@@ -1 +1 @@
-CPUs: 48/48, MHz: 2100, Nodes: 8, Sockets: 1, Cores: 6, Threads: 1
+CPUs: 48/48, MHz: 2100, Nodes: 8, Sockets: 1, Cores: 6, Threads: 1, Total cores: 48
diff --git a/tests/nodeinfodata/linux-x86-test4.expected
b/tests/nodeinfodata/linux-x86-test4.expected
index 0c8f956..3f01271 100644
--- a/tests/nodeinfodata/linux-x86-test4.expected
+++ b/tests/nodeinfodata/linux-x86-test4.expected
@@ -1 +1 @@
-CPUs: 16/16, MHz: 1064, Nodes: 2, Sockets: 1, Cores: 8, Threads: 1
+CPUs: 16/16, MHz: 1064, Nodes: 2, Sockets: 1, Cores: 8, Threads: 1, Total cores: 16
diff --git a/tests/nodeinfodata/linux-x86-test5.expected
b/tests/nodeinfodata/linux-x86-test5.expected
index e63cf76..f5a2dfb 100644
--- a/tests/nodeinfodata/linux-x86-test5.expected
+++ b/tests/nodeinfodata/linux-x86-test5.expected
@@ -1 +1 @@
-CPUs: 4/4, MHz: 1861, Nodes: 1, Sockets: 1, Cores: 4, Threads: 1
+CPUs: 4/4, MHz: 1861, Nodes: 1, Sockets: 1, Cores: 4, Threads: 1, Total cores: 4
diff --git a/tests/nodeinfodata/linux-x86-test6.expected
b/tests/nodeinfodata/linux-x86-test6.expected
index 7ffcb8e..6d2e6a9 100644
--- a/tests/nodeinfodata/linux-x86-test6.expected
+++ b/tests/nodeinfodata/linux-x86-test6.expected
@@ -1 +1 @@
-CPUs: 6/8, MHz: 1596, Nodes: 1, Sockets: 1, Cores: 4, Threads: 2
+CPUs: 6/8, MHz: 1596, Nodes: 1, Sockets: 1, Cores: 4, Threads: 2, Total cores: 4
diff --git a/tests/nodeinfodata/linux-x86-test7.expected
b/tests/nodeinfodata/linux-x86-test7.expected
index e56360d..b32095e 100644
--- a/tests/nodeinfodata/linux-x86-test7.expected
+++ b/tests/nodeinfodata/linux-x86-test7.expected
@@ -1 +1 @@
-CPUs: 24/24, MHz: 2200, Nodes: 1, Sockets: 1, Cores: 24, Threads: 1
+CPUs: 24/24, MHz: 2200, Nodes: 1, Sockets: 1, Cores: 24, Threads: 1, Total cores: 12
diff --git a/tests/nodeinfodata/linux-x86-test8.expected
b/tests/nodeinfodata/linux-x86-test8.expected
index 124ed32..44ec1b5 100644
--- a/tests/nodeinfodata/linux-x86-test8.expected
+++ b/tests/nodeinfodata/linux-x86-test8.expected
@@ -1 +1 @@
-CPUs: 64/64, MHz: 2593, Nodes: 1, Sockets: 1, Cores: 64, Threads: 1
+CPUs: 64/64, MHz: 2593, Nodes: 1, Sockets: 1, Cores: 64, Threads: 1, Total cores: 64
diff --git a/tests/nodeinfotest.c b/tests/nodeinfotest.c
index d900eb9..076e28e 100644
--- a/tests/nodeinfotest.c
+++ b/tests/nodeinfotest.c
@@ -27,7 +27,8 @@ main(void)
extern int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
char *sysfs_dir,
- virNodeInfoPtr nodeinfo);
+ virNodeInfoPtr nodeinfo,
+ unsigned int *totalcores);
static int
linuxTestCompareFiles(const char *cpuinfofile,
@@ -39,6 +40,7 @@ linuxTestCompareFiles(const char *cpuinfofile,
char *expectData = NULL;
virNodeInfo nodeinfo;
FILE *cpuinfo;
+ unsigned int totalcores = 0;
if (virtTestLoadFile(outputfile, &expectData) < 0)
goto fail;
@@ -48,7 +50,8 @@ linuxTestCompareFiles(const char *cpuinfofile,
goto fail;
memset(&nodeinfo, 0, sizeof(nodeinfo));
- if (linuxNodeInfoCPUPopulate(cpuinfo, sysfs_dir, &nodeinfo) < 0) {
+ if (linuxNodeInfoCPUPopulate(cpuinfo, sysfs_dir,
+ &nodeinfo, &totalcores) < 0) {
if (virTestGetDebug()) {
virErrorPtr error = virSaveLastError();
if (error && error->code != VIR_ERR_OK)
@@ -62,10 +65,10 @@ linuxTestCompareFiles(const char *cpuinfofile,
if (virAsprintf(&actualData,
"CPUs: %u/%u, MHz: %u, Nodes: %u, Sockets: %u, "
- "Cores: %u, Threads: %u\n",
+ "Cores: %u, Threads: %u, Total cores: %u\n",
nodeinfo.cpus, VIR_NODEINFO_MAXCPUS(nodeinfo),
nodeinfo.mhz, nodeinfo.nodes, nodeinfo.sockets,
- nodeinfo.cores, nodeinfo.threads) < 0)
+ nodeinfo.cores, nodeinfo.threads, totalcores) < 0)
goto fail;
if (STRNEQ(actualData, expectData)) {
--
1.8.1
8:45 a.m.
New subject: [libvirt] [PATCH 2/2] capabilities: Add total number of cores and threads to the XML
Management applications may want to limit the maximum number of vCPUs
the guest has assigned based on the number of physical cores in the
system (excluding threads) for performance reasons.
This patch adds output of the total number of cores and total number of
threads present in the system as this information couldn't be reliably
determined in all cases by the data provided by libvirt.
The new output looks like this on a system with HyperThreading:
<capabilities>
<host>
<cpu>
<arch>x86_64</arch>
<model>SandyBridge</model>
<vendor>Intel</vendor>
<topology sockets='1' cores='2' threads='2'
totalcores='2' totalthreads='4'/>
---
docs/formatcaps.html.in | 2 +-
docs/schemas/capability.rng | 8 ++++++++
src/conf/cpu_conf.c | 4 ++++
src/conf/cpu_conf.h | 2 ++
src/qemu/qemu_capabilities.c | 3 ++-
tests/testutilsqemu.c | 2 ++
6 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/docs/formatcaps.html.in b/docs/formatcaps.html.in
index 9d42426..c16267c 100644
--- a/docs/formatcaps.html.in
+++ b/docs/formatcaps.html.in
@@ -23,7 +23,7 @@ BIOS you will see</p>
</features>
<model>core2duo</model>
<vendor>Intel</vendor>
- <topology sockets="1" cores="2"
threads="1"/>
+ <topology sockets="1" cores="2" threads="2"
totalcores="2" totalthreads="4" />
<feature name="lahf_lm"/>
<feature name='xtpr'/>
...
diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng
index 8c928bc..0fb7d6a 100644
--- a/docs/schemas/capability.rng
+++ b/docs/schemas/capability.rng
@@ -100,6 +100,14 @@
<attribute name='threads'>
<ref name='positiveInteger'/>
</attribute>
+ <optional>
+ <attribute name='totalthreads'>
+ <ref name='positiveInteger'/>
+ </attribute>
+ <attribute name='totalcores'>
+ <ref name='positiveInteger'/>
+ </attribute>
+ </optional>
</element>
<zeroOrMore>
<element name='feature'>
diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
index bdc5f1d..753a6dd 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -626,6 +626,10 @@ virCPUDefFormatBuf(virBufferPtr buf,
virBufferAsprintf(buf, " sockets='%u'", def->sockets);
virBufferAsprintf(buf, " cores='%u'", def->cores);
virBufferAsprintf(buf, " threads='%u'", def->threads);
+ if (def->totalcores) {
+ virBufferAsprintf(buf, " totalcores='%u'",
def->totalcores);
+ virBufferAsprintf(buf, " totalthreads='%u'",
def->totalthreads);
+ }
virBufferAddLit(buf, "/>\n");
}
diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h
index 23ea455..7a881a1 100644
--- a/src/conf/cpu_conf.h
+++ b/src/conf/cpu_conf.h
@@ -113,6 +113,8 @@ struct _virCPUDef {
unsigned int sockets;
unsigned int cores;
unsigned int threads;
+ unsigned int totalcores;
+ unsigned int totalthreads;
size_t nfeatures;
size_t nfeatures_max;
virCPUFeatureDefPtr features;
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index b166dd6..6920ea8 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -842,13 +842,14 @@ qemuCapsInitCPU(virCapsPtr caps,
cpu->arch = arch;
- if (nodeGetInfo(NULL, &nodeinfo))
+ if (nodeGetInfoCores(&nodeinfo, &cpu->totalcores))
goto error;
cpu->type = VIR_CPU_TYPE_HOST;
cpu->sockets = nodeinfo.sockets;
cpu->cores = nodeinfo.cores;
cpu->threads = nodeinfo.threads;
+ cpu->totalthreads = nodeinfo.cpus;
caps->host.cpu = cpu;
if (!(data = cpuNodeData(arch))
diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c
index 573927d..f95ba05 100644
--- a/tests/testutilsqemu.c
+++ b/tests/testutilsqemu.c
@@ -155,6 +155,8 @@ virCapsPtr testQemuCapsInit(void) {
1, /* sockets */
2, /* cores */
1, /* threads */
+ 2, /* totalcores */
+ 2, /* totalthreads */
ARRAY_CARDINALITY(host_cpu_features), /* nfeatures */
ARRAY_CARDINALITY(host_cpu_features), /* nfeatures_max */
host_cpu_features, /* features */
--
1.8.1
8:54 a.m.
New subject: [libvirt] [PATCH 2/2] capabilities: Add total number of cores and threads to the XML
On Tue, Jan 15, 2013 at 03:45:24PM +0100, Peter Krempa wrote:
Management applications may want to limit the maximum number of
vCPUs
the guest has assigned based on the number of physical cores in the
system (excluding threads) for performance reasons.
This patch adds output of the total number of cores and total number of
threads present in the system as this information couldn't be reliably
determined in all cases by the data provided by libvirt.
The new output looks like this on a system with HyperThreading:
<capabilities>
<host>
<cpu>
<arch>x86_64</arch>
<model>SandyBridge</model>
<vendor>Intel</vendor>
<topology sockets='1' cores='2' threads='2'
totalcores='2' totalthreads='4'/>
NACK, we should not be adding this data here - it is duplicating info
better provide in the NUMA <topology> hierarchy. Please explain why
this is not already sufficient ?
Regards,
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 :|
4330
days inactive
4330
days old
3 comments
2 participants
participants (2)
-
Daniel P. Berrange -
Peter Krempa