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

Re: [Xen-devel] [PATCH v3 1/2] libxl: Implement the handler to handle unrecoverable AER errors [and 1 more messages]

Venu Busireddy writes ("Re: [PATCH v3 1/2] libxl: Implement the handler to 
handle unrecoverable AER errors [and 1 more messages]"):
> On 2018-04-03 16:06:17 +0100, Ian Jackson wrote:
> > Ian Jackson writes ("Re: [PATCH v3 1/2] libxl: Implement the handler to 
> > handle unrecoverable AER errors"):
> > > I'm afraid that I still have reservations about the design questions.
> > > Evidently I didn't make my questions clear enough.
> > > 
> > > [ 64 lines of detailed discussion elided ]
> > 
> > I haven't seen a reply to that.
> Reply to that is the v5 patch. Your concern in v4 was, "why is this
> error handling done only in some cases?" Meaning, the error handling
> happens only for guests created using xl, but it does not happen for
> guests created using libvirt. I addressed that in the v5 patch. Please
> see below for more details.

Oh.  I see.

> > I'm confused by the responses in the thread which relate to libvirt.
> > ISTM that a libvirt patch is also required.  Do you mean that in v5
> > there is also a libvirt patch ?
> libvirt ends up calling do_domain_create() in tools/libxl/libxl_create.c,
> and that is where I am registering the error handler. That change takes
> care of guests created using xl command as well as libvirt. Hence there
> is no change in libvirt.

I'm sorry to say that this is completely wrong.  I didn't spot that
hunk in the v5 2/2 patch.  I don't think your description in your v4
to v5 changes summary really highlights the substantial design change.

I think it would have been better to reply to my prose email.  We
would have been able to explore the design possibilities.

What you have done is wrong because:

 * You have removed the libxl__aer_watch from the
   libxl_reg_aer_events_handler API which means the effect is now
   global for the ctx.  This is not correct for a libxl event
   generation request function.  (Although this isn't one.)

 * Not all callers of libxl will necessarily retain the process, or
   the ctx, in which they called libxl_domain_create.  I think libvirt
   does (but I'm not sure), and xl usually does, but it's not

 * It's quite unclear why this function is a public one.

The entire approach is wrong, I'm afraid.

We need to go back to the design.  Please would you reply to my mail
from September.


Xen-devel mailing list



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