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

[Roger Pau Monne <roger.pau@xxxxxxxxxx>]

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@xxxxxxxxxxxxx>
>
> Ian.

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