[libvirt] [PATCH go] domain.go: construct cpumaps correctly for CPU pinning verbs

In PinVcpu(), PinVcpuFlags(), PinEmulator() and PinIOThread() there is an almost identical code that converts []bool into a bitmask. It calculates the location in the bitmask and then sets it always to 1, instead of looking at the actual bool value. --- domain.go | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/domain.go b/domain.go index 5482f1a..9a24bc5 100644 --- a/domain.go +++ b/domain.go @@ -1750,7 +1750,9 @@ func (d *Domain) PinVcpu(vcpu uint, cpuMap []bool) error { if cpuMap[i] { byte := i / 8 bit := i % 8 - ccpumap[byte] |= (1 << uint(bit)) + if cpuMap[i] { + ccpumap[byte] |= (1 << uint(bit)) + } } } @@ -1770,7 +1772,9 @@ func (d *Domain) PinVcpuFlags(vcpu uint, cpuMap []bool, flags DomainModification if cpuMap[i] { byte := (i + 7) / 8 bit := i % 8 - ccpumap[byte] |= (1 << uint(bit)) + if cpuMap[i] { + ccpumap[byte] |= (1 << uint(bit)) + } } } @@ -3970,7 +3974,9 @@ func (d *Domain) PinEmulator(cpumap []bool, flags DomainModificationImpact) erro byte := i / 8 bit := i % 8 - ccpumaps[byte] |= (1 << uint(bit)) + if cpumap[i] { + ccpumaps[byte] |= (1 << uint(bit)) + } } ret := C.virDomainPinEmulator(d.ptr, &ccpumaps[0], C.int(maplen), C.uint(flags)) @@ -3992,7 +3998,9 @@ func (d *Domain) PinIOThread(iothreadid uint, cpumap []bool, flags DomainModific byte := i / 8 bit := i % 8 - ccpumaps[byte] |= (1 << uint(bit)) + if cpumap[i] { + ccpumaps[byte] |= (1 << uint(bit)) + } } ret := C.virDomainPinIOThreadCompat(d.ptr, C.uint(iothreadid), &ccpumaps[0], C.int(maplen), C.uint(flags)) -- 2.11.0

Please ignore this patch, I failed to see that in 2 out of 4 locations we already are looking at the value. I'm sending a better patch instead. On Tue, Feb 14, 2017 at 4:48 PM, Leonid Podolny <leonid@podolny.net> wrote:
In PinVcpu(), PinVcpuFlags(), PinEmulator() and PinIOThread() there is an almost identical code that converts []bool into a bitmask. It calculates the location in the bitmask and then sets it always to 1, instead of looking at the actual bool value. --- domain.go | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/domain.go b/domain.go index 5482f1a..9a24bc5 100644 --- a/domain.go +++ b/domain.go @@ -1750,7 +1750,9 @@ func (d *Domain) PinVcpu(vcpu uint, cpuMap []bool) error { if cpuMap[i] { byte := i / 8 bit := i % 8 - ccpumap[byte] |= (1 << uint(bit)) + if cpuMap[i] { + ccpumap[byte] |= (1 << uint(bit)) + } } }
@@ -1770,7 +1772,9 @@ func (d *Domain) PinVcpuFlags(vcpu uint, cpuMap []bool, flags DomainModification if cpuMap[i] { byte := (i + 7) / 8 bit := i % 8 - ccpumap[byte] |= (1 << uint(bit)) + if cpuMap[i] { + ccpumap[byte] |= (1 << uint(bit)) + } } }
@@ -3970,7 +3974,9 @@ func (d *Domain) PinEmulator(cpumap []bool, flags DomainModificationImpact) erro byte := i / 8 bit := i % 8
- ccpumaps[byte] |= (1 << uint(bit)) + if cpumap[i] { + ccpumaps[byte] |= (1 << uint(bit)) + } }
ret := C.virDomainPinEmulator(d.ptr, &ccpumaps[0], C.int(maplen), C.uint(flags)) @@ -3992,7 +3998,9 @@ func (d *Domain) PinIOThread(iothreadid uint, cpumap []bool, flags DomainModific byte := i / 8 bit := i % 8
- ccpumaps[byte] |= (1 << uint(bit)) + if cpumap[i] { + ccpumaps[byte] |= (1 << uint(bit)) + } }
ret := C.virDomainPinIOThreadCompat(d.ptr, C.uint(iothreadid), &ccpumaps[0], C.int(maplen), C.uint(flags)) -- 2.11.0
participants (1)
-
Leonid Podolny