[libvirt] [PATCHv7 10/18] conf: Remove virDomainResctrlAppend and introduce virDomainResctrlNew

[John Ferlan]

On 10/22/18 4:01 AM, Wang Huaqiang wrote:
> Introduced virDomainResctrlNew to do the most part of virDomainResctrlAppend
> and move the operation of appending resctrl to @def->resctrls out of
> function.
> 
> Rather than rely on virDomainResctrlAppend to perform the allocation, move
> the onus to the caller and make use of virBitmapNewCopy for @vcpus and
> virObjectRef for @alloc, thus removing the need to set each to NULL after the
> call.
> 
> Signed-off-by: Wang Huaqiang <huaqiang.wang at intel.com>
> ---
>  src/conf/domain_conf.c | 60 +++++++++++++++++++++++++++++---------------------
>  1 file changed, 35 insertions(+), 25 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index e8e0adc..39bd396 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -18920,26 +18920,22 @@ virDomainCachetuneDefParseCache(xmlXPathContextPtr ctxt,
>  }
>  
>  
> -static int
> -virDomainResctrlAppend(virDomainDefPtr def,
> -                       xmlNodePtr node,
> -                       virResctrlAllocPtr alloc,
> -                       virBitmapPtr vcpus,
> -                       unsigned int flags)
> +static virDomainResctrlDefPtr
> +virDomainResctrlNew(xmlNodePtr node,
> +                    virResctrlAllocPtr *alloc,
> +                    virBitmapPtr *vcpus,

Because we're not "stealing" @*alloc and/or @*vcpus, they do not need to
be passed by reference. I can change these.  There's some minor merge
impact in later patches too, but no big deal.

> +                    unsigned int flags)
>  {
>      char *vcpus_str = NULL;
>      char *alloc_id = NULL;
> -    virDomainResctrlDefPtr tmp_resctrl = NULL;
> -    int ret = -1;
> -
> -    if (VIR_ALLOC(tmp_resctrl) < 0)
> -        goto cleanup;
> +    virDomainResctrlDefPtr resctrl = NULL;
> +    virDomainResctrlDefPtr ret = NULL;
>  
>      /* We need to format it back because we need to be consistent in the naming
>       * even when users specify some "sub-optimal" string there. */
> -    vcpus_str = virBitmapFormat(vcpus);
> +    vcpus_str = virBitmapFormat(*vcpus);
>      if (!vcpus_str)
> -        goto cleanup;
> +        return NULL;
>  
>      if (!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE))
>          alloc_id = virXMLPropString(node, "id");
> @@ -18954,18 +18950,23 @@ virDomainResctrlAppend(virDomainDefPtr def,
>              goto cleanup;
>      }
>  

    /* NB: Callers assume new @alloc, need to fill in ID now */

Not that it would prevent someone in the future from passing an @alloc
w/ ->id already filled in and overwriting, but at least for now it's not
the case.

With the changes (that I can make),

Reviewed-by: John Ferlan <jferlan at redhat.com>

John

[...]