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

[Venu Busireddy <venu.busireddy@xxxxxxxxxx>]

On 2018-04-03 17:51:50 +0100, Ian Jackson wrote:
> 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.

Sure. I will reply to your email from September.

Venu

> Thanks,
> Ian.

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