[libvirt] [PATCH] numatune: setting --mode does not work well

[Ján Tomko]

On 08/20/2014 12:49 PM, Erik Skultety wrote:
> When trying to set numatune mode directly using virsh numatune command,
> correct error is raised, however numatune structure was not deallocated,
> thus resulting in creating an empty numatune element in the guest XML,
> if none was present before. Running the same command aftewards results
> in a successful change with broken XML structure. Patch fixes the
> deallocation problem as well as checking for invalid attribute
> combination VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO + a nonempty nodeset.
> 
> Resolves https://bugzilla.redhat.com/show_bug.cgi?id=1129998
> ---
>  src/conf/numatune_conf.c | 28 +++++++++++++++++++---------
>  1 file changed, 19 insertions(+), 9 deletions(-)
> 
> diff --git a/src/conf/numatune_conf.c b/src/conf/numatune_conf.c
> index 48d1d04..45bf0cb 100644
> --- a/src/conf/numatune_conf.c
> +++ b/src/conf/numatune_conf.c
> @@ -439,7 +439,7 @@ virDomainNumatuneSet(virDomainNumatunePtr *numatunePtr,
>  {
>      bool create = !*numatunePtr;  /* Whether we are creating new struct */
>      int ret = -1;
> -    virDomainNumatunePtr numatune = NULL;
> +    virDomainNumatunePtr numatune = *numatunePtr;
>  
>      /* No need to do anything in this case */
>      if (mode == -1 && placement == -1 && !nodeset)
> @@ -461,9 +461,15 @@ virDomainNumatuneSet(virDomainNumatunePtr *numatunePtr,
>          goto cleanup;
>      }
>  
> -    if (create && VIR_ALLOC(*numatunePtr) < 0)
> +    if (placement_static && !nodeset) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                       _("nodeset for NUMA memory tuning must be set "
> +                         "if 'placement' is 'static'"));
> +        goto cleanup;

You moved this error above the allocation, but there is another 'goto cleanup'
possible after the successful allocation of 'numatune':

     if (nodeset) {
         virBitmapFree(numatune->memory.nodeset);
         numatune->memory.nodeset = virBitmapNewCopy(nodeset);
         if (!numatune->memory.nodeset)
             goto cleanup;
         if (placement == -1)
             placement = VIR_DOMAIN_NUMATUNE_PLACEMENT_STATIC;
     }

If virBitmapNewCopy fails and numatune was allocated by this function, it's
leaked.

Jan

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140820/0a769e36/attachment-0001.sig>