[libvirt] [PATCH v5 0/2] List only online cpus for vcpupin/emulatorpin when vcpu placement static
Currently when the vcpu placement is static and cpuset is not specified, CPU Affinity shows 0.. CPUMAX. This patchset will result in display of only online CPU's under CPU Affinity on linux. Fixes the following Bug: virsh dumpxml Fedora <domain type='kvm' id='4'> <name>Fedora</name> <uuid>aecf3e5e-6f9a-42a3-9d6a-223a75569a66</uuid> <maxMemory slots='32' unit='KiB'>3145728</maxMemory> <memory unit='KiB'>524288</memory> <currentMemory unit='KiB'>524288</currentMemory> <vcpu placement='static' current='8'>160</vcpu> <resource> <partition>/machine</partition> </resource> ..................... ....................... ......................... <memballoon model='virtio'> <alias name='balloon0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> </memballoon> </devices> <seclabel type='dynamic' model='dac' relabel='yes'> <label>+0:+0</label> <imagelabel>+0:+0</imagelabel> </seclabel> </domain> lscpu Architecture: x86_64 CPU op-mode(s): 32-bit, 64-bit Byte Order: Little Endian CPU(s): 8 On-line CPU(s) list: 0-2,4-7 Off-line CPU(s) list: 3 Thread(s) per core: 1 Core(s) per socket: 7 Socket(s): 1 .......... .......... NUMA node0 CPU(s): 0-2,4-7 NUMA node1 CPU(s): cat /sys/devices/system/cpu/online 0-2,4-7 Before Patch virsh vcpupin Fedora VCPU: CPU Affinity ---------------------------------- 0: 0-7 1: 0-7 ... ... 158: 0-7 159: 0-7 virsh emulatorpin Fedora emulator: CPU Affinity ---------------------------------- *: 0-7 After Patch virsh vcpupin Fedora VCPU: CPU Affinity ---------------------------------- 0: 0-2,4-7 1: 0-2,4-7 ... ... 158: 0-2,4-7 159: 0-2,4-7 virsh emulatorpin Fedora emulator: CPU Affinity ---------------------------------- *: 0-2,4-7 Nitesh Konkar (2): conf: List only online cpus for virsh vcpupin conf: List only online cpus for virsh emulatorpin src/conf/domain_conf.c | 6 ++++++ src/qemu/qemu_driver.c | 5 +++++ 2 files changed, 11 insertions(+) -- 2.1.0
Currently when the vcpu placement is static and cpuset is not specified, CPU Affinity under virsh vcpupin 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@linux.vnet.ibm.com> --- src/conf/domain_conf.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e7517d9..f25cf63 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -56,6 +56,7 @@ #include "virstring.h" #include "virnetdev.h" #include "virhostdev.h" +#include "virhostcpu.h" #define VIR_FROM_THIS VIR_FROM_DOMAIN @@ -1549,10 +1550,15 @@ virDomainDefGetVcpuPinInfoHelper(virDomainDefPtr def, if (hostcpus < 0) return -1; +#ifdef __linux__ + if (!(allcpumap = virHostCPUGetOnlineBitmap())) + return -1; +#else if (!(allcpumap = virBitmapNew(hostcpus))) return -1; virBitmapSetAll(allcpumap); +#endif for (i = 0; i < maxvcpus && i < ncpumaps; i++) { virDomainVcpuDefPtr vcpu = virDomainDefGetVcpu(def, i); -- 2.1.0
On 11/24/2016 04:55 AM, Nitesh Konkar wrote:
Currently when the vcpu placement is static and cpuset is not specified, CPU Affinity under virsh vcpupin 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@linux.vnet.ibm.com> --- src/conf/domain_conf.c | 6 ++++++ 1 file changed, 6 insertions(+)
It seems there are some similarities to Viktor's series: http://www.redhat.com/archives/libvir-list/2016-November/msg01320.html at least with respect to creating an accessor function (in patch 1) virHostCPUHasBitmap that will hide that virHostCPUGetOnlineBitmap call and the ugly #ifdef __linux__ in the domain_conf.c file. Using virHostCPUHasBitmap will hide the need for the #ifdef... Secondary, should there be a fallback in the event that a NULL is returned from virHostCPUGetOnlineBitmap? Finally, does there need to be a similar call to like what Viktor did w/r/t virProcessGetAffinity and making sure the bitmaps are equal? John
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e7517d9..f25cf63 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -56,6 +56,7 @@ #include "virstring.h" #include "virnetdev.h" #include "virhostdev.h" +#include "virhostcpu.h"
#define VIR_FROM_THIS VIR_FROM_DOMAIN
@@ -1549,10 +1550,15 @@ virDomainDefGetVcpuPinInfoHelper(virDomainDefPtr def, if (hostcpus < 0) return -1;
+#ifdef __linux__ + if (!(allcpumap = virHostCPUGetOnlineBitmap())) + return -1; +#else if (!(allcpumap = virBitmapNew(hostcpus))) return -1;
virBitmapSetAll(allcpumap); +#endif
for (i = 0; i < maxvcpus && i < ncpumaps; i++) { virDomainVcpuDefPtr vcpu = virDomainDefGetVcpu(def, i);
On 14.12.2016 00:57, John Ferlan wrote:
On 11/24/2016 04:55 AM, Nitesh Konkar wrote:
Currently when the vcpu placement is static and cpuset is not specified, CPU Affinity under virsh vcpupin 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@linux.vnet.ibm.com> --- src/conf/domain_conf.c | 6 ++++++ 1 file changed, 6 insertions(+)
It seems there are some similarities to Viktor's series:
http://www.redhat.com/archives/libvir-list/2016-November/msg01320.html
at least with respect to creating an accessor function (in patch 1) virHostCPUHasBitmap that will hide that virHostCPUGetOnlineBitmap call and the ugly #ifdef __linux__ in the domain_conf.c file.
Using virHostCPUHasBitmap will hide the need for the #ifdef...
Secondary, should there be a fallback in the event that a NULL is returned from virHostCPUGetOnlineBitmap?
Finally, does there need to be a similar call to like what Viktor did w/r/t virProcessGetAffinity and making sure the bitmaps are equal?
The intention of this series is (iiuc) to reflect the live situation inflicted by the host's current online cpu map. In order to do that, one would have to distinguish between running and a defined only domains. The code as proposed would currently only reflect the 'defined' situation. For a running domain, virProcessGetAffinity would return the equivalent cpu mask. Which, btw. would have to be used even if the domain is carrying a cpumask. Otherwise the live state wouldn't be reflected properly. The question is probably whether the vcpu pin info is supposed to reflect the real or the intended state. The current implementation treats it as the intended state. Personally, I'd find the following behavior most logical: Invoked with VIR_DOMAIN_AFFECT_LIVE: Get/set the process current cpu affinity. Don't touch the definition. This would reflect the real live state. Invoked with VIR_DOMAIN_AFFECT_CONFIG: Get/set the values in the persistent domain configuration only. This would reflect the intended state. I don't think however that this intended state should depend on the current online host cpus. This would still not reveal whether the vcpus are pinned or not. I am wondering whether we just might need a new API telling us whether a domain is pinned or not (if we don't want to client to parse the domain XML)? [...] -- 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
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@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 cpumask = bitmap; } -- 2.1.0
On 11/24/2016 04:55 AM, 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@linux.vnet.ibm.com> --- src/qemu/qemu_driver.c | 5 +++++ 1 file changed, 5 insertions(+)
Similar questions/observations here obviously...
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;
Also the above is misaligned in the } else { It would be using the new function in any case, but be sure it's aligned properly and not just cut-n-paste John
+#else if (!(bitmap = virBitmapNew(hostcpus))) goto cleanup; virBitmapSetAll(bitmap); +#endif cpumask = bitmap; }
participants (3)
-
John Ferlan -
Nitesh Konkar -
Viktor Mihajlovski