On 23.11.2016 18:03, John Ferlan wrote:
>
>
> On 11/04/2016 04:01 AM, Viktor Mihajlovski wrote:
>> Running VMs couldn't use newly hot plugged host CPUs even if the VMs
>> had no CPU pinning defined AND the cpuset controller was disabled in
>> the libvirt qemu configuration.
>
> Add blank lines between paragraphs - just makes it easier to read.
>
>> This was because in this case the process affinity was set by libvirt
>> to all currently present host CPUs in order to avoid situations, where
>> libvirtd was deliberately running on a CPU subset and thus the spawned
>> VMs would be involuntarily restricted to the CPU subset inherited by
>> libvirtd.
>> That however prevents new host CPUs to be utilized when they show up.
>> With this change we will NOT set the VM's affinity mask if it
>> matches the online host CPU mask.
>>
>> 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@xxxxxxxxxxxxxxxxxx>
>> Tested-by: Matthew Rosato <mjrosato@xxxxxxxxxxxxxxxxxx>
>> ---
>> src/qemu/qemu_process.c | 12 ++++++++++++
>> 1 file changed, 12 insertions(+)
>>
>> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
>> index 1b67aee..76f9210 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,6 +2224,16 @@ qemuProcessInitCpuAffinity(virDomainObjPtr vm)
>> * the spawned QEMU instance to all pCPUs if no map is given in
>> * its config file */
>> int hostcpus;
>> + hostcpumap = virHostCPUGetOnlineBitmap();
>> + cpumap = virProcessGetAffinity(vm->pid);
>
> Wouldn't this set 'cpumap' to something that shortly would be
> overwritten by virBitmapNew if we don't jump to cleanup in this patch?
you're right, this will leak memory, I'll rework ...
>
> Beyond that - I'll let someone with more detailed knowledge of
> SetAffinity nuances decide whether avoiding the call is proper.
>
> John
Sure ... in essence setting the affinity is pointless if the process'
cpu affinity matches the online host cpu list as the the resulting mask
is the intersection of both sets. Worse then pointless, as it imposes an
unnecessary restriction on the CPUs usable by the spawned QEMU process.
>
>> +
>> + 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;
>> + }
>>
>> /* setaffinity fails if you set bits for CPUs which
>> * aren't present, so we have to limit ourselves */
>> @@ -2248,6 +2259,7 @@ qemuProcessInitCpuAffinity(virDomainObjPtr vm)
>>
>> cleanup:
>> virBitmapFree(cpumap);
>> + virBitmapFree(hostcpumap);
>> return ret;
>> }
>>
>>
>
--
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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
