[libvirt] [PATCH 1/2 v2] conf: List only online cpus for virsh vcpupin

[Peter Krempa]

On Thu, Nov 10, 2016 at 12:30:00 +0530, 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 at linux.vnet.ibm.com>
> ---
>  src/conf/domain_conf.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index a233c0c..78efbc6 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -55,6 +55,7 @@
>  #include "virtpm.h"
>  #include "virstring.h"
>  #include "virnetdev.h"
> +#include "virhostcpu.h"
>  
>  #define VIR_FROM_THIS VIR_FROM_DOMAIN
>  
> @@ -1543,6 +1544,7 @@ virDomainDefGetVcpuPinInfoHelper(virDomainDefPtr def,
>  {
>      int maxvcpus = virDomainDefGetVcpusMax(def);
>      virBitmapPtr allcpumap = NULL;
> +    virBitmapPtr bitmap = NULL;
>      size_t i;
>  
>      if (hostcpus < 0)
> @@ -1555,13 +1557,16 @@ virDomainDefGetVcpuPinInfoHelper(virDomainDefPtr def,
>  
>      for (i = 0; i < maxvcpus && i < ncpumaps; i++) {
>          virDomainVcpuDefPtr vcpu = virDomainDefGetVcpu(def, i);
> -        virBitmapPtr bitmap = NULL;
> +        bitmap = NULL;
>  
>          if (vcpu && vcpu->cpumask)
>              bitmap = vcpu->cpumask;

By assigning vcpu->cpumask to bitmap here, you free it at the end and
break the domain definition by freeing it's content.

>          else if (def->placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO &&
>                   autoCpuset)
>              bitmap = autoCpuset;
> +        else if (def->placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_STATIC &&
> +                 virHostCPUGetOnlineBitmap())

This first call returns a different instance of the bitmap which is
leaked afterwards since it's not put into a variable.

Additionally on non-linux platforms the function call would report an
error which would not be used later.

> +            bitmap = virHostCPUGetOnlineBitmap();

And this may still return NULL since it's a separate call.

>          else if (def->cpumask)
>              bitmap = def->cpumask;
>          else

Peter
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20161110/8d2606a9/attachment-0001.sig>