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

[Eric Blake]

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 at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 617 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120830/955dae5f/attachment-0001.sig>