From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH v2 07/18] libxl: separate device add/rm complete callbacks Date: Wed, 27 Aug 2014 02:41:55 +0100 Message-ID: <1409103715.28009.64.camel@citrix.com> References: <1406744639-28782-1-git-send-email-wei.liu2@citrix.com> <1406744639-28782-8-git-send-email-wei.liu2@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1406744639-28782-8-git-send-email-wei.liu2@citrix.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: Wei Liu Cc: ian.jackson@eu.citrix.com, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On Wed, 2014-07-30 at 19:23 +0100, Wei Liu wrote: > This is in preparation for device hotplug / unplug configuration > synchronisation. No functional change introduced. This change is > necessary because rm is type specific, using a single callback cannot Not "is" but "is going to become" I think? > meet our need. > > Signed-off-by: Wei Liu > --- > tools/libxl/libxl.c | 54 +++++++++++++++++++++++++++++++++++++++------------ > 1 file changed, 42 insertions(+), 12 deletions(-) > > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c > index 9ac2c3d..01dffa0 100644 > --- a/tools/libxl/libxl.c > +++ b/tools/libxl/libxl.c > @@ -1798,22 +1798,26 @@ out: > } > > /******************************************************************************/ > +#define DEVICE_AO_FAILED_MSG \ I don't see why this couldn't be a static log_ao_failure message, but since it is used like a function it should be defined/called as DEVICE_AO_FAILED_MSG() so it looks normal. I think it should just be a function though. > @@ -3537,6 +3541,32 @@ out: > } > > /******************************************************************************/ > +#define DEFINE_DEVICE_RM_AOCOMPLETE(type) \ > + static void device_##type##_rm_aocomplete(libxl__egc *egc, \ > + libxl__ao_device *aodev) \ > + { \ > + STATE_AO_GC(aodev->ao); \ > + if (aodev->rc) { \ > + DEVICE_AO_FAILED_MSG; \ > + goto out; \ > + } \ > + \ > + out: \ Perhaps you are going to add stuff between } and out: in a future patch, but if not then the goto+label are a bit pointless (I see now that you inherited that from the original code, so feel free to leave it if you prefer). > + libxl__ao_complete(egc, ao, aodev->rc); \ > + } > + > +/* > + * device_vtpm_rm_aocomplete > + * device_nic_rm_aocomplete > + * device_disk_rm_aocomplete > + * device_vfb_rm_aocomplete > + * device_vkb_rm_aocomplete > + */ > +DEFINE_DEVICE_RM_AOCOMPLETE(vtpm); > +DEFINE_DEVICE_RM_AOCOMPLETE(nic); > +DEFINE_DEVICE_RM_AOCOMPLETE(disk); > +DEFINE_DEVICE_RM_AOCOMPLETE(vfb); > +DEFINE_DEVICE_RM_AOCOMPLETE(vkb); I was going to suggest merging this with DEFINE_DEVICE_REMOVE but that is used twice for each device so that won't work. But perhaps consider adding these in the same blocks as the usage of DEFINE_DEVICE_REMOVE?