[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v1 2/8] use virBitmap to store cpupin info



On 08/30/2012 02:55 AM, Hu Tao wrote:
> ---
>  src/conf/domain_conf.c  |   98 +++++++++++------------------------------------
>  src/conf/domain_conf.h  |    3 +-
>  src/qemu/qemu_cgroup.c  |    3 +-
>  src/qemu/qemu_driver.c  |   14 +++++--
>  src/qemu/qemu_process.c |   20 ++++++----
>  5 files changed, 49 insertions(+), 89 deletions(-)
> 

> @@ -11005,34 +11001,6 @@ virDomainVcpuPinFindByVcpu(virDomainVcpuPinDefPtr *def,
>      return NULL;
>  }
>  
> -static char *bitmapFromBytemap(unsigned char *bytemap, int maplen)
> -{
> -    char *bitmap = NULL;
> -    int i;
> -
> -    if (VIR_ALLOC_N(bitmap, VIR_DOMAIN_CPUMASK_LEN) < 0) {
> -        virReportOOMError();
> -        goto cleanup;
> -    }
> -
> -    /* Reset bitmap to all 0s. */
> -    for (i = 0; i < VIR_DOMAIN_CPUMASK_LEN; i++)
> -        bitmap[i] = 0;
> -
> -    /* Convert bitmap (bytemap) to bitmap, which is byte map? */

Thank you for killing this horrible comment!

> -    for (i = 0; i < maplen; i++) {
> -        int cur;
> -
> -        for (cur = 0; cur < 8; cur++) {
> -            if (bytemap[i] & (1 << cur))
> -                bitmap[i * 8 + cur] = 1;
> -        }
> -    }

Indeed, the old code was converting a packed array (8 bits per char) to
an exploded array (1 bit per char), but using horribly confusing names
in the process.  But the new code uses virBitmap from the get-go.

> @@ -11040,20 +11008,19 @@ int virDomainVcpuPinAdd(virDomainVcpuPinDefPtr *vcpupin_list,
>                          int vcpu)
>  {
>      virDomainVcpuPinDefPtr vcpupin = NULL;
> -    char *cpumask = NULL;
>  
>      if (!vcpupin_list)
>          return -1;
>  
> -    if ((cpumask = bitmapFromBytemap(cpumap, maplen)) == NULL)
> -        return -1;
> -
>      vcpupin = virDomainVcpuPinFindByVcpu(vcpupin_list,
>                                           *nvcpupin,
>                                           vcpu);
>      if (vcpupin) {
>          vcpupin->vcpuid = vcpu;
> -        vcpupin->cpumask = cpumask;
> +        virBitmapFree(vcpupin->cpumask);
> +        vcpupin->cpumask = virBitmapAllocFromData(cpumap, maplen);
> +        if (!vcpupin->cpumask)
> +            return -1;

Shouldn't this report OOM?

>  
>          return 0;
>      }
> @@ -11062,16 +11029,15 @@ int virDomainVcpuPinAdd(virDomainVcpuPinDefPtr *vcpupin_list,
>  
>      if (VIR_ALLOC(vcpupin) < 0) {
>          virReportOOMError();
> -        VIR_FREE(cpumask);
>          return -1;
>      }
>      vcpupin->vcpuid = vcpu;
> -    vcpupin->cpumask = cpumask;
> -
> +    vcpupin->cpumask = virBitmapAllocFromData(cpumap, maplen);
> +    if (!vcpupin->cpumask)
> +        return -1;

And this?

> @@ -1980,11 +1981,13 @@ qemuProcessSetVcpuAffinites(virConnectPtr conn,
>          vcpu = def->cputune.vcpupin[n]->vcpuid;
>  
>          memset(cpumap, 0, cpumaplen);
> -        cpumask = (unsigned char *)def->cputune.vcpupin[n]->cpumask;
> +        cpumask = def->cputune.vcpupin[n]->cpumask;
>          vcpupid = priv->vcpupids[vcpu];
>  
> -        for (i = 0 ; i < VIR_DOMAIN_CPUMASK_LEN ; i++) {
> -            if (cpumask[i])
> +        for (i = 0 ; i < virBitmapSize(cpumask); i++) {
> +            if (virBitmapGetBit(cpumask, i, &result) < 0)
> +                goto cleanup;
> +            if (result)
>                  VIR_USE_CPU(cpumap, i);

I wonder if virBitmap _also_ needs new functions for converting between
virBitmapPtr and the API maps, instead of having to handroll our own
VIR_[UN]USE_CPU loops.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]