[libvirt] [PATCH 4/4] qemu: Fix qemuProcessInitCpuAffinity()

[John Ferlan]

On 5/31/19 11:22 AM, Andrea Bolognani wrote:
> Commit f136b83139c6 made some changes in the way we set CPU
> affinity for QEMU processes, and while doing so unfortunately
> also introduced a logic bug in that it tried to use a NUMA node
> map where a CPU map was expected.
> 
> Because of that, guests using <numatune> would suddenly fail to
> start:
> 
>   # virsh start guest
>   error: Failed to start domain guest
>   error: cannot set CPU affinity on process 40055: Invalid argument
> 
> This was particularly easy to trigger on POWER 8 machines,
> where secondary threads always show up as offline in the host:
> having
> 
>   <numatune>
>     <memory mode='strict' placement='static' nodeset='1'/>
>   </numatune>
> 
> in the guest configuration, for example, would result in libvirt
> trying to set the process affinity so that it would prefer
> running on CPU 1, but since that's a secondary thread and thus
> shows up as offline, the operation would fail, and so would
> starting the guest.
> 
> Use the newly introduced virNumaNodesetToCPUset() to convert
> the NUMA node map to a CPU map, which in the example above would
> be 8-15 with CPU 8 showing up as online in the guest.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1703661
> 
> Signed-off-by: Andrea Bolognani <abologna at redhat.com>
> ---
>  src/qemu/qemu_process.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index fa5909e9be..295ad2e1ff 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -2489,11 +2489,16 @@ qemuProcessInitCpuAffinity(virDomainObjPtr vm)
>      if (virDomainNumaGetNodeCount(vm->def->numa) <= 1 &&
>          virDomainNumatuneGetMode(vm->def->numa, -1, &mem_mode) == 0 &&
>          mem_mode == VIR_DOMAIN_NUMATUNE_MEM_STRICT) {
> +        virBitmapPtr nodeset = NULL;
> +
>          if (virDomainNumatuneMaybeGetNodeset(vm->def->numa,
>                                               priv->autoNodeset,
> -                                             &cpumapToSet,
> +                                             &nodeset,
>                                               -1) < 0)
>              goto cleanup;
> +
> +        if (virNumaNodesetToCPUset(nodeset, &cpumapToSet) < 0)
> +            goto cleanup;

Coverity complained this morning because virNumaNodesetToCPUset will
allocate something into @cpumapToSet which isn't free'd when this code
jumps to cleanup.


John

>      } else if (vm->def->cputune.emulatorpin) {
>          cpumapToSet = vm->def->cputune.emulatorpin;
>      } else {
>