Commit Graph

43 Commits

Author SHA1 Message Date
Michal Privoznik
1c24cfe9d8 virNumaSetPagePoolSize: Produce friendlier error message
https://bugzilla.redhat.com/show_bug.cgi?id=1224587

The function takes two important arguments (among many others): @node
and @page_size. From these two a path under /sys is constructed. The
path is then used to read and write the desired size of huge pages
pool. However, if the path does not exists due to either @node or
@page_size having nonexistent value (e.g. there's no such NUMA node or
no page size like -2), an cryptic error message is produced:

  virsh # allocpages --pagesize 2049 --pagecount 8 --cellno -2
  error: Failed to open file '/sys/devices/system/node/node-2/hugepages/hugepages-2049kB/nr_hugepages': No such file or directory

Add two more checks to catch this and therefore produce much more
friendlier error messages.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2015-06-10 17:27:16 +02:00
Peter Krempa
8ad871f39e util: NUMA: Change error code in virNumaNodesetIsAvailable()
An invalid nodeset is a configuration problem rather than an internal
error.
2015-03-25 12:24:40 +01:00
Daniel P. Berrange
b38da58423 Make tests independant of system page size
Some code paths have special logic depending on the page size
reported by sysconf, which in turn affects the test results.
We must mock this so tests always have a consistent page size.
2015-02-02 20:27:43 +00:00
Martin Kletzander
d277d61420 util: Add virNumaGetHostNodeset
That function tries its best to create a bitmap of host NUMA nodes.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
2014-12-16 11:15:27 +01:00
Chen Fan
fc313364b1 virnuma: add nodeset NULL check in virNumaSetupMemoryPolicy
Introduced by commit c63ef0452b, when nodeset is NULL, validation will
pass in virNumaSetupMemoryPolicy, but virBitmapNextSetBit must ensure
bitmap is not NULL, otherwise that might cause a segmentation fault.
This patch fixes it.

Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
2014-11-07 12:04:57 +01:00
Martin Kletzander
877a222449 numa: fix assumption in virNumaNodeIsAvailable()
When compiled without full numa support, the stub function for
virNumaNodeIsAvailable() just checks whether specified node is in range
<0, max); where max is maximum NUMA node available on the host.  But
because the maximum node number is the highest usabe number (and not the
count of nodes), the check is incorrect as it should check whether the
specified node is in range <0, max> instead.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
2014-11-06 15:13:55 +01:00
Martin Kletzander
c63ef0452b numa: split util/ and conf/ and support non-contiguous nodesets
This is a reaction to Michal's fix [1] for non-NUMA systems that also
splits out conf/ out of util/ because libvirt_util shouldn't require
libvirt_conf if it is the other way around.  This particular use case
worked, but we're trying to avoid it as mentioned [2], many times.

The only functions from virnuma.c that needed numatune_conf were
virDomainNumatuneNodesetIsAvailable() and virNumaSetupMemoryPolicy().
The first one should be in numatune_conf as it works with
virDomainNumatune, the second one just needs nodeset and mode, both of
which can be passed without the need of numatune_conf.

Apart from fixing that, this patch also fixes recently added
code (between commits d2460f85^..5c8515620) that doesn't support
non-contiguous nodesets.  It uses new function
virNumaNodesetIsAvailable(), which doesn't need a stub as it doesn't use
any libnuma functions, to check if every specified nodeset is available.

[1] https://www.redhat.com/archives/libvir-list/2014-November/msg00118.html
[2] http://www.redhat.com/archives/libvir-list/2011-June/msg01040.html

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
2014-11-06 15:13:55 +01:00
Michal Privoznik
dda1a235c1 virnuma: Add some more comments
Well, in fact only add comments to mark ifdef,
else and endif sections.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2014-11-06 09:08:47 +01:00
Chen Fan
5c8515620b virnuma: use virNumaNodesetIsAvailable checking nodeset in virNumaSetupMemoryPolicy
Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
2014-11-04 07:03:36 +01:00
Chen Fan
902864184e numatune: add check for numatune nodeset range
There was no check for 'nodeset' attribute in numatune-related
elements.  This patch adds validation that any nodeset specified does
not exceed maximum host node.

Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
2014-11-04 07:03:36 +01:00
Michal Privoznik
d1fd4a9b3b virnuma: Introduce virNumaSetPagePoolSize
This internal API can be used to allocate or free some pages in
the huge pages pool.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2014-09-25 10:24:45 +02:00
Michal Privoznik
f4c87a0c35 nodeCapsInitNUMA: Avoid @cpumap leak
In case the host has 2 or more NUMA nodes, we fetch CPU map for each
node. However, we need to free the CPU map in between loops:

==29513== 96 (72 direct, 24 indirect) bytes in 3 blocks are definitely lost in loss record 951 of 1,264
==29513==    at 0x4C2A700: calloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==29513==    by 0x52AD24B: virAlloc (viralloc.c:144)
==29513==    by 0x52AF0E6: virBitmapNew (virbitmap.c:78)
==29513==    by 0x52FB720: virNumaGetNodeCPUs (virnuma.c:294)
==29513==    by 0x53C700B: nodeCapsInitNUMA (nodeinfo.c:1886)
==29513==    by 0x11759708: vboxCapsInit (vbox_common.c:398)
==29513==    by 0x11759CC4: vboxConnectOpen (vbox_common.c:514)
==29513==    by 0x53C965F: do_open (libvirt.c:1147)
==29513==    by 0x53C9EBC: virConnectOpen (libvirt.c:1317)
==29513==    by 0x142905: remoteDispatchConnectOpen (remote.c:1215)
==29513==    by 0x126ADF: remoteDispatchConnectOpenHelper (remote_dispatch.h:2346)
==29513==    by 0x5453D21: virNetServerProgramDispatchCall (virnetserverprogram.c:437)

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2014-08-20 16:15:00 +02:00
Eric Blake
b50c8603a2 maint: improve syntax check for space around =
Laine Stump noted on IRC that syntax check wasn't flagging his
typo of 'i= 0'.  This fixes it.

* build-aux/bracket-spacing.pl: Tighten 'space around =' rule.
* src/storage/storage_backend.c
(virStorageBackendCreateExecCommand): Fix offenders.
* src/util/virnuma.c (virNumaGetDistances): Likewise.
* src/vbox/vbox_tmpl.c (vboxDomainSnapshotDeleteMetadataOnly)
(vboxNetworkGetXMLDesc): Likewise.
* src/xenapi/xenapi_driver.c (xenapiDomainLookupByName):
Likewise.

Signed-off-by: Eric Blake <eblake@redhat.com>
2014-08-12 11:21:17 -06:00
Eric Blake
72823b4443 build: fix build without numactl
Under ./configure --without-numactl but with numactl-devel installed,
the build fails with:

../../src/util/virnuma.c: In function 'virNumaNodeIsAvailable':
../../src/util/virnuma.c:407:5: error: implicit declaration of function 'numa_bitmask_isbitset' [-Werror=implicit-function-declaration]
     return numa_bitmask_isbitset(numa_nodes_ptr, node);
     ^

and other failures, all because the configure results for particular
functions were used without regard to whether libnuma was even being
linked in.

* src/util/virnuma.c (virNumaGetPages): Fix message typo.
(virNumaNodeIsAvailable): Correct build when not using numactl.

Signed-off-by: Eric Blake <eblake@redhat.com>
2014-07-21 12:50:00 -06:00
Martin Kletzander
1a7be8c600 numatune: add support for per-node memory bindings in private APIs
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
2014-07-16 20:15:45 +02:00
Martin Kletzander
93e82727ec numatune: Encapsulate numatune configuration in order to unify results
There were numerous places where numatune configuration (and thus
domain config as well) was changed in different ways.  On some
places this even resulted in persistent domain definition not to be
stable (it would change with daemon's restart).

In order to uniformly change how numatune config is dealt with, all
the internals are now accessible directly only in numatune_conf.c and
outside this file accessors must be used.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
2014-07-16 20:15:45 +02:00
Martin Kletzander
e764ec7ae3 numatune: unify numatune struct and enum names
Since there was already public virDomainNumatune*, I changed the
private virNumaTune to match the same, so all the uses are unified and
public API is kept:

s/vir\(Domain\)\?Numa[tT]une/virDomainNumatune/g

then shrunk long lines, and mainly functions, that were created after
that:

sed -i 's/virDomainNumatuneMemPlacementMode/virDomainNumatunePlacement/g'

And to cope with the enum name, I haad to change the constants as
well:

s/VIR_NUMA_TUNE_MEM_PLACEMENT_MODE/VIR_DOMAIN_NUMATUNE_PLACEMENT/g

Last thing I did was at least a little shortening of already long
name:

s/virDomainNumatuneDef/virDomainNumatune/g

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
2014-07-16 20:15:45 +02:00
Martin Kletzander
293d5f21b6 numatune: create new module for numatune
There are many places with numatune-related code that should be put
into special numatune_conf and this patch creates a basis for that.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
2014-07-16 20:15:45 +02:00
Michal Privoznik
2c78a270da virNumaGetPages: Don't fail on huge page-less systems
If we are running on a system that is not capable of huge pages (e.g.
because the kernel is not configured that way) we still try to open
"/sys/kernel/mm/hugepages/" which however does not exist. We should
be tolerant to this specific use case.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2014-06-24 11:50:32 +02:00
Michal Privoznik
3499eedd4b virNumaGetPageInfo: Take huge pages into account
On the Linux kernel, if huge pages are allocated the size they cut off
from memory is accounted under the 'MemUsed' in the meminfo file.
However, we want the sum to be subtracted from 'MemTotal'. This patch
implements this feature. After this change, we can enable reporting
of the ordinary system pages in the capability XML:

<capabilities>

  <host>
    <uuid>01281cda-f352-cb11-a9db-e905fe22010c</uuid>
    <cpu>
      <arch>x86_64</arch>
      <model>Haswell</model>
      <vendor>Intel</vendor>
      <topology sockets='1' cores='1' threads='1'/>
      <feature/>
      <pages unit='KiB' size='4'/>
      <pages unit='KiB' size='2048'/>
      <pages unit='KiB' size='1048576'/>
    </cpu>
    <power_management/>
    <migration_features/>
    <topology>
      <cells num='4'>
        <cell id='0'>
          <memory unit='KiB'>4048248</memory>
          <pages unit='KiB' size='4'>748382</pages>
          <pages unit='KiB' size='2048'>3</pages>
          <pages unit='KiB' size='1048576'>1</pages>
          <distances/>
          <cpus num='1'>
            <cpu id='0' socket_id='0' core_id='0' siblings='0'/>
          </cpus>
        </cell>
        ...
      </cells>
    </topology>
  </host>
</capabilities>

You can see the beautiful thing about this: if you sum up all the
<pages/> you'll get <memory/>.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2014-06-24 11:50:31 +02:00
Roman Bogorodskiy
4a82ef92be Fix closedir usage in virNumaGetPages
virNumaGetPages calls closedir(dir) in cleanup and dir could
be NULL if we jump there from the failed opendir() call.

While it's not harmful on Linux, FreeBSD libc crashes [1], so
make sure that dir is not NULL before calling closedir.

1: http://lists.freebsd.org/pipermail/freebsd-standards/2014-January/002704.html
2014-06-23 19:25:59 +04:00
Michal Privoznik
9e72691b7e virnuma: Actually build huge page code
One of previous commits (e6258a33) tried to build the huge page code
only on Linux since it's Linux centric indeed. But it failed miserably
as it used 'WITH_LINUX' which is an automake conditional not a gcc
one. In the sources we need to use __linux__.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2014-06-23 16:24:02 +02:00
Ján Tomko
9a2e523eba Fix invalid write in virNumaGetDistances
==== Invalid write of size 4
====    at 0x52E678C: virNumaGetDistances (virnuma.c:479)
====    by 0x5396890: nodeCapsInitNUMA (nodeinfo.c:1796)
====    by 0x203C2B: virQEMUCapsInit (qemu_capabilities.c:960)
====  Address 0xe10a1e0 is 0 bytes after a block of size 0 alloc'd
====    at 0x4C2A6D0: calloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
====    by 0x52A10D6: virAllocN (viralloc.c:191)
====    by 0x52E674D: virNumaGetDistances (virnuma.c:470)
====    by 0x5396890: nodeCapsInitNUMA (nodeinfo.c:1796)
====    by 0x203C2B: virQEMUCapsInit (qemu_capabilities.c:960)
2014-06-23 14:16:23 +02:00
Peter Krempa
e6258a333c util: numa: Stub out hugepage code on non-Linux platforms
The hugepage sizing and counting code gathers the information from sysfs
and thus isn't portable. Stub it out for non-Linux so that we can report
a better error. This patch also avoids calling sysinfo() on Mingw where
it isn't supported.
2014-06-23 14:07:57 +02:00
Peter Krempa
00b2317a66 util: numa: Catch readdir errors in virNumaGetPages
Don't return possibly incomplete result if virDirRead fails.
2014-06-23 14:00:10 +02:00
Michal Privoznik
35f1095e12 virnuma: Introduce pages helpers
For future work we need two functions that fetches total number of
pages and number of free pages for given NUMA node and page size
(virNumaGetPageInfo()).

Then we need to learn pages of what sizes are supported on given node
(virNumaGetPages()).

Note that system page size is disabled at the moment as there's one
issue connected. If you have a NUMA node with huge pages allocated the
kernel would return the normal size of memory for that node. It
basically ignores the fact that huge pages steal size from the system
memory. Until we resolve this, it's safer to not confuse users and
hence not report any system pages yet.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2014-06-19 15:10:49 +02:00
Michal Privoznik
356c6f389f virnuma: Introduce virNumaNodeIsAvailable
Not on all hosts the set of NUMA nodes IDs is continuous. This is
critical, because our code currently assumes the set doesn't contain
holes. For instance in nodeGetFreeMemory() we can see the following
pattern:

    if ((max_node = virNumaGetMaxNode()) < 0)
        return 0;

    for (n = 0; n <= max_node; n++) {
        ...
    }

while it should be something like this:

    if ((max_node = virNumaGetMaxNode()) < 0)
        return 0;

    for (n = 0; n <= max_node; n++) {
        if (!virNumaNodeIsAvailable(n))
            continue;
        ...
    }

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2014-06-19 15:10:49 +02:00
Michal Privoznik
9e0cbcd888 virnuma: Check for numa_bitmask_isbitset presence
On some systems, libnuma can be present but it's so ancient that
it misses some symbols that virNumaGetDistances() needs. To be
more precise: numa_bitmask_isbitset() and numa_nodes_ptr are the
symbols in question. Fortunately, they were both introduced in
the same release so it's sufficient for us to check for only one
of them. And the winner is numa_bitmask_isbitset().

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2014-06-04 16:06:51 +02:00
Michal Privoznik
3289edd224 virnuma: Implement virNumaGetDistances stub for non-NUMA
In case the libvirt is built without numactl support, we're
missing the virNumaGetDistances() stub so the linking fails:

  CCLD     libvirt_lxc
libvirt_lxc-nodeinfo.o: In function `virNodeCapsGetSiblingInfo':
/home/zippy/tmp/libvirt.git/src/nodeinfo.c:1763: undefined reference to `virNumaGetDistances'
collect2: error: ld returned 1 exit status
make[3]: *** [libvirt_lxc] Error 1

The issue was introduced in 77c830d8c4.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2014-06-04 11:09:11 +02:00
Michal Privoznik
77c830d8c4 virnuma: Introduce virNumaGetDistances
The API gets a NUMA node and find distances to other nodes.  The
distances are returned in an array. If an item X within the array
equals to value of zero, then there's no such node as X.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2014-06-04 08:54:16 +02:00
Michal Privoznik
3dd23b6106 virnuma.c: Fix some comments
In 9dd02965 the virNumaGetNodeMemory was introduced, however the
comment describing the function mentions virNumaGetNodeMemorySize.
And there's one typo in virNumaIsAvailable() description.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2014-06-03 08:58:24 +02:00
Ján Tomko
bada4222e5 Indent top-level labels by one space in src/util/ 2014-03-25 14:58:40 +01:00
Daniel P. Berrange
2835c1e730 Add virLogSource variables to all source files
Any source file which calls the logging APIs now needs
to have a VIR_LOG_INIT("source.name") declaration at
the start of the file. This provides a static variable
of the virLogSource type.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
2014-03-18 14:29:22 +00:00
Osier Yang
ae2860b4c6 util: Correct the NUMA node range checking
There are 2 issues here: First we shouldn't add "1" to the return
value of numa_max_node(), since the semanteme of the error message
was changed, it's not saying about the number of total NUMA nodes
anymore.  Second, the value of "bit" is the position of the first
bit which exceeds either numa_max_node() or NUMA_NUM_NODES, it can
be any number in the range, so saying "bigger than $bit" is quite
confused now. For example, assuming there is a NUMA machine which
has 10 NUMA nodes, and one specifies the "nodeset" as "0,5,88",
the error message will be like:

Nodeset is out of range, host cannot support NUMA node bigger than 88

It sounds like all NUMA node number less than 88 is fine, but
actually the maximum NUMA node number the machine supports is 9.

This patch fixes the issues by removing the addition with "1" and
simplifies the error message as "NUMA node $bit is out of range".
Also simplifies the comparision in the while loop by getting the
smaller one of numa_max_node() and NUMA_NUM_NODES up front.
2014-01-23 13:19:56 +08:00
Peter Krempa
223521bd33 numa: Add wrapper of numa_node_to_cpus and use it 2013-11-04 10:50:20 +01:00
Peter Krempa
15ca990229 numa: Replace NUMA_MAX_N_CPUS macro with virNumaGetMaxCPUs() 2013-11-04 10:50:20 +01:00
Peter Krempa
9dd02965a5 numa: Introduce virNumaGetNodeMemory and use it instead of numa_node_size64 2013-11-04 10:50:19 +01:00
Peter Krempa
f72cfea1ab numa: Introduce virNumaGetMaxNode and use it instead of numa_max_node
Avoid necessary checks for the numa library with this helper.
2013-11-04 10:50:19 +01:00
Peter Krempa
f8ee8fe3f9 numa: Introduce virNumaIsAvailable and use it instead of numa_available
All functions from libnuma must be protected with ifdefs. Avoid this by
using our own wrapper.
2013-11-04 10:48:00 +01:00
Daniel P. Berrange
f8b42f3224 Convert 'int i' to 'size_t i' in src/util/ files
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>
2013-07-10 17:40:13 +01:00
Osier Yang
148edcce66 util: Error out if the numa nodeset is out of range
Instead of a silent warning, it's better to error out if the
numa nodeset is out of range. Just like for numa node larger
than NUMA_NUM_NODES.
2013-04-24 23:23:31 +08:00
Gao feng
45e9d27ad8 NUMA: cleanup for numa related codes
Intend to reduce the redundant code,use virNumaSetupMemoryPolicy
to replace virLXCControllerSetupNUMAPolicy and
qemuProcessInitNumaMemoryPolicy.

This patch also moves the numa related codes to the
file virnuma.c and virnuma.h

Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
2013-03-20 19:37:00 +08:00
Gao feng
763edb5ebe rename qemuGetNumadAdvice to virNumaGetAutoPlacementAdvice
qemuGetNumadAdvice will be used by LXC driver, rename
it to virNumaGetAutoPlacementAdvice and move it to virnuma.c

Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
2013-03-19 15:55:40 -06:00