[PATCH 0/5] Cleanup of callers of virDomainDefGetVcpu
Fix few issues with how virDomainDefGetVcpu is called and in related code. Peter Krempa (5): Unexport virCHProcessSetupVcpu virDomainVcpuDefPostParse: Remove impossible check qemu: domain: Remove unused qemuDomainGetVcpuHalted virCHDomainRefreshThreadInfo: Don't trust vcpu ID returned by hypervisor virCHDomainRefreshThreadInfo: Remove illusion that caller cares about return value src/ch/ch_domain.c | 17 ++++++++++------- src/ch/ch_domain.h | 2 +- src/ch/ch_process.c | 2 +- src/ch/ch_process.h | 3 --- src/conf/domain_postparse.c | 4 ---- src/qemu/qemu_domain.c | 15 --------------- src/qemu/qemu_domain.h | 1 - 7 files changed, 12 insertions(+), 32 deletions(-) -- 2.50.1
From: Peter Krempa <pkrempa@redhat.com> The function is not used outside of the module. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/ch/ch_process.c | 2 +- src/ch/ch_process.h | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/ch/ch_process.c b/src/ch/ch_process.c index cc84823fdc..cd2e88af1e 100644 --- a/src/ch/ch_process.c +++ b/src/ch/ch_process.c @@ -409,7 +409,7 @@ virCHProcessSetupEmulatorThreads(virDomainObj *vm) * * Returns 0 on success, -1 on error. */ -int +static int virCHProcessSetupVcpu(virDomainObj *vm, unsigned int vcpuid) { diff --git a/src/ch/ch_process.h b/src/ch/ch_process.h index 7a6995b7cf..70ae8f700d 100644 --- a/src/ch/ch_process.h +++ b/src/ch/ch_process.h @@ -30,9 +30,6 @@ int virCHProcessStop(virCHDriver *driver, virDomainObj *vm, virDomainShutoffReason reason); -int virCHProcessSetupVcpu(virDomainObj *vm, - unsigned int vcpuid); - int virCHProcessStartRestore(virCHDriver *driver, virDomainObj *vm, const char *from); -- 2.50.1
From: Peter Krempa <pkrempa@redhat.com> Many callers of 'virDomainDefGetVcpu' don't validate return value when iterating CPUs up to def->maxvcpus/virDomainDefGetVcpusMax. Remove this one. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_postparse.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/conf/domain_postparse.c b/src/conf/domain_postparse.c index a07ec8d94e..38e731348d 100644 --- a/src/conf/domain_postparse.c +++ b/src/conf/domain_postparse.c @@ -1013,10 +1013,6 @@ virDomainVcpuDefPostParse(virDomainDef *def) for (i = 0; i < maxvcpus; i++) { vcpu = virDomainDefGetVcpu(def, i); - /* impossible but some compilers don't like it */ - if (!vcpu) - continue; - switch (vcpu->hotpluggable) { case VIR_TRISTATE_BOOL_ABSENT: if (vcpu->online) -- 2.50.1
From: Peter Krempa <pkrempa@redhat.com> Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 15 --------------- src/qemu/qemu_domain.h | 1 - 2 files changed, 16 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index a2c7c88a7e..e45757ccd5 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -8603,21 +8603,6 @@ qemuDomainRefreshVcpuInfo(virDomainObj *vm, return ret; } -/** - * qemuDomainGetVcpuHalted: - * @vm: domain object - * @vcpu: cpu id - * - * Returns the vCPU halted state. - */ -bool -qemuDomainGetVcpuHalted(virDomainObj *vm, - unsigned int vcpuid) -{ - virDomainVcpuDef *vcpu = virDomainDefGetVcpu(vm->def, vcpuid); - return QEMU_DOMAIN_VCPU_PRIVATE(vcpu)->halted; -} - /** * qemuDomainRefreshVcpuHalted: * @driver: qemu driver data diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 1afd932764..ffe5bee1bf 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -870,7 +870,6 @@ int qemuDomainValidateVcpuInfo(virDomainObj *vm); int qemuDomainRefreshVcpuInfo(virDomainObj *vm, int asyncJob, bool state); -bool qemuDomainGetVcpuHalted(virDomainObj *vm, unsigned int vcpu); int qemuDomainRefreshVcpuHalted(virDomainObj *vm, int asyncJob); -- 2.50.1
On a Thursday in 2025, Peter Krempa via Devel wrote:
From: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 15 --------------- src/qemu/qemu_domain.h | 1 - 2 files changed, 16 deletions(-)
Unused since: commit 2222548b1e55257dc8806abdbefa71a87b22dea1
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index a2c7c88a7e..e45757ccd5 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -8603,21 +8603,6 @@ qemuDomainRefreshVcpuInfo(virDomainObj *vm, return ret; }
-/** - * qemuDomainGetVcpuHalted: - * @vm: domain object - * @vcpu: cpu id - * - * Returns the vCPU halted state. - */ -bool -qemuDomainGetVcpuHalted(virDomainObj *vm, - unsigned int vcpuid) -{ - virDomainVcpuDef *vcpu = virDomainDefGetVcpu(vm->def, vcpuid); - return QEMU_DOMAIN_VCPU_PRIVATE(vcpu)->halted; -} - /** * qemuDomainRefreshVcpuHalted: * @driver: qemu driver data diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 1afd932764..ffe5bee1bf 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -870,7 +870,6 @@ int qemuDomainValidateVcpuInfo(virDomainObj *vm); int qemuDomainRefreshVcpuInfo(virDomainObj *vm, int asyncJob, bool state); -bool qemuDomainGetVcpuHalted(virDomainObj *vm, unsigned int vcpu); int qemuDomainRefreshVcpuHalted(virDomainObj *vm, int asyncJob);
-- 2.50.1
From: Peter Krempa <pkrempa@redhat.com> The hypervisor may return an index out of range of current vCPUs defined in the domain which would cause a NULL dereference. Validate that the vCPU struct with ID fetched from hypervisor exists before dereferencing it. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/ch/ch_domain.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/ch/ch_domain.c b/src/ch/ch_domain.c index 7231fdc49f..85bd99e1e9 100644 --- a/src/ch/ch_domain.c +++ b/src/ch/ch_domain.c @@ -276,10 +276,15 @@ virCHDomainRefreshThreadInfo(virDomainObj *vm) /* TODO: hotplug support */ vcpuInfo = &info[i].vcpuInfo; - vcpu = virDomainDefGetVcpu(vm->def, vcpuInfo->cpuid); - vcpupriv = CH_DOMAIN_VCPU_PRIVATE(vcpu); - vcpupriv->tid = vcpuInfo->tid; - ncpus++; + + if ((vcpu = virDomainDefGetVcpu(vm->def, vcpuInfo->cpuid))) { + vcpupriv = CH_DOMAIN_VCPU_PRIVATE(vcpu); + vcpupriv->tid = vcpuInfo->tid; + ncpus++; + } else { + VIR_WARN("vcpu '%d' reported by hypervisor but not found in definition", + vcpuInfo->cpuid); + } } /* TODO: Remove the warning when hotplug is implemented.*/ -- 2.50.1
From: Peter Krempa <pkrempa@redhat.com> The caller doesn't check the return value. Remove it to avoid confusing readers. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/ch/ch_domain.c | 4 +--- src/ch/ch_domain.h | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/ch/ch_domain.c b/src/ch/ch_domain.c index 85bd99e1e9..6ace9eafbf 100644 --- a/src/ch/ch_domain.c +++ b/src/ch/ch_domain.c @@ -254,7 +254,7 @@ chValidateDomainDeviceDef(const virDomainDeviceDef *dev, return 0; } -int +void virCHDomainRefreshThreadInfo(virDomainObj *vm) { unsigned int maxvcpus = virDomainDefGetVcpusMax(vm->def); @@ -291,8 +291,6 @@ virCHDomainRefreshThreadInfo(virDomainObj *vm) if (ncpus != maxvcpus) VIR_WARN("Mismatch in the number of cpus, expected: %u, actual: %zu", maxvcpus, ncpus); - - return 0; } virDomainDefParserConfig virCHDriverDomainDefParserConfig = { diff --git a/src/ch/ch_domain.h b/src/ch/ch_domain.h index 69a657f6af..4532fe9ce0 100644 --- a/src/ch/ch_domain.h +++ b/src/ch/ch_domain.h @@ -62,7 +62,7 @@ void virCHDomainRemoveInactive(virCHDriver *driver, virDomainObj *vm); -int +void virCHDomainRefreshThreadInfo(virDomainObj *vm); pid_t -- 2.50.1
On a Thursday in 2025, Peter Krempa via Devel wrote:
Fix few issues with how virDomainDefGetVcpu is called and in related code.
Peter Krempa (5): Unexport virCHProcessSetupVcpu virDomainVcpuDefPostParse: Remove impossible check qemu: domain: Remove unused qemuDomainGetVcpuHalted virCHDomainRefreshThreadInfo: Don't trust vcpu ID returned by hypervisor virCHDomainRefreshThreadInfo: Remove illusion that caller cares about return value
src/ch/ch_domain.c | 17 ++++++++++------- src/ch/ch_domain.h | 2 +- src/ch/ch_process.c | 2 +- src/ch/ch_process.h | 3 --- src/conf/domain_postparse.c | 4 ---- src/qemu/qemu_domain.c | 15 --------------- src/qemu/qemu_domain.h | 1 - 7 files changed, 12 insertions(+), 32 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko -
Peter Krempa