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