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
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list