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

Re: [libvirt] [PATCH v1 3/8] use virBitmap to store cpu affinity info



On 08/30/2012 02:55 AM, Hu Tao wrote:

>      if (ctrl->def->cpumask) {
>          /* XXX why don't we keep 'cpumask' in the libvirt cpumap
>           * format to start with ?!?! */

This comment now feels a bit out of date.

>          for (i = 0 ; i < maxcpu && i < ctrl->def->cpumasklen ; i++)
>              if (ctrl->def->cpumask[i])
> -                VIR_USE_CPU(cpumap, i);
> +                ignore_value(virBitmapSetBit(cpumap, i));

Why isn't ctrl->def->cpumask also stored as a virBitmap, at which point
this would be a copy operation instead of a hand-rolled loop?

>  
>      /* We are pressuming we are running between fork/exec of LXC

While you are here, s/pressuming/presuming/

>       * so use '0' to indicate our own process ID. No threads are
>       * running at this point
>       */
> -    if (virProcessInfoSetAffinity(0, /* Self */
> -                                  cpumap, cpumaplen, maxcpu) < 0) {
> +    if (virProcessInfoSetAffinity(0 /* Self */, cpumap) < 0) {
>          VIR_FREE(cpumap);

Oops - this cannot be VIR_FREE(cpumap)...

>          return -1;
>      }
> -    VIR_FREE(cpumap);
> +    virBitmapFree(cpumap);

...but must match this change to virBitmapFree(cpumap).


> @@ -4044,10 +4046,10 @@ qemudDomainPinEmulator(virDomainPtr dom,
>      virNodeInfo nodeinfo;
>      int ret = -1;
>      qemuDomainObjPrivatePtr priv;
> -    bool canResetting = true;
> -    int pcpu;
> +    bool canResetting = false;

'canResetting' sounds odd.  While you are touching this, can you rename
it to something saner, like 'doReset'?

> +++ b/src/util/processinfo.c

> @@ -59,8 +57,10 @@ realloc:
>      }
>  
>      CPU_ZERO_S(masklen, mask);
> -    for (i = 0 ; i < maxcpu ; i++) {
> -        if (VIR_CPU_USABLE(map, maplen, 0, i))
> +    for (i = 0 ; i < virBitmapSize(map); i++) {
> +        if (virBitmapGetBit(map, i, &set) < 0)
> +            return -1;
> +        if (set)
>              CPU_SET_S(i, masklen, mask);

Another case where virBitmap should do this in a single function call,
rather than you rolling the loop.

-- 
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]