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