From mboxrd@z Thu Jan 1 00:00:00 1970 From: Roger Pau Monne Subject: Re: [PATCH v6 07/13] libxl: convert libxl_device_nic_add to an async operation Date: Tue, 26 Jun 2012 17:17:21 +0100 Message-ID: <4FE9E091.2090001@citrix.com> References: <1339676475-33265-1-git-send-email-roger.pau@citrix.com> <1339676475-33265-8-git-send-email-roger.pau@citrix.com> <20452.22802.102071.107153@mariner.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20452.22802.102071.107153@mariner.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Jackson Cc: Ian Campbell , "xen-devel@lists.xen.org" List-Id: xen-devel@lists.xenproject.org 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.