On Mon, Aug 31, 2026 at 14:56:14 +0200, Thomas Prescher wrote:

From: Stefan Kober <stefan.kober@cyberus-technology.de>

Make use of the numa configuration the user has configured via libvirt and transform it into the right cloud hypervisor API calls.

On-behalf-of: SAP stefan.kober@sap.com On-behalf-of: SAP thomas.prescher@sap.com

Signed-off-by: Stefan Kober <stefan.kober@cyberus-technology.de> Signed-off-by: Thomas Prescher <thomas.prescher@cyberus-technology.de> --- src/ch/ch_monitor.c | 140 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 138 insertions(+), 2 deletions(-)

diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c index 52f281cf73..9bd5eb0114 100644 --- a/src/ch/ch_monitor.c +++ b/src/ch/ch_monitor.c @@ -99,6 +99,71 @@ virCHMonitorBuildCPUJson(virJSONValue *content, virDomainDef *vmdef) return 0; }

+/** + * CHV example NUMA cmdline: + *--numa guest_numa_id=0,cpus=[0,1],memory_zones=[fast_mem] \ + --numa guest_numa_id=1,cpus=[2,3],memory_zones=[bulk_mem] \ + --memory-zone id=fast_mem,size=2G,host_numa_node=0,hugepages=on,hugepage_size=1G,prefault=on \ + --memory-zone id=bulk_mem,size=6G,host_numa_node=1,hugepages=on,hugepage_size=2M \

If you are about to use this styling of comment use the * leaders consistently.

+ */ +static int +virCHMonitorBuildNumaJson(virJSONValue *content, virDomainDef *def)

JSON in name and one argument per line.

+{ + size_t ncells = virDomainNumaGetNodeCount(def->numa); + size_t i = 0; + size_t j = 0; + virBitmap *cpus = NULL; + g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; + g_autoptr(virJSONValue) numas = virJSONValueNewArray(); + + if (ncells == 0) { + return 0; + } + + for (i = 0; i < ncells; i++) { + ssize_t lastcpu = 0; + g_autoptr(virJSONValue) numa = virJSONValueNewObject(); + g_autoptr(virJSONValue) mem_zones = virJSONValueNewArray(); + char *mem_zone_str = g_strdup_printf("zone%lu", i); + g_autoptr(virJSONValue) mem_zone_id = virJSONValueNewString(mem_zone_str); + g_autoptr(virJSONValue) cpu_arr = virJSONValueNewArray(); + + cpus = virDomainNumaGetNodeCpumask(def->numa, i); + lastcpu = virBitmapLastSetBit(cpus); + + // Go through bitmap and check set bits which correspond to CPUs + // We create an array of vCPU IDs: [1,2,3] of CPUs belonging to the + // respective NUMA node.

Do not use line comments (//) see coding style docs.

+ for (j = 0; j < lastcpu + 1; j++) { + if (virBitmapIsBitSet(cpus, j)) { + g_autoptr(virJSONValue) cpu_id = virJSONValueNewNumberUint(j); + if (virJSONValueArrayAppend(cpu_arr, &cpu_id) < 0) + return -1; + } + + } + + if (virJSONValueObjectAppendNumberUlong(numa, "guest_numa_id", i) < 0) + return -1; + + if (virJSONValueArrayAppend(mem_zones, &mem_zone_id) < 0) + return -1; + + if (virJSONValueObjectAppend(numa, "memory_zones", &mem_zones) < 0) + return -1; + + if (virJSONValueObjectAppend(numa, "cpus", &cpu_arr) < 0) + return -1;

Consider using virJSONValueObjectAdd to construct objects instead of multiple calls.

+ + if (virJSONValueArrayAppend(numas, &numa) < 0) + return -1; + } + + if (virJSONValueObjectAppend(content, "numa", &numas) < 0) + return -1; + return 0; +} + static int virCHMonitorBuildConsoleJson(virJSONValue *content, virDomainDef *vmdef) @@ -222,16 +287,84 @@ virCHMonitorBuildKernelRelatedJson(virJSONValue *content, virDomainDef *vmdef) return 0; }

+/** + * CHV example NUMA cmdline: + *--numa guest_numa_id=0,cpus=[0,1],memory_zones=[fast_mem] \ + --numa guest_numa_id=1,cpus=[2,3],memory_zones=[bulk_mem] \ + --memory-zone id=fast_mem,size=2G,host_numa_node=0,hugepages=on,hugepage_size=1G,prefault=on \ + --memory-zone id=bulk_mem,size=6G,host_numa_node=1,hugepages=on,hugepage_size=2M \ + + In this function we only define the memory zones + */ +static int +virCHMonitorBuildMemoryZonesJson(virJSONValue *content, virDomainDef *def) +{ + size_t ncells = virDomainNumaGetNodeCount(def->numa); + size_t i = 0; + g_autoptr(virJSONValue) zones = virJSONValueNewArray(); + + VIR_INFO("Found NUMA cell configuration. Creating %ld NUMA nodes for guest VM.", ncells);

Do not use VIR_INFO. It's either VIR_DEBUG or it doesn't belong into the log.

+ + for (i = 0; i < ncells; i++) { + g_autofree char *id = g_strdup_printf("zone%zu", i); + // Memory returned is in KiB, so we multiply by 1024

Everything that I wrote above applies to this function too.

+ unsigned long long memsize = virDomainNumaGetNodeMemorySize(def->numa, i) * 1024; + g_autoptr(virJSONValue) zone = virJSONValueNewObject(); + virBitmap *nodes = virDomainNumatuneGetNodeset(def->numa, NULL, i); + g_autofree char *nodeset = virBitmapFormat(nodes); + size_t hostNodeCount = virBitmapCountBits(nodes); + size_t hostNode = virBitmapLastSetBit(nodes); + + if (hostNodeCount > 1) { + VIR_ERROR(_("There are %ld host nodes specified but Cloud Hypervisor only supports 1"), hostNodeCount);

VIR_ERROR is an internal macro. If you want to raise an error you must use virReportError instead. The line is also very long.

+ return -1; + } + + if (virJSONValueObjectAppendString(zone, "id", id) < 0) + return -1; + + if (virJSONValueObjectAppendNumberUlong(zone, "size", memsize) < 0) + return -1; + + if (hostNodeCount == 1) { + VIR_DEBUG("Associating guest node %lu with host node %s", i , nodeset); + + if (virJSONValueObjectAppendNumberUlong(zone, "host_numa_node", hostNode) < 0) + return -1; + } + + if (virJSONValueArrayAppend(zones, &zone) < 0) + return -1; + } + if (virJSONValueObjectAppend(content, "zones", &zones) < 0) + return -1; + + return 0; +} + static int virCHMonitorBuildMemoryJson(virJSONValue *content, virDomainDef *vmdef) { unsigned long long total_memory = virDomainDefGetMemoryInitial(vmdef) * 1024; + size_t ncells = virDomainNumaGetNodeCount(vmdef->numa);

if (total_memory != 0) { g_autoptr(virJSONValue) memory = virJSONValueNewObject();

- if (virJSONValueObjectAppendNumberUlong(memory, "size", total_memory) < 0) - return -1; + // If we have multiple NUMA nodes, then we define memory zones. When + // memory zones are defined, the "size" field in the CHV memory config + // must be 0 + if (ncells >= 1) { + if (virCHMonitorBuildMemoryZonesJson(memory, vmdef) < 0) { + return -1; + } + if (virJSONValueObjectAppendNumberUlong(memory, "size", 0) < 0) { + return -1; + } + } else { + if (virJSONValueObjectAppendNumberUlong(memory, "size", total_memory) < 0) + return -1; + }

if (virJSONValueObjectAppend(content, "memory", &memory) < 0) return -1; @@ -560,6 +693,9 @@ virCHMonitorBuildVMJson(virCHDriver *driver, virDomainDef *vmdef, if (virCHMonitorBuildMemoryJson(content, vmdef) < 0) return -1;

+ if (virCHMonitorBuildNumaJson(content, vmdef) < 0) + return -1; + if (virBitmapIsBitSet(driver->chCaps, CH_KERNEL_API_DEPRCATED)) { if (virCHMonitorBuildPayloadJson(content, vmdef) < 0) return -1; -- 2.53.0

On Mon, Aug 31, 2026 at 14:56:15 +0200, Thomas Prescher wrote:

From: Stefan Kober <stefan.kober@cyberus-technology.de>

Parse the user provided cpu topology information and transform it into correct cloud-hypervisor API calls.

On-behalf-of: SAP stefan.kober@sap.com On-behalf-of: SAP thomas.prescher@sap.com

Signed-off-by: Stefan Kober <stefan.kober@cyberus-technology.de> Signed-off-by: Thomas Prescher <thomas.prescher@cyberus-technology.de> --- src/ch/ch_monitor.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+)

diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c index 9bd5eb0114..e0fae8206c 100644 --- a/src/ch/ch_monitor.c +++ b/src/ch/ch_monitor.c @@ -69,10 +69,28 @@ virCHMonitorPut(virCHMonitor *mon, domainLogContext *logCtxt, virJSONValue **answer);

+static int +virCHMonitorBuildCPUTopologyJson(virJSONValue *content, virDomainDef *vmdef)

For functions which have 'JSON' in name we prefer the uppercase spelling. Also one argument per line please.

+{ + if (virDomainDefGetVcpusTopology(vmdef, NULL) != 0) { + return -1; + }

This returns '1' when topology is not configured. Here you convert it to -1 which is 'error' in our usual call convention ...

+ if (virJSONValueObjectAppendNumberInt(content, "threads_per_core", vmdef->cpu->threads) < 0) + return -1;

... as used here. The problem is that when 1 is returned from virDomainDefGetVcpusTopology no libvirt error is reported, but here ther e is one which we don't allow because the caller then can return an error without an message.

+ if (virJSONValueObjectAppendNumberInt(content, "cores_per_die", vmdef->cpu->cores) < 0) + return -1; + if (virJSONValueObjectAppendNumberInt(content, "dies_per_package", vmdef->cpu->dies) < 0) + return -1;

Consider using qvirJSONValueObjectAdd instead of the 4 calls to virJSONValueObjectAppendNumberInt

+ if (virJSONValueObjectAppendNumberInt(content, "packages", vmdef->cpu->sockets) < 0) + return -1; + return 0; +} + static int virCHMonitorBuildCPUJson(virJSONValue *content, virDomainDef *vmdef) { g_autoptr(virJSONValue) cpus = NULL; + g_autoptr(virJSONValue) topology = virJSONValueNewObject(); unsigned int maxvcpus = 0; unsigned int nvcpus = 0; virDomainVcpuDef *vcpu; @@ -92,6 +110,11 @@ virCHMonitorBuildCPUJson(virJSONValue *content, virDomainDef *vmdef) return -1; if (virJSONValueObjectAppendNumberInt(cpus, "max_vcpus", vmdef->maxvcpus) < 0) return -1; + if (virCHMonitorBuildCPUTopologyJson(topology, vmdef) == 0) { + VIR_INFO("Using CPU topology: %d:%d:%d:%d", vmdef->cpu->sockets, vmdef->cpu->dies, vmdef->cpu->cores, vmdef->cpu->threads);

Do not use VIR_INFO. Either it's a debug statement and VIR_DEBUG should be used or it doesn't belong into logs.

+ if (virJSONValueObjectAppend(cpus, "topology", &topology) < 0) + return -1; + } if (virJSONValueObjectAppend(content, "cpus", &cpus) < 0) return -1; } -- 2.53.0