[libvirt] [PATCH 0/2] Fix vcpu hotplug with memoryless NUMA nodes

See patch 2. Peter Krempa (2): util: numa: Remove impossible error handling numa: Rename virNumaGetHostNodeset and make it return only nodes with memory src/libvirt_private.syms | 2 +- src/qemu/qemu_cgroup.c | 4 ++-- src/util/virnuma.c | 18 +++++++++++------- src/util/virnuma.h | 2 +- 4 files changed, 15 insertions(+), 11 deletions(-) -- 2.10.0

The code guarantees that virBitmapSetBit won't be called with out of range values. Just ignore the return value and remove dead error handling. --- src/util/virnuma.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/util/virnuma.c b/src/util/virnuma.c index fc25051..c4d11fa 100644 --- a/src/util/virnuma.c +++ b/src/util/virnuma.c @@ -1004,12 +1004,7 @@ virNumaGetHostNodeset(void) if (!virNumaNodeIsAvailable(i)) continue; - if (virBitmapSetBit(nodeset, i) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Problem setting bit in bitmap")); - virBitmapFree(nodeset); - return NULL; - } + ignore_value(virBitmapSetBit(nodeset, i)); } return nodeset; -- 2.10.0

Name it virNumaGetHostMemoryNodeset and return only NUMA nodes which have memory installed. This is necessary as the kernel is not very happy to set the memory cgroup setting for nodes which do not have any memory. This would break vcpu hotplug with following message on such configruation: Invalid value '0,8' for 'cpuset.mems': Invalid argument Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1375268 --- src/libvirt_private.syms | 2 +- src/qemu/qemu_cgroup.c | 4 ++-- src/util/virnuma.c | 13 +++++++++++-- src/util/virnuma.h | 2 +- 4 files changed, 15 insertions(+), 6 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 2569772..80d5e86 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2059,7 +2059,7 @@ virNodeSuspendGetTargetMask; # util/virnuma.h virNumaGetAutoPlacementAdvice; virNumaGetDistances; -virNumaGetHostNodeset; +virNumaGetHostMemoryNodeset; virNumaGetMaxNode; virNumaGetNodeMemory; virNumaGetPageInfo; diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index fe94613..4bce601 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -860,7 +860,7 @@ qemuRestoreCgroupState(virDomainObjPtr vm) virBitmapPtr all_nodes; virCgroupPtr cgroup_temp = NULL; - if (!(all_nodes = virNumaGetHostNodeset())) + if (!(all_nodes = virNumaGetHostMemoryNodeset())) goto error; if (!(mem_mask = virBitmapFormat(all_nodes))) @@ -1166,7 +1166,7 @@ qemuCgroupEmulatorAllNodesAllow(virCgroupPtr cgroup, !virCgroupHasController(cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) return 0; - if (!(all_nodes = virNumaGetHostNodeset())) + if (!(all_nodes = virNumaGetHostMemoryNodeset())) goto cleanup; if (!(all_nodes_str = virBitmapFormat(all_nodes))) diff --git a/src/util/virnuma.c b/src/util/virnuma.c index c4d11fa..bebe301 100644 --- a/src/util/virnuma.c +++ b/src/util/virnuma.c @@ -987,10 +987,17 @@ virNumaNodesetIsAvailable(virBitmapPtr nodeset) return true; } + +/** + * virNumaGetHostMemoryNodeset: + * + * Returns a bitmap of guest numa node ids that contain memory. + */ virBitmapPtr -virNumaGetHostNodeset(void) +virNumaGetHostMemoryNodeset(void) { int maxnode = virNumaGetMaxNode(); + unsigned long long nodesize; size_t i = 0; virBitmapPtr nodeset = NULL; @@ -1004,7 +1011,9 @@ virNumaGetHostNodeset(void) if (!virNumaNodeIsAvailable(i)) continue; - ignore_value(virBitmapSetBit(nodeset, i)); + /* if we can't detect NUMA node size assume that it's present */ + if (virNumaGetNodeMemory(i, &nodesize, NULL) < 0 || nodesize > 0) + ignore_value(virBitmapSetBit(nodeset, i)); } return nodeset; diff --git a/src/util/virnuma.h b/src/util/virnuma.h index 1f3c0ad..f3eef32 100644 --- a/src/util/virnuma.h +++ b/src/util/virnuma.h @@ -33,7 +33,7 @@ char *virNumaGetAutoPlacementAdvice(unsigned short vcups, int virNumaSetupMemoryPolicy(virDomainNumatuneMemMode mode, virBitmapPtr nodeset); -virBitmapPtr virNumaGetHostNodeset(void); +virBitmapPtr virNumaGetHostMemoryNodeset(void); bool virNumaNodesetIsAvailable(virBitmapPtr nodeset); bool virNumaIsAvailable(void); int virNumaGetMaxNode(void); -- 2.10.0

On Tue, Sep 13, 2016 at 05:08:02PM +0200, Peter Krempa wrote:
See patch 2.
Peter Krempa (2): util: numa: Remove impossible error handling numa: Rename virNumaGetHostNodeset and make it return only nodes with memory
Looks sane, ACK.
src/libvirt_private.syms | 2 +- src/qemu/qemu_cgroup.c | 4 ++-- src/util/virnuma.c | 18 +++++++++++------- src/util/virnuma.h | 2 +- 4 files changed, 15 insertions(+), 11 deletions(-)
-- 2.10.0
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (2)
-
Martin Kletzander
-
Peter Krempa