Re: [Xen-devel] [PATCH v6 07/13] libxl: convert libxl_device_nic_add to an async operation

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Roger Pau Monne writes ("[PATCH v6 07/13] libxl: convert libxl_device_nic_add 
to an async operation"):
> This patch converts libxl_device_nic_add to an ao operation that
> waits for device backend to reach state XenbusStateInitWait and then
> marks the operation as completed. This is not really useful now, but
> will be used by latter patches that will launch hotplug scripts after
> we reached the desired xenbus state.
> 
> Calls to libxl_device_nic_add have also been moved to occur after the
> device model has been launched, so when hotplug scripts are called
> from this functions the interfaces already exists.
> 
> As usual, libxl_device_nic_add callers have been modified, and the
> internal function libxl__device_disk_add has been used if the call was
> inside an already running ao.

This all looks reasonable to me.  But seeing this:

> diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
> index 5d34ed5..f7217aa 100644
> --- a/tools/libxl/libxl_device.c
> +++ b/tools/libxl/libxl_device.c
> @@ -454,8 +454,8 @@ void libxl__ao_devices_callback(libxl__egc *egc, 
> libxl__ao_device *aodev)
>  #define libxl__device_disk_add(egc, domid, disk, aodev)                      
>  \
>          libxl__device_disk_add(egc, domid, XBT_NULL, disk, aodev)
> 
> -#define DEFINE_DEVICES_ADD(type)                                             
>  \
> -    void libxl__add_##type##s(libxl__egc *egc, libxl__ao *ao, uint32_t 
> domid, \
> +#define DEFINE_DEVICES_ADD(type, name)                                       
>  \
> +    void libxl__add_##name##s(libxl__egc *egc, libxl__ao *ao, uint32_t 
> domid, \
>                                libxl_domain_config *d_config,                 
>  \
>                                libxl__ao_devices *aodevs,                     
>  \
>                                libxl__devices_callback *callback)             
>  \
> @@ -469,12 +469,13 @@ void libxl__ao_devices_callback(libxl__egc *egc, 
> libxl__ao_device *aodev)
>                                                                               
>  \
>          for (i = 0; i < aodevs->size; i++) {                                 
>  \
>              aodevs->array[i].callback = libxl__ao_devices_callback;          
>  \
> -            libxl__device_##type##_add(egc, domid, &d_config->type##s[i],    
>  \
> +            libxl__device_##name##_add(egc, domid, &d_config->type##s[i],    
>  \
>                                         &aodevs->array[i]);                   
>  \
>          }                                                                    
>  \
>      }
> 
> -DEFINE_DEVICES_ADD(disk)
> +DEFINE_DEVICES_ADD(disk, disk)
> +DEFINE_DEVICES_ADD(vif, nic)

it seems to me that this is an anomaly which might be better fixed
than worked around.

Should we rename the functions libxl_*nic* or the structures *vif* ?
Or should we rename both, to "net" perhaps ?


In any case,

Acked-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel