[libvirt] [PATCH V3 2/2] qemu: Allow use of hot plugged host CPUs if no affinity set

[Viktor Mihajlovski]

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 at linux.vnet.ibm.com>
>> Tested-by: Matthew Rosato <mjrosato at 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