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

[Ian Jackson <ian.jackson@xxxxxxxxxx>]

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
   guaranteed.

 * 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.

Thanks,
Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel