Re-arrange the code so that the returned bitmap is always initialized to
NULL even on early failures and return an error message as some callers
are already expecting it. Fix up the rest not to shadow the error.
nodeGetFreeMemory and nodeGetCellsFreeMemory assumed that the NUMA nodes
are contiguous and starting from 0. Unfortunately there are machines
that don't match this assumption:
available: 1 nodes (1)
node 1 cpus: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15
node 1 size: 16340 MB
node 1 free: 11065 MB
Before this patch:
error: internal error Failed to query NUMA free memory
error: internal error Failed to query NUMA free memory for node: 0
After this patch:
Total: 15772580 KiB
0: 0 KiB
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Convert the type of loop iterators named 'i', 'j', k',
'ii', 'jj', 'kk', to be 'size_t' instead of 'int' or
'unsigned int', also santizing 'ii', 'jj', 'kk' to use
the normal 'i', 'j', 'k' naming
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The individual hypervisor drivers were directly referencing
APIs in src/nodeinfo.c in their virDriverPtr struct. Separate
these methods, so there is always a wrapper in the hypervisor
driver. This allows the unused virConnectPtr args to be
removed from the nodeinfo.c file. Again this will ensure that
ACL checks will only be performed on invocations that are
directly associated with public API usage.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The source code base needs to be adapted as well. Some files
include virutil.h just for the string related functions (here,
the include is substituted to match the new file), some include
virutil.h without any need (here, the include is removed), and
some require both.
If libnuma is not compiled in, or numa_available() returns an
error, stub out fake NUMA info consisting of one NUMA cell
containing all CPUs and memory.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Not all kernel builds have any entries under the location
/sys/devices/system/cpu/cpu0/topology. We already cope with
that being missing in some cases, but not all. Update the
code which looks for thread_siblings to cope with the missing
file
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
'virsh capabilities' will now include a new <memory> element
per <cell> of the topology, as in:
<topology>
<cells num='2'>
<cell id='0'>
<memory unit='KiB'>12572412</memory>
<cpus num='12'>
...
</cell>
Signed-off-by: Eric Blake <eblake@redhat.com>
When linuxNodeInfoCPUPopulate() method triggered use of an
uninitialize value, since it did not initialize the 'sockets'
field in the virNodeInfoPtr struct:
==30020== Conditional jump or move depends on uninitialised value(s)
==30020== at 0x5125DBD: linuxNodeInfoCPUPopulate (nodeinfo.c:513)
==30020== by 0x51261A0: nodeGetInfo (nodeinfo.c:884)
==30020== by 0x149B9B10: qemuCapsInit (qemu_capabilities.c:846)
==30020== by 0x14A11B25: qemuCreateCapabilities (qemu_driver.c:424)
==30020== by 0x14A12426: qemuStartup (qemu_driver.c:874)
==30020== by 0x512A7AF: virStateInitialize (libvirt.c:822)
==30020== by 0x40DE04: daemonRunStateInit (libvirtd.c:877)
==30020== by 0x50ADCE5: virThreadHelper (virthreadpthread.c:161)
==30020== by 0x328CA07D14: start_thread (pthread_create.c:308)
==30020== by 0x328C6F246C: clone (clone.S:114)
(happened twice)
if (socks > nodeinfo->sockets) <--- here
nodeinfo->sockets = socks;
Rather than doing this for each field, just make the caller memset
the entire struct to zero.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
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.
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.
The use of switch statements inside a bounded for loop resulted in some
false positives regarding the "default:" label which cannot be reached
since each of the other case statements use the possible for loop values.
A [dead_error_begin] was added before the default label.
Commit id ebdbe25a adjusted the algorithm and the caller guarantees that
the 'params' will have a '_' in the name being searched. Add the [returned_null]
tag to the two instances.
Replace use of uname in nodeGetInfo with virArch APIs to
provide canonicalization of host architecture name
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
It makes no sense to fail the whole getting command if there is
a parameter unsupported by the kernel. This patch fixes it by
omitting the unsupported parameter for getMemoryParameters.
And for setMemoryParameters, this checks if there is an unsupported
parameter up front of the setting, and just returns failure if not
all parameters are supported.
Prior to this patch, 'virsh nodecpumap' on older kernels reported:
error: Unable to get cpu map
error: out of memory
* src/nodeinfo.c (linuxParseCPUmax): Don't overwrite error.
(nodeGetCPUBitmap): Provide backup implementation.
On RHEL 5, I was getting a segfault trying to start libvirtd,
because we were failing virNodeParseSocket but not checking
for errors, and then calling CPU_SET(-1, &sock_map) as a result.
But if you don't have a topology/physical_package_id file,
then you can just assume that the cpu belongs to socket 0.
* src/nodeinfo.c (virNodeGetCpuValue): Change bool into
default_value.
(virNodeParseSocket): Allow for default value when file is missing,
different from fatal error on reading file.
(virNodeParseNode): Update call sites to fail on error.
Since /sys/devices/system/cpu/present is not available on
older kernels like on RHEL 5.x nodeGetCPUCount will
fail there. The fallback implemented is to scan for
/sys/devices/system/cpu/cpuNN entries.
Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
Lately there were a few reports of the output of the virsh nodeinfo
command being inaccurate. This patch tries to avoid that by checking if
the topology actually makes sense. If it doesn't we then report a
synthetic topology that indicates to the user that the host capabilities
should be checked for the actual topology.
The libvirt coding standard is to use 'function(...args...)'
instead of 'function (...args...)'. A non-trivial number of
places did not follow this rule and are fixed in this patch.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
It turns out that calling virNodeGetCPUMap(conn, NULL, NULL, 0)
is both useful, and with Viktor's patches, common enough to
optimize. Since this interface hasn't been released yet, we
can change the RPC call.
A bit more background on the optimization - learning the cpu count
is a single file read (/sys/devices/system/cpu/possible), but
learning the number of online cpus can possibly trigger a file
read per cpu, depending on the age of the kernel, and all wasted
if the caller passed NULL for both arguments.
* src/nodeinfo.c (nodeGetCPUMap): Avoid bitmap when not needed.
* src/remote/remote_protocol.x (remote_node_get_cpu_map_args):
Supply two separate flags for needed arguments.
* src/remote/remote_driver.c (remoteNodeGetCPUMap): Update
caller.
* daemon/remote.c (remoteDispatchNodeGetCPUMap): Likewise.
* src/remote_protocol-structs: Regenerate.
Ever since commit eefb881, ATTRIBUTE_NONNULL has normally been a
no-op under gcc (since it tends to cause more bugs than it cures
given gcc's current lame implementation of the attribute). However,
the macro is still useful to Coverity and other static-analysis
tools, but only if we use it correctly. Coverity follows gcc's lead
in accepting function declarations with attributes at the end, but
function bodies must attach attributes to the return type. That is,
these are valid:
void foo(void *arg) ATTRIBUTE_NONNULL(1);
void ATTRIBUTE_NONNULL(1) foo(void *arg);
void ATTRIBUTE_NONNULL(1) foo(void *arg) {}
but this is not:
void foo(void *arg) ATTRIBUTE_NONNULL(1) {}
even though you don't get a compile failure until you do static
analysis. Bug introduced in commit 80533ca, with these symptoms:
nodeinfo.c:206: error: expected ',' or ';' before '{' token
cc1: warning: unrecognized command line option "-Wno-suggest-attribute=const"
cc1: warning: unrecognized command line option "-Wno-suggest-attribute=pure"
make[3]: *** [libvirt_driver_la-nodeinfo.lo] Error 1
* src/nodeinfo.c (virNodeParseNode): Fix syntax error when
non-null attribute is in use.
It turns out that the cpuacct results properly account for offline
cpus, and always returns results for every possible cpu, not just
the online ones. So there is no need to check the map of online
cpus in the first place, merely only a need to know the maximum
possible cpu. Meanwhile, virNodeGetCPUBitmap had a subtle change
from returning the maximum id to instead returning the width of
the bitmap (one larger than the maximum id) in commit 2f4c5338,
which made this code encounter some off-by-one logic leading to
bad error messages when a cpu was offline:
$ virsh cpu-stats dom
error: Failed to virDomainGetCPUStats()
error: An error occurred, but the cause is unknown
Cleaning this up unraveled a chain of other unused variables.
* src/qemu/qemu_driver.c (qemuDomainGetPercpuStats): Drop
pointless check for cpumap changes, and use correct number of
cpus. Simplify signature.
(qemuDomainGetCPUStats): Adjust caller.
* src/nodeinfo.h (nodeGetCPUCount): New prototype.
(nodeGetCPUBitmap): Drop unused parameter.
* src/nodeinfo.c (nodeGetCPUBitmap): Likewise.
(nodeGetCPUMap): Adjust caller.
(nodeGetCPUCount): New function.
* src/libvirt_private.syms (nodeinfo.h): Export it.
Added an implemention of virNodeGetCPUMap to nodeinfo.c,
(nodeGetCPUMap) which can be used by all drivers for a Linux
hypervisor host.
Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
Callers should not need to know what the name of the file to
be read in the Linux-specific version of nodeGetCPUmap;
furthermore, qemu cares about online cpus, not present cpus,
when determining which cpus to skip.
While at it, I fixed the fact that we were computing the maximum
online cpu id by doing a slow iteration, when what we really want
to know is the max available cpu.
* src/nodeinfo.h (nodeGetCPUmap): Rename...
(nodeGetCPUBitmap): ...and simplify signature.
* src/nodeinfo.c (linuxParseCPUmax): New function.
(linuxParseCPUmap): Simplify and alter signature.
(nodeGetCPUBitmap): Change implementation.
* src/libvirt_private.syms (nodeinfo.h): Reflect rename.
* src/qemu/qemu_driver.c (qemuDomainGetPercpuStats): Update
caller.
Upstream kernel introduced new sysfs knob "merge_across_nodes" to
specify if pages from different numa nodes can be merged. When set
to 0, only pages which physically reside in the memory area of
same NUMA node can be merged. When set to 1, pages from all nodes
can be merged.
This patch supports the tuning by adding new param field
"shm_merge_across_nodes".
https://www.gnu.org/licenses/gpl-howto.html recommends that
the 'If not, see <url>.' phrase be a separate sentence.
* tests/securityselinuxhelper.c: Remove doubled line.
* tests/securityselinuxtest.c: Likewise.
* globally: s/; If/. If/
Commit aaa8ab3 added new static functions that are only used on Linux;
but commit 22acfdc didn't go far enough to fix compiler issues.
* src/nodeinfo.c (nodeSetMemoryParameterValue)
(nodeGetMemoryParameterValue): Conditionally compile based on use.
Only implemented for linux platform.
* src/nodeinfo.h: (Declare node{Get,Set}MemoryParameters)
* src/nodeinfo.c: (Implement node{Get,Set}MemoryParameters)
* src/libvirt_private.syms: (Export those two new internal APIs to
private symbols)
Building on RHEL 5 warned:
nodeinfo.c: 305: warning: implicit declaration of function 'CPU_COUNT'
This extension macro in <sched.h> was not added until later glibc.
* src/nodeinfo.c (CPU_COUNT): Add fallback implementation.
Per the FSF address could be changed from time to time, and GNU
recommends the following now: (http://www.gnu.org/licenses/gpl-howto.html)
You should have received a copy of the GNU General Public License
along with Foobar. If not, see <http://www.gnu.org/licenses/>.
This patch removes the explicit FSF address, and uses above instead
(of course, with inserting 'Lesser' before 'General').
Except a bunch of files for security driver, all others are changed
automatically, the copyright for securify files are not complete,
that's why to do it manually:
src/security/security_selinux.h
src/security/security_driver.h
src/security/security_selinux.c
src/security/security_apparmor.h
src/security/security_apparmor.c
src/security/security_driver.c
Update the nodeinfo helper code to use virReportError instead
of the nodeReportError custom macro
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Commit 80533ca forgot to think about offline cpus. When a node
cpu is offline, then its topology/ subdirectory is not present,
leading to spurious error messages leaked to the user such as:
libvir: error : cannot open /home/dummy/libvirt/tests/nodeinfodata/linux-nodeinfo-sysfs-test-6/node/node0/cpu7/topology/physical_package_id: No such file or directory
Fix that, as well as test it; the test data is gathered from a
machine with one NUMA node, hyperthreading, and with 2 of the
8 cpus offline.
* src/nodeinfo.c (virNodeParseNode): Don't parse topology of
offline cpus.
* tests/nodeinfotest.c (mymain): Run new test.
* tests/nodeinfodata/linux-nodeinfo-sysfs-test-6*: New data.
This patch changes the way data to fill the nodeinfo structure are
gathered. We've gathere the test data by iterating processors an sockets
separately from nodes. The reported data was based solely on information
about core id. Problems arise when eg cores in mulit-processor machines
don't have same id's on both processors or maybe one physical processor
contains more NUMA nodes.
This patch changes the approach how we detect processors and nodes. Now
we start at enumerating nodes and for each node processors, sockets and
threads are enumerated separately. This approach provides acurate data
that comply to docs about the nodeinfo structure. This also enables to
get rid of hacks: see commits 10d9038b744a69c8d4bd29c2e8c012a097481586,
ac9dd4a676f21b5e3ca6dbe0526f2a6709072beb. (Those changes in nodeinfo.c
are efectively reverted by this patch).
This patch also changes output of one of the tests, as the processor
topology is now acquired more precisely.
