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
Show replies by date
From: Peter Krempa <pkrempa(a)redhat.com>
The function is not used outside of the module.
Signed-off-by: Peter Krempa <pkrempa(a)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(a)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(a)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(a)redhat.com>
Signed-off-by: Peter Krempa <pkrempa(a)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
From: Peter Krempa <pkrempa(a)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(a)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(a)redhat.com>
The caller doesn't check the return value. Remove it to avoid confusing
readers.
Signed-off-by: Peter Krempa <pkrempa(a)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