On Mon, Aug 31, 2026 at 14:56:15 +0200, Thomas Prescher wrote:
> From: Stefan Kober <[email protected]>
>
> Parse the user provided cpu topology information and
> transform it into correct cloud-hypervisor API calls.
>
> On-behalf-of: SAP [email protected]
> On-behalf-of: SAP [email protected]
>
> Signed-off-by: Stefan Kober <[email protected]>
> Signed-off-by: Thomas Prescher <[email protected]>
> ---
> 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
>
