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

[Roger Pau Monne <roger.pau@citrix.com>]

Ian Jackson wrote:
> 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 ?

I think we should either rename the strucutres to nic also (because the 
also hold ioemu cards), or get rid of nic/vif an name everything net.

>
> In any case,
>
> Acked-by: Ian Jackson<ian.jackson@eu.citrix.com>
>
> Ian.