[libvirt] [PATCH 2/9] conf: Extract code filling data for virDomainGetVcpuPinInfo

[John Ferlan]

On 02/24/2016 09:22 AM, Peter Krempa wrote:
> The implementation of the inner guts of the function is similar for all
> drivers, so we can add a helper and not have to reimplement it three
> times.
> ---
>  src/conf/domain_conf.c   | 64 ++++++++++++++++++++++++++++++++++++++++++++++++
>  src/conf/domain_conf.h   |  8 ++++++
>  src/libvirt_private.syms |  1 +
>  src/libxl/libxl_driver.c | 38 +++-------------------------
>  src/qemu/qemu_driver.c   | 43 +++-----------------------------
>  src/test/test_driver.c   | 38 ++++------------------------
>  6 files changed, 84 insertions(+), 108 deletions(-)
> 

ACK with one nit...

One difference I noted, the 'libxl' code would:

    memset(cpumaps, 0x00, maplen * ncpumaps);

just before filling.  Should that be replicated in the common code?  If
not then for the libxl code should the lines be kept?


John

[...]

> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index 404016e..a99c99c 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -2423,8 +2423,7 @@ libxlDomainGetVcpuPinInfo(virDomainPtr dom, int ncpumaps,
>      libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
>      virDomainObjPtr vm = NULL;
>      virDomainDefPtr targetDef = NULL;
> -    int hostcpus, vcpu, ret = -1;
> -    virBitmapPtr allcpumap = NULL;
> +    int ret = -1;
> 
>      virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
>                    VIR_DOMAIN_AFFECT_CONFIG, -1);
> @@ -2445,41 +2444,10 @@ libxlDomainGetVcpuPinInfo(virDomainPtr dom, int ncpumaps,
>      /* Make sure coverity knows targetDef is valid at this point. */
>      sa_assert(targetDef);
> 
> -    /* Clamp to actual number of vcpus */
> -    if (ncpumaps > virDomainDefGetVcpus(targetDef))
> -        ncpumaps = virDomainDefGetVcpus(targetDef);
> -
> -    if ((hostcpus = libxl_get_max_cpus(cfg->ctx)) < 0)
> -        goto cleanup;
> -
> -    if (!(allcpumap = virBitmapNew(hostcpus)))
> -        goto cleanup;
> -
> -    virBitmapSetAll(allcpumap);
> -
> -    memset(cpumaps, 0x00, maplen * ncpumaps);

^^^^^

> -
> -    for (vcpu = 0; vcpu < ncpumaps; vcpu++) {
> -        virDomainVcpuInfoPtr vcpuinfo = virDomainDefGetVcpu(targetDef, vcpu);
> -        virBitmapPtr bitmap = NULL;
> -
> -        if (!vcpuinfo->online)
> -            continue;
> -
> -        if (vcpuinfo->cpumask)
> -            bitmap = vcpuinfo->cpumask;
> -        else if (targetDef->cpumask)
> -            bitmap = targetDef->cpumask;
> -        else
> -            bitmap = allcpumap;
> -
> -        virBitmapToDataBuf(bitmap, VIR_GET_CPUMAP(cpumaps, maplen, vcpu), maplen);
> -    }
> -
> -    ret = ncpumaps;
> +    ret = virDomainDefGetVcpuPinInfoHelper(targetDef, maplen, ncpumaps, cpumaps,
> +                                           libxl_get_max_cpus(cfg->ctx), NULL);
> 
>   cleanup:
> -    virBitmapFree(allcpumap);
>      if (vm)
>          virObjectUnlock(vm);
>      virObjectUnref(cfg);

[...]