Re: [Xen-devel] [PATCH 1/3] libxl: ao: allow immediate completion

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Ian Campbell writes ("Re: [Xen-devel] [PATCH 1/3] libxl: ao: allow immediate 
completion"):
> On Fri, 2012-02-17 at 18:43 +0000, Ian Jackson wrote:
> > -    assert(ao->in_initiator);
> > +    assert(ao->constructing && ao->in_initiator);
> 
> Doing two asserts will make it clearer which condition is not satisfied
> if we ever see this.

Good point.  I was foolishly thinking of bitfield optimisations but
this code is not a hot path.

> > -    AO_GC;
> > +    EGC_INIT(ctx);
> 
> This means that the "gc" within a function which uses this macro is now
> "&egc->gc" rather than "&ao->gc".

You are right, and this is wrong.

> Another difference is that any function calling AO_CREATE cannot now be
> called from within libxl, since it now calls LIBXL__INIT_EGC which has
> this restriction. libxl_cdrom_insert does not obey this new restriction.

This restriction is not new.  libxl_cdrom_insert has always violated
it and needs converting to the new scheme of things.

The restriction is documented here in libxl_internal.h but sadly has a
"outside" when it should say "inside":

 * All "slow" functions (includes anything that might block on a
 * guest or an external script) need to use the asynchronous
 * operation ("ao") machinery.  The function should take a parameter
 * const libxl_asyncop_how *ao_how and must start with a call to
 * AO_INITIATOR_ENTRY.  These functions MAY NOT be called from
 * outside libxl, because they can cause reentrancy callbacks.

In practice I think libxl_cdrom_insert will not mind the reentrancy so
the bug is latent.  It doesn't take out the ctx lock, and although a
callback might involve messing with the same device there is no
difference between that and another process doing the same.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel