[libvirt] [PATCH V3 0/2] Allow hot plugged host CPUs
This is a rework of the original patch allowing to use all host CPUs if the cpuset controller is not configured by libvirt. The major enhancement is that we won't try to retrieve the online host CPU map on platforms where this is not supported and just fall back to the old behavior. V3: - Added new function (in seperate patch) to find out whether the host CPU online map can be retrieved gracefully V2: - Avoid memory leak Viktor Mihajlovski (2): util: Allow to query the presence of host CPU bitmaps qemu: Allow use of hot plugged host CPUs if no affinity set src/libvirt_private.syms | 1 + src/qemu/qemu_process.c | 33 ++++++++++++++++++++++++--------- src/util/virhostcpu.c | 10 ++++++++++ src/util/virhostcpu.h | 1 + 4 files changed, 36 insertions(+), 9 deletions(-) -- 1.9.1
The functions to retrieve online and present host CPU information are only supported on Linux for the time being. This leads to runtime errors if these function are used on other platforms. To avoid that, code in higher levels using the functions must replicate the conditional compilation in higher level which is error prone (and is plainly spoken ugly). Adding a function virHostCPUHasBitmap that can be used to check for host CPU bitmap support. NB: There are other functions including the host CPU count that are lacking support on all platforms, but they are too essential in order to be bypassed. Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> --- src/libvirt_private.syms | 1 + src/util/virhostcpu.c | 10 ++++++++++ src/util/virhostcpu.h | 1 + 3 files changed, 12 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index b7d26fd..7468623 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1118,6 +1118,7 @@ virHostCPUGetOnlineBitmap; virHostCPUGetPresentBitmap; virHostCPUGetStats; virHostCPUGetThreadsPerSubcore; +virHostCPUHasBitmap; virHostCPUStatsAssign; virHostMemAllocPages; virHostMemGetCellsFree; diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c index f68176f..ec2469d 100644 --- a/src/util/virhostcpu.c +++ b/src/util/virhostcpu.c @@ -1112,6 +1112,16 @@ virHostCPUGetCount(void) #endif } +bool +virHostCPUHasBitmap(void) +{ +#ifdef __linux__ + return true; +#else + return false; +#endif +} + virBitmapPtr virHostCPUGetPresentBitmap(void) { diff --git a/src/util/virhostcpu.h b/src/util/virhostcpu.h index b048704..39f7cf8 100644 --- a/src/util/virhostcpu.h +++ b/src/util/virhostcpu.h @@ -35,6 +35,7 @@ int virHostCPUGetStats(int cpuNum, int *nparams, unsigned int flags); +bool virHostCPUHasBitmap(void); virBitmapPtr virHostCPUGetPresentBitmap(void); virBitmapPtr virHostCPUGetOnlineBitmap(void); int virHostCPUGetCount(void); -- 1.9.1
On 11/25/2016 08:57 AM, Viktor Mihajlovski wrote:
The functions to retrieve online and present host CPU information are only supported on Linux for the time being.
This leads to runtime errors if these function are used on other platforms. To avoid that, code in higher levels using the functions must replicate the conditional compilation in higher level which is error prone (and is plainly spoken ugly).
Adding a function virHostCPUHasBitmap that can be used to check for host CPU bitmap support.
NB: There are other functions including the host CPU count that are lacking support on all platforms, but they are too essential in order to be bypassed.
Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> --- src/libvirt_private.syms | 1 + src/util/virhostcpu.c | 10 ++++++++++ src/util/virhostcpu.h | 1 + 3 files changed, 12 insertions(+)
ACK John
If the cpuset cgroup controller is disabled in /etc/libvirt/qemu.conf QEMU virtual machines can in principle use all host CPUs, even if they are hot plugged, if they have no explicit CPU affinity defined. However, there's libvirt code supposed to handle the situation where the libvirt daemon itself is not using all host CPUs. The code in qemuProcessInitCpuAffinity attempts to set an affinity mask including all defined host CPUs. Unfortunately, the resulting affinity mask for the process will not contain the offline CPUs. See also the sched_setaffinity(2) man page. That means that even if the host CPUs come online again, they won't be used by the QEMU process anymore. The same is true for newly hot plugged CPUs. So we are effectively preventing that QEMU uses all processors instead of enabling it to use them. It only makes sense to set the QEMU process affinity if we're able to actually grow the set of usable CPUs, i.e. if the process affinity is a subset of the online host CPUs. There's still the chance that for some reason the deliberately chosen libvirtd affinity matches the online host CPU mask by accident. In this case the behavior remains as it was before (CPUs offline while setting the affinity will not be used if they show up later on). Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> Tested-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com> --- src/qemu/qemu_process.c | 33 ++++++++++++++++++++++++--------- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 4758c49..9d1bfa4 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2202,6 +2202,7 @@ qemuProcessInitCpuAffinity(virDomainObjPtr vm) int ret = -1; virBitmapPtr cpumap = NULL; virBitmapPtr cpumapToSet = NULL; + virBitmapPtr hostcpumap = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; if (!vm->pid) { @@ -2223,21 +2224,34 @@ qemuProcessInitCpuAffinity(virDomainObjPtr vm) * the spawned QEMU instance to all pCPUs if no map is given in * its config file */ int hostcpus; + cpumap = virProcessGetAffinity(vm->pid); - /* setaffinity fails if you set bits for CPUs which - * aren't present, so we have to limit ourselves */ - if ((hostcpus = virHostCPUGetCount()) < 0) + if (virHostCPUHasBitmap()) + hostcpumap = virHostCPUGetOnlineBitmap(); + + if (hostcpumap && cpumap && virBitmapEqual(hostcpumap, cpumap)) { + /* we're using all available CPUs, no reason to set + * mask. If libvirtd is running without explicit + * affinity, we can use hotplugged CPUs for this VM */ + ret = 0; goto cleanup; + } else { + /* setaffinity fails if you set bits for CPUs which + * aren't present, so we have to limit ourselves */ + if ((hostcpus = virHostCPUGetCount()) < 0) + goto cleanup; - if (hostcpus > QEMUD_CPUMASK_LEN) - hostcpus = QEMUD_CPUMASK_LEN; + if (hostcpus > QEMUD_CPUMASK_LEN) + hostcpus = QEMUD_CPUMASK_LEN; - if (!(cpumap = virBitmapNew(hostcpus))) - goto cleanup; + virBitmapFree(cpumap); + if (!(cpumap = virBitmapNew(hostcpus))) + goto cleanup; - virBitmapSetAll(cpumap); + virBitmapSetAll(cpumap); - cpumapToSet = cpumap; + cpumapToSet = cpumap; + } } } @@ -2248,6 +2262,7 @@ qemuProcessInitCpuAffinity(virDomainObjPtr vm) cleanup: virBitmapFree(cpumap); + virBitmapFree(hostcpumap); return ret; } -- 1.9.1
On 11/25/2016 02:57 PM, Viktor Mihajlovski wrote:
If the cpuset cgroup controller is disabled in /etc/libvirt/qemu.conf QEMU virtual machines can in principle use all host CPUs, even if they are hot plugged, if they have no explicit CPU affinity defined.
However, there's libvirt code supposed to handle the situation where the libvirt daemon itself is not using all host CPUs. The code in qemuProcessInitCpuAffinity attempts to set an affinity mask including all defined host CPUs. Unfortunately, the resulting affinity mask for the process will not contain the offline CPUs. See also the sched_setaffinity(2) man page.
That means that even if the host CPUs come online again, they won't be used by the QEMU process anymore. The same is true for newly hot plugged CPUs. So we are effectively preventing that QEMU uses all processors instead of enabling it to use them.
It only makes sense to set the QEMU process affinity if we're able to actually grow the set of usable CPUs, i.e. if the process affinity is a subset of the online host CPUs.
There's still the chance that for some reason the deliberately chosen libvirtd affinity matches the online host CPU mask by accident. In this case the behavior remains as it was before (CPUs offline while setting the affinity will not be used if they show up later on).
FWIW, newly started QEMU do indeed use new CPUs, only the ones that are currently running have this bug. So this fix does not only allow hotplug it also fixes an imbalance for running/new QEMUs. So I think it makes a lot of sense to also allow running QEMUs to use new CPUs. Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> Tested-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>
--- src/qemu/qemu_process.c | 33 ++++++++++++++++++++++++--------- 1 file changed, 24 insertions(+), 9 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 4758c49..9d1bfa4 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2202,6 +2202,7 @@ qemuProcessInitCpuAffinity(virDomainObjPtr vm) int ret = -1; virBitmapPtr cpumap = NULL; virBitmapPtr cpumapToSet = NULL; + virBitmapPtr hostcpumap = NULL; qemuDomainObjPrivatePtr priv = vm->privateData;
if (!vm->pid) { @@ -2223,21 +2224,34 @@ qemuProcessInitCpuAffinity(virDomainObjPtr vm) * the spawned QEMU instance to all pCPUs if no map is given in * its config file */ int hostcpus; + cpumap = virProcessGetAffinity(vm->pid);
- /* setaffinity fails if you set bits for CPUs which - * aren't present, so we have to limit ourselves */ - if ((hostcpus = virHostCPUGetCount()) < 0) + if (virHostCPUHasBitmap()) + hostcpumap = virHostCPUGetOnlineBitmap(); + + if (hostcpumap && cpumap && virBitmapEqual(hostcpumap, cpumap)) { + /* we're using all available CPUs, no reason to set + * mask. If libvirtd is running without explicit + * affinity, we can use hotplugged CPUs for this VM */ + ret = 0; goto cleanup; + } else { + /* setaffinity fails if you set bits for CPUs which + * aren't present, so we have to limit ourselves */ + if ((hostcpus = virHostCPUGetCount()) < 0) + goto cleanup;
- if (hostcpus > QEMUD_CPUMASK_LEN) - hostcpus = QEMUD_CPUMASK_LEN; + if (hostcpus > QEMUD_CPUMASK_LEN) + hostcpus = QEMUD_CPUMASK_LEN;
- if (!(cpumap = virBitmapNew(hostcpus))) - goto cleanup; + virBitmapFree(cpumap); + if (!(cpumap = virBitmapNew(hostcpus))) + goto cleanup;
- virBitmapSetAll(cpumap); + virBitmapSetAll(cpumap);
- cpumapToSet = cpumap; + cpumapToSet = cpumap; + } } }
@@ -2248,6 +2262,7 @@ qemuProcessInitCpuAffinity(virDomainObjPtr vm)
cleanup: virBitmapFree(cpumap); + virBitmapFree(hostcpumap); return ret; }
On 11/25/2016 08:57 AM, Viktor Mihajlovski wrote:
If the cpuset cgroup controller is disabled in /etc/libvirt/qemu.conf QEMU virtual machines can in principle use all host CPUs, even if they are hot plugged, if they have no explicit CPU affinity defined.
However, there's libvirt code supposed to handle the situation where the libvirt daemon itself is not using all host CPUs. The code in qemuProcessInitCpuAffinity attempts to set an affinity mask including all defined host CPUs. Unfortunately, the resulting affinity mask for the process will not contain the offline CPUs. See also the sched_setaffinity(2) man page.
That means that even if the host CPUs come online again, they won't be used by the QEMU process anymore. The same is true for newly hot plugged CPUs. So we are effectively preventing that QEMU uses all processors instead of enabling it to use them.
It only makes sense to set the QEMU process affinity if we're able to actually grow the set of usable CPUs, i.e. if the process affinity is a subset of the online host CPUs.
There's still the chance that for some reason the deliberately chosen libvirtd affinity matches the online host CPU mask by accident. In this case the behavior remains as it was before (CPUs offline while setting the affinity will not be used if they show up later on).
Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> Tested-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com> --- src/qemu/qemu_process.c | 33 ++++++++++++++++++++++++--------- 1 file changed, 24 insertions(+), 9 deletions(-)
ACK w/ one slight adjustment noted below and both are now pushed. John
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 4758c49..9d1bfa4 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2202,6 +2202,7 @@ qemuProcessInitCpuAffinity(virDomainObjPtr vm) int ret = -1; virBitmapPtr cpumap = NULL; virBitmapPtr cpumapToSet = NULL; + virBitmapPtr hostcpumap = NULL; qemuDomainObjPrivatePtr priv = vm->privateData;
if (!vm->pid) { @@ -2223,21 +2224,34 @@ qemuProcessInitCpuAffinity(virDomainObjPtr vm) * the spawned QEMU instance to all pCPUs if no map is given in * its config file */ int hostcpus; + cpumap = virProcessGetAffinity(vm->pid);
Moved this slightly... It's now: if (virHostCPUHasBitmap()) { hostcpumap = virHostCPUGetOnlineBitmap(); cpumap = virProcessGetAffinity(vm->pid); } Since cpumap would only be useful in that virBitmapEqual comparison if the HasBitmap returns true. It's still Free'd in the } else { prior to refetching and setting all.
- /* setaffinity fails if you set bits for CPUs which - * aren't present, so we have to limit ourselves */ - if ((hostcpus = virHostCPUGetCount()) < 0) + if (virHostCPUHasBitmap()) + hostcpumap = virHostCPUGetOnlineBitmap(); + + if (hostcpumap && cpumap && virBitmapEqual(hostcpumap, cpumap)) { + /* we're using all available CPUs, no reason to set + * mask. If libvirtd is running without explicit + * affinity, we can use hotplugged CPUs for this VM */ + ret = 0; goto cleanup; + } else { + /* setaffinity fails if you set bits for CPUs which + * aren't present, so we have to limit ourselves */ + if ((hostcpus = virHostCPUGetCount()) < 0) + goto cleanup;
- if (hostcpus > QEMUD_CPUMASK_LEN) - hostcpus = QEMUD_CPUMASK_LEN; + if (hostcpus > QEMUD_CPUMASK_LEN) + hostcpus = QEMUD_CPUMASK_LEN;
- if (!(cpumap = virBitmapNew(hostcpus))) - goto cleanup; + virBitmapFree(cpumap); + if (!(cpumap = virBitmapNew(hostcpus))) + goto cleanup;
- virBitmapSetAll(cpumap); + virBitmapSetAll(cpumap);
- cpumapToSet = cpumap; + cpumapToSet = cpumap; + } } }
@@ -2248,6 +2262,7 @@ qemuProcessInitCpuAffinity(virDomainObjPtr vm)
cleanup: virBitmapFree(cpumap); + virBitmapFree(hostcpumap); return ret; }
On 14.12.2016 00:27, John Ferlan wrote:
On 11/25/2016 08:57 AM, Viktor Mihajlovski wrote:
If the cpuset cgroup controller is disabled in /etc/libvirt/qemu.conf QEMU virtual machines can in principle use all host CPUs, even if they are hot plugged, if they have no explicit CPU affinity defined.
However, there's libvirt code supposed to handle the situation where the libvirt daemon itself is not using all host CPUs. The code in qemuProcessInitCpuAffinity attempts to set an affinity mask including all defined host CPUs. Unfortunately, the resulting affinity mask for the process will not contain the offline CPUs. See also the sched_setaffinity(2) man page.
That means that even if the host CPUs come online again, they won't be used by the QEMU process anymore. The same is true for newly hot plugged CPUs. So we are effectively preventing that QEMU uses all processors instead of enabling it to use them.
It only makes sense to set the QEMU process affinity if we're able to actually grow the set of usable CPUs, i.e. if the process affinity is a subset of the online host CPUs.
There's still the chance that for some reason the deliberately chosen libvirtd affinity matches the online host CPU mask by accident. In this case the behavior remains as it was before (CPUs offline while setting the affinity will not be used if they show up later on).
Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> Tested-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com> --- src/qemu/qemu_process.c | 33 ++++++++++++++++++++++++--------- 1 file changed, 24 insertions(+), 9 deletions(-)
ACK w/ one slight adjustment noted below and both are now pushed.
John
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 4758c49..9d1bfa4 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2202,6 +2202,7 @@ qemuProcessInitCpuAffinity(virDomainObjPtr vm) int ret = -1; virBitmapPtr cpumap = NULL; virBitmapPtr cpumapToSet = NULL; + virBitmapPtr hostcpumap = NULL; qemuDomainObjPrivatePtr priv = vm->privateData;
if (!vm->pid) { @@ -2223,21 +2224,34 @@ qemuProcessInitCpuAffinity(virDomainObjPtr vm) * the spawned QEMU instance to all pCPUs if no map is given in * its config file */ int hostcpus; + cpumap = virProcessGetAffinity(vm->pid);
Moved this slightly... It's now:
if (virHostCPUHasBitmap()) { hostcpumap = virHostCPUGetOnlineBitmap(); cpumap = virProcessGetAffinity(vm->pid); }
Since cpumap would only be useful in that virBitmapEqual comparison if the HasBitmap returns true.
It's still Free'd in the } else { prior to refetching and setting all.
Makes sense, thanks! -- 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
On 25.11.2016 14:57, Viktor Mihajlovski wrote:
This is a rework of the original patch allowing to use all host CPUs if the cpuset controller is not configured by libvirt.
The major enhancement is that we won't try to retrieve the online host CPU map on platforms where this is not supported and just fall back to the old behavior.
V3: - Added new function (in seperate patch) to find out whether the host CPU online map can be retrieved gracefully
V2: - Avoid memory leak
Viktor Mihajlovski (2): util: Allow to query the presence of host CPU bitmaps qemu: Allow use of hot plugged host CPUs if no affinity set
src/libvirt_private.syms | 1 + src/qemu/qemu_process.c | 33 ++++++++++++++++++++++++--------- src/util/virhostcpu.c | 10 ++++++++++ src/util/virhostcpu.h | 1 + 4 files changed, 36 insertions(+), 9 deletions(-)
a friendly season's ping ... -- 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
participants (3)
-
Christian Borntraeger -
John Ferlan -
Viktor Mihajlovski