Re: [Xen-devel] [PATCH v2 05/15] libxl: change libxl__ao_device_remove to libxl__ao_device

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Roger Pau Monne writes ("Re: [PATCH v2 05/15] libxl: change 
libxl__ao_device_remove to libxl__ao_device"):
> Ian Jackson wrote:
> > Roger Pau Monne writes ("[PATCH v2 05/15] libxl: change 
> > libxl__ao_device_remove to libxl__ao_device"):
> >> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> >> index ae5527b..b62110a 100644
> >> --- a/tools/libxl/libxl_internal.h
> >> +++ b/tools/libxl/libxl_internal.h
> >> @@ -1802,6 +1795,44 @@ _hidden void 
> >> libxl__bootloader_init(libxl__bootloader_state *bl);
> > ...
> >> +{
> >> +    aorm->ao = ao;
> >> +    aorm->active = 1;
> >> +    aorm->rc = 0;
> >> +    aorm->base = base;
> >> +}
> >
> > I think a function "init" should set "active" to 0, surely ?
> > Since the operation has not in fact been started.  So perhaps this
> > should just be a memset.
> 
> Forget to answer this, we cannot use active = 0 at init because in the 
> hypothetical case that a device add/remove finished before queuing all 
> the others (so the others would be active == 0) it would mark the ao as 
> complete, since it would see all other operations as finished.

Hmmm.  I see.  Yes.

I think you need to rename _init to _prepare or something then.  And
it will then need a comment explaining the semantics.  Are you allowed
to call the operation start function without calling _prepare, for
example?  Do you have to take any destruction steps after calling just
prepare but changing your mind ? (answer seems to be "no") etc.

Ian.

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