[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



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.

Ian.

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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.