Re: [Xen-devel] [PATCH v2 1/2] libxl: Implement the handler to handle unrecoverable AER errors.

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Fri, Jul 28, 2017 at 04:56:56PM -0700, Venu Busireddy wrote:
> On 2017-07-28 17:39:52 +0100, Ian Jackson wrote:
> > Venu Busireddy writes ("[PATCH v2 1/2] libxl: Implement the handler to 
> > handle unrecoverable AER errors."):
> > > Implement the callback function to handle unrecoverable AER errors, and
> > > also the public APIs that can be used to register/unregister the handler.
> > > When an AER error occurs, the handler will forcibly remove the erring
> > > PCIe device from the guest.
> > 
> > Why is this only sometimes the right thing to do ?  On what basis
> > might a user choose ?
> 
> This is not an "only sometimes" thing. User doesn't choose it. We always
> want to watch for AER errors.
> 
> > If this is always the right thing to do then maybe we need to recast
> > this as a general "please run monitoring for this domain" call ?
> 
> What does "recast" imply? Which "call" are you referring to?
> 
> > > +int libxl_reg_aer_events_handler(libxl_ctx *, uint32_t) 
> > > LIBXL_EXTERNAL_CALLERS_ONLY;
> > > +void libxl_unreg_aer_events_handler(libxl_ctx *, uint32_t) 
> > > LIBXL_EXTERNAL_CALLERS_ONLY;
> > 
> > I think these names are very unintuitive.  They describe the
> > implementation, not the effect.
> 
> These names can be changed to anything you want. Please suggest any names
> of your choice, and I will change them. That ensures that we don't spend
> more review revisions in fine tuning those names.
> 
> > The API seems awkward.  Inside libxl, events are only processed while
> > the application is inside libxl.  So for these functions to be
> > effective, the calling application must arrange to be running the
> > libxl event loop.  This should be documented, at least.
> 
> I am afraid I don't follow you. This scheme is tested and it works. So, I
> do not follow you when you say, "...for these functions to be effective,..."

Libxl has an internal event loop. The code as-is registers a watch which
runs on the internal event loop. The event loop's event is only
processed when the process enters libxl (calls libxl functions).

Imagine a toolstack which doesn't use libxl's internal event loop -- for
example libvirt has its own event loop, or a toolstack which only calls
the register function then nothing else. Your API would not work for
those cases.

Ian, please correct me if I'm wrong.

> 
> > What happens if more than one process calls this at once ?
> 
> Each call is handled within a separate 'xl' process's context. I don't
> see a problem there. Do you have anything specific in mind?
> 

It is possible that both xl processes see its watch fires and try to
write to the same node at the same time.

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