Re: [Xen-devel] [PATCH 1/2] libxl: fix race in libxl__devices_destroy

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2012-07-31 at 15:44 +0100, Ian Jackson wrote:
> Ian Campbell writes ("Re: [PATCH 1/2] libxl: fix race in 
> libxl__devices_destroy"):
> > On Fri, 2012-07-27 at 19:26 +0100, Ian Jackson wrote:
> > 
> > > +void libxl__multidev_prepared(libxl__egc *egc, libxl__ao_devices *aodevs,
> > > +                              int rc)
> > > +{
> > > +    multidev_one_callback(egc, aodevs->preparation);
> > 
> > Don't we need to propagate rc here? Perhaps with 
> >     aodevs->preparation->rc= rc
> > ?
> 
> Yes, good catch.  This pattern where we have to explicitly assign to
> the rc in the aodev is perhaps less than ideal.  I spotted a similar
> bug in one of Roger's patches.
> 
> I wonder if there's a way to get around this.
> 
> > > diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> > > index f0c94e8..7072b09 100644
> > > --- a/tools/libxl/libxl_internal.h
> > > +++ b/tools/libxl/libxl_internal.h
> > > @@ -1810,20 +1810,6 @@ typedef void libxl__device_callback(libxl__egc*, 
> > > libxl__ao_device*);
> > [...]
> > > +/*
> > > + * Multiple devices "multidevs" handling.
> > > + *
> > > + * Firstly, you should
> > > + *    libxl__multidev_begin
> > > + *    multidevs->callback = ...
> > > + * Then zero or more times
> > > + *    libxl__multidev_prepare
> > > + *    libal__initiate_device_{remove/addition}.
> > > + * Finally, once
> > > + *    libxl__multidev_prepared
> > > + * which will result (perhaps reentrantly) in one call to callback().
> > 
> > Briefly mention the multidev vs ao_devices naming? Or just do the rename
> > now in a new patch.
> 
> If you're happy with that renaming now then I will do the latter.

It's mostly mechanical so its pretty safe. Lets do it.



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