[PATCH v4] libxl: adjust handling of libxl_device_nic objects

Jim Fehlig jfehlig at suse.com
Wed May 26 20:23:17 UTC 2021


On 5/25/21 3:55 PM, Olaf Hering wrote:
> Am Tue, 25 May 2021 23:40:59 +0200
> schrieb Olaf Hering <olaf at aepfle.de>:
> 
>> In case of error the objects are disposed by libxl_domain_config_dispose.
> 
> I just realized this is not always true, unless this change is applied on top.
> If libxlMakeNic fails, libxl_domain_config_dispose can not release anything.
> With this additional change each successfully initialized object can be fully disposed.
> 
> Olaf
> 
> --- a/src/libxl/libxl_conf.c
> +++ b/src/libxl/libxl_conf.c
> @@ -1523,13 +1523,14 @@ libxlMakeNicList(virDomainDef *def,  libxl_domain_config *d_config)
>       libxl_device_nic *x_nics;
>       size_t i, nvnics = 0;
>   
> -    x_nics = g_new0(libxl_device_nic, nnics);
> +    d_config->nics = x_nics = g_new0(libxl_device_nic, nnics);
>   
>       for (i = 0; i < nnics; i++) {
>           if (virDomainNetGetActualType(l_nics[i]) == VIR_DOMAIN_NET_TYPE_HOSTDEV)
>               continue;
>   
>           libxl_device_nic_init(&x_nics[nvnics]);
> +        d_config->num_nics = nvnics + 1;

These changes make the function a little more difficult to read IMO. How about 
if we keep the goto, but relabel it to 'out' (another common pattern in 
libvirt)? E.g.

https://listman.redhat.com/archives/libvir-list/2021-May/msg00842.html

Regards,
Jim

>           if (libxlMakeNic(def, l_nics[i], &x_nics[nvnics], false))
>               return -1;
>           /*
> @@ -1544,8 +1545,6 @@ libxlMakeNicList(virDomainDef *def,  libxl_domain_config *d_config)
>       }
>   
>       VIR_SHRINK_N(x_nics, nnics, nnics - nvnics);
> -    d_config->nics = x_nics;
> -    d_config->num_nics = nvnics;
>   
>       return 0;
>   }
> 




More information about the libvir-list mailing list