[PATCH v2] libxl: adjust handling of libxl_device_nic objects

[Jim Fehlig]

On 5/19/21 3:37 AM, Olaf Hering wrote:
> libxl objects are supposed to be initialized and disposed.
> Adjust libxlMakeNic to use an already initialized object, it is owned by
> the caller.
> 
> Adjust libxlMakeNicList to initialize the list of objects, before they
> are filled by libxlMakeNic. In case of error the objects are disposed
> by libxl_domain_config_dispose.
> 
> The usage of g_new0 is suspicious in the context of libxl because the
> memory allocated via glib is released with plain free() inside libxl.
> 
> Signed-off-by: Olaf Hering <olaf at aepfle.de>
> ---
>   src/libxl/libxl_conf.c | 36 +++++++++++++++++-------------------
>   1 file changed, 17 insertions(+), 19 deletions(-)
> 
> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> index 2d2aab7e66..e4afa578b0 100644
> --- a/src/libxl/libxl_conf.c
> +++ b/src/libxl/libxl_conf.c
> @@ -1345,8 +1345,6 @@ libxlMakeNic(virDomainDef *def,
>           return -1;
>       }
>   
> -    libxl_device_nic_init(x_nic);
> -
>       virMacAddrGetRaw(&l_nic->mac, x_nic->mac);
>   
>       /*
> @@ -1532,39 +1530,39 @@ libxlMakeNicList(virDomainDef *def,  libxl_domain_config *d_config)
>   {
>       virDomainNetDef **l_nics = def->nets;
>       size_t nnics = def->nnets;
> -    libxl_device_nic *x_nics;
>       size_t i, nvnics = 0;
> -
> -    x_nics = g_new0(libxl_device_nic, nnics);
> +    int ret = -1;
>   
>       for (i = 0; i < nnics; i++) {
>           if (virDomainNetGetActualType(l_nics[i]) == VIR_DOMAIN_NET_TYPE_HOSTDEV)
>               continue;
> +        nvnics++;
> +    }
> +    if (!nvnics)
> +        return 0;
> +
> +    d_config->nics = g_new0(libxl_device_nic, nvnics);
> +    d_config->num_nics = nvnics;
> +
> +    for (i = 0; i < nvnics; i++)
> +        libxl_device_nic_init(&d_config->nics[i]);
>   
> -        if (libxlMakeNic(def, l_nics[i], &x_nics[nvnics], false))
> +    for (i = 0; i < nvnics; i++) {
> +        if (libxlMakeNic(def, l_nics[i], &d_config->nics[i], false))
>               goto error;
>           /*
>            * The devid (at least right now) will not get initialized by
>            * libxl in the setup case but is required for starting the
>            * device-model.
>            */
> -        if (x_nics[nvnics].devid < 0)
> -            x_nics[nvnics].devid = nvnics;
> -
> -        nvnics++;
> +        if (d_config->nics[i].devid < 0)
> +            d_config->nics[i].devid = i;
>       }

Same comment as the disk patch: initializing and making the NIC can occur in a 
single loop.

>   
> -    VIR_SHRINK_N(x_nics, nnics, nnics - nvnics);
> -    d_config->nics = x_nics;
> -    d_config->num_nics = nvnics;
> -
> -    return 0;
> +    ret = 0;
>   
>    error:
> -    for (i = 0; i < nnics; i++)
> -        libxl_device_nic_dispose(&x_nics[i]);
> -    VIR_FREE(x_nics);
> -    return -1;
> +    return ret;

Error label can be dropped here too.

Regards,
Jim