Re: [PATCH v1 04/10] libxl: separate device add/rm complete callbacks

[Wei Liu <wei.liu2@citrix.com>]

On Wed, Jul 16, 2014 at 05:26:20PM +0100, Ian Campbell wrote:
> On Thu, 2014-07-10 at 15:32 +0100, Wei Liu wrote:
> > This is in preparation for device hotplug / unplug configuration
> > synchronisation. No functional change introduced. This change is
> > necessary because we need to do different things for add and remove.
> > Using a single callback won't meet our need anymore.
> 
> Looking at the next two patches it seems like this is because on add you
> do it at the start and on rm you do it at the end?
> 
> I did wonder if it was possible that you could continue to share the
> same callback if you could add a flag, but the real reason for not
> sharing is that the code you want to add to the remove case is device
> type specific, so you need multiple rm callbacks. I misunderstood your

Yes, RM is type specific, hence this change.

I shall write this in commit message, replacing "do different things" to
make it clearer.

> comment about a single callback to mean a single add+rm callback as
> opposed to a single vs. multiple rm callbacks.
> 
> Given that the actual implementation looks ok to me.
> 
> > +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);
> 
> Elsewhere in this file there is:
> /* disk */
> DEFINE_DEVICE_REMOVE(disk, remove, 0)
> DEFINE_DEVICE_REMOVE(disk, destroy, 1)
> ...
> 
> Perhaps add this call there too? Also that place has a comment with the
> resulting function names to serve as grep fodder. That would be nice
> here too.
> 

Will do.

Wei.

> Ian.
>