[libvirt] [PATCH v5 2/2] conf: List only online cpus for virsh emulatorpin

[Viktor Mihajlovski]

On 06.12.2016 12:33, Peter Krempa wrote:
> On Fri, Nov 25, 2016 at 14:53:07 +0530, Nitesh Konkar wrote:
>> Currently when the vcpu placement is static and cpuset is not
>> specified, CPU Affinity under virsh emulatorpin shows 0..CPUMAX.
>> This patchset will result in display of only online CPU's under
>> CPU Affinity on linux.
>> 
>> Signed-off-by: Nitesh Konkar <nitkon12 at linux.vnet.ibm.com> --- 
>> src/qemu/qemu_driver.c | 5 +++++ 1 file changed, 5 insertions(+)
>> 
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c 
>> index fdfe912..e69d92d 100644 --- a/src/qemu/qemu_driver.c +++
>> b/src/qemu/qemu_driver.c @@ -5435,9 +5435,14 @@
>> qemuDomainGetEmulatorPinInfo(virDomainPtr dom, autoCpuset) { 
>> cpumask = autoCpuset; } else { +#ifdef __linux__ +    if
>> (!(bitmap = virHostCPUGetOnlineBitmap())) +        goto cleanup; 
>> +#else if (!(bitmap = virBitmapNew(hostcpus))) goto cleanup; 
>> virBitmapSetAll(bitmap); +#endif
> 
> Adding conditionally compiled code twice does not look good.
> Please extract the above into a helper function and use it in the
> two places rather than having two blocks of conditional code.
> 
I've already posted a patch for that purpose in
https://www.redhat.com/archives/libvir-list/2016-November/msg01321.html
> Also I'm not entirely perusaded that this is correct.
> 
> When you are getting information for the offline VM the
> configuration may change until you start the VM and thus the data
> gathered here will not be valid. (e.g. if you turn the cpus back
> on)
> 
> For running VMs the data depends on the state when the VM was
> started and at least for vcpu threads is gathered back using 
> virProcessGetAffinity and thus should be reported correctly.
> 
> Additionally we historically treat the "all bits set" cpu map as
> that there is no pinning configured. This would change this
> behavior. When the VM is started, the real cpu map should be
> returned, but for the config time I think the current state is
> correct and doing your change would be incorrect.
I have no strong opinion on whether the CPU mask should match the host
online mask for the defined case, but the "all bits set" map will
change with the old code as well, if new CPUs are added via hotplug.
The crux is that we don't have any way to find out there's in fact no
pinning in place for a process nor a way to "unpin" it.
> 
> Peter
> 
> 
> 
> -- libvir-list mailing list libvir-list at redhat.com 
> https://www.redhat.com/mailman/listinfo/libvir-list
> 


-- 

Mit freundlichen Grüßen/Kind Regards
   Viktor Mihajlovski

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martina Köderitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294