Re: [Xen-devel] [PATCH v2 10/10] libxl: add device backend listener in order to launch backends

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Roger Pau Monné writes ("Re: [PATCH v2 10/10] libxl: add device backend 
listener in order to launch backends"):
> On 15/11/13 18:54, Ian Jackson wrote:
> > I would suggest: rename "skip" to "out".
> 
> Ack, I don't mind changing it to out. I don't see any flow in which we
> could leak the nested_ao right now.

Looking at it again I think I was confused.

> > Would it be possible to abolish the "free_ao" variable, and to change
> > this:
> > 
> >> +        rc = add_device(egc, nested_ao, dguest, ddev);
> >> +        if (rc > 0)
> >> +            free_ao = true;
> > 
> > To this:
> >            if (!rc)
> >                /* device callback requires, and will dispose of,
> >                 * nested_ao; ddev and dguest are linked in */
> >                return;
> > 
> > and always free the ao on ordinary exit ?
> 
> I'm not sure I follow, if instead of setting free_ao to true I just
> return, who is going to free the ddev and dguest if necessary? (note
> that the callback is not using ddev or dguest at all)

Right.  Sorry.

> Also, doing it in the callback is not safe, because we are no longer
> holding the "Big lock", so we would have to add another lock which I
> would prefer to avoid.

That's not actually a problem: all the in-libxl callbacks take place
with the libxl ctx lock held.  But I don't think that's relevant,
actually.

Acked-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>

Ian.

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