On 01/14/2016 11:27 AM, Peter Krempa wrote:
Now with the new struct the data can be stored in a much saner
place.
---
src/conf/domain_conf.c | 131 ++++++++++++++++++--------------------------
src/conf/domain_conf.h | 3 +-
src/libxl/libxl_domain.c | 17 +++---
src/libxl/libxl_driver.c | 39 ++++++--------
src/qemu/qemu_cgroup.c | 15 ++----
src/qemu/qemu_driver.c | 138 ++++++++++++++++++++++-------------------------
src/qemu/qemu_process.c | 38 +++++++------
src/test/test_driver.c | 43 ++++++---------
8 files changed, 179 insertions(+), 245 deletions(-)
As noted from my review of 10/34, I'm just noting something Coverity
found - will review more as I get to this later.
[...]
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 5986749..ed4de12 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -2455,15 +2455,14 @@ static int testDomainGetVcpus(virDomainPtr domain,
memset(cpumaps, 0, maxinfo * maplen);
for (i = 0; i < maxinfo; i++) {
- virDomainPinDefPtr pininfo;
+ virDomainVcpuInfoPtr vcpu = virDomainDefGetVcpu(def, i);
virBitmapPtr bitmap = NULL;
- pininfo = virDomainPinFind(def->cputune.vcpupin,
- def->cputune.nvcpupin,
- i);
+ if (vcpu && !vcpu->online)
+ continue;
Coverity notes that by comparing vcpu to NULL here, but not doing so in
the following line could cause a NULL deref in the following lines.
- if (pininfo && pininfo->cpumask)
- bitmap = pininfo->cpumask;
+ if (vcpu->cpumask)
+ bitmap = vcpu->cpumask;
else if (def->cpumask)
bitmap = def->cpumask;
else
@@ -2492,6 +2491,7 @@ static int testDomainPinVcpu(virDomainPtr domain,
unsigned char *cpumap,
int maplen)
{
+ virDomainVcpuInfoPtr vcpuinfo;
virDomainObjPtr privdom;
virDomainDefPtr def;
int ret = -1;
@@ -2507,29 +2507,21 @@ static int testDomainPinVcpu(virDomainPtr domain,
goto cleanup;
}
- if (vcpu > virDomainDefGetVcpus(privdom->def)) {
+ if (!(vcpuinfo = virDomainDefGetVcpu(def, vcpu)) ||
+ !vcpuinfo->online) {
virReportError(VIR_ERR_INVALID_ARG,
_("requested vcpu '%d' is not present in the
domain"),
vcpu);
goto cleanup;
}
- if (!def->cputune.vcpupin) {
- if (VIR_ALLOC(def->cputune.vcpupin) < 0)
- goto cleanup;
- def->cputune.nvcpupin = 0;
- }
- if (virDomainPinAdd(&def->cputune.vcpupin,
- &def->cputune.nvcpupin,
- cpumap,
- maplen,
- vcpu) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("failed to update or add vcpupin"));
+ virBitmapFree(vcpuinfo->cpumask);
+
+ if (!(vcpuinfo->cpumask = virBitmapNewData(cpumap, maplen)))
goto cleanup;
- }
ret = 0;
+
cleanup:
virDomainObjEndAPI(&privdom);
return ret;
@@ -2566,15 +2558,14 @@ testDomainGetVcpuPinInfo(virDomainPtr dom,
ncpumaps = virDomainDefGetVcpus(def);
for (vcpu = 0; vcpu < ncpumaps; vcpu++) {
- virDomainPinDefPtr pininfo;
+ virDomainVcpuInfoPtr vcpuinfo = virDomainDefGetVcpu(def, vcpu);
virBitmapPtr bitmap = NULL;
- pininfo = virDomainPinFind(def->cputune.vcpupin,
- def->cputune.nvcpupin,
- vcpu);
+ if (vcpuinfo && !vcpuinfo->online)
+ continue;
Coverity notes that by comparing vcpuinfo to NULL here, but not doing so
in the following line could cause a NULL deref in the following lines.
- if (pininfo && pininfo->cpumask)
- bitmap = pininfo->cpumask;
+ if (vcpuinfo->cpumask)
+ bitmap = vcpuinfo->cpumask;
else if (def->cpumask)
bitmap = def->cpumask;
else