Re: [openxt-dev] Re: Follow up on libxl-fix-reboot.patch

[Chris Rogers <crogers122@xxxxxxxxx>]

Hopefully I can provide a little more context.  Here is a link to the patch:

https://github.com/OpenXT/xenclient-oe/blob/master/recipes-extended/xen/files/libxl-fix-reboot.patch

The patch is a bit mis-named.  It does not implement XEN_DOMCTL_SENDTRIGGER_RESET.  It's just a workaround to handle the missing RESET implementation.

Its purpose is to make an HVM guest "reboot" regardless of whether PV tools have been installed and the xenstore interface is listening or not.  From the client perspective that OpenXT is concerned with, this is for ease-of-use for working with HVM guests before PV tools are installed.  To summarize the flow of the patch:

- User input causes high level toolstack, xenmgr, to do xl reboot <domid>

- libxl hits "PV interface not available", so it tries the fallback ACPI reset trigger (but that's not implemented in domctl)

- therefore, the patch changes the RESET trigger to POWER trigger, and sets a 'reboot' flag

- when the xl create process handles the domain_death event for LIBXL_SHUTDOWN_REASON_POWEROFF, we check for our 'reboot' flag.
- It's set, so we set "action" as if we came from a real restart, which makes the xl create process take the 'goto start' codepath to rebuild the domain.

I think we'd like to get rid of this patch, but at the moment I don't have any code or a design to propose that would implement the XEN_DOMCTL_SENDTRIGGER_RESET.

On Mon, Dec 14, 2020 at 7:42 PM Stefano Stabellini <sstabellini@xxxxxxxxxx> wrote:

On Mon, 14 Dec 2020, Rich Persaud wrote:
> (adding xen-devel & toolstack devs)
>
> On Dec 14, 2020, at 16:12, Jason Andryuk <jandryuk@xxxxxxxxx> wrote:
> >
> > On Fri, Dec 11, 2020 at 3:56 PM Chris Rogers <crogers122@xxxxxxxxx> wrote:
> >>
> >> This is a follow up to a request during our roadmapping meeting to clarify the purpose of libxl-fix-reboot.patch on the current version of Xen in OpenXT (4.12).  It's pretty simple.  While the domctl API does define a trigger for reset in xen/include/public/domctl.h:
> >>
> >
> >> The call stack looks like this:
> >>> libxl_send_trigger(ctx, domid, LIBXL_TRIGGER_RESET, 0);
> >>> xc_domain_send_trigger(ctx->xch, domid, XEN_DOMCTL_SENDTRIGGER_RESET, vcupid);
> >>> do_domctl()
> >>> arch_do_domctl()
> >> and reaching the case statement in arch_do_domctl() for XEN_DOMCTL_sendtrigger, with RESET, we get -ENOSYS as illustrated above.
> >
> > Thanks, Chris.  It's surprising that xl trigger reset exists, but
> > isn't wired through to do anything.  And that reboot has a fallback
> > command to something that doesn't work.

I miss some of the context of this thread -- let me try to understand
the issue properly.

It looks like HVM reboot doesn't work properly, or is it HVM reset
(in-guest reset)? It looks like it is implemented by calling "xl trigger
reset", which is implemented by libxl_send_trigger. The call chain leads
to a XEN_DOMCTL_sendtrigger domctl with XEN_DOMCTL_SENDTRIGGER_RESET as
a parameter that is not implemented on x86.

That looks like a pretty serious bug :-)


I imagine the reason why it is in that state is that the main way to
reboot would be to call "xl reboot" which is implemented with the PV
protocol "reboot" write to xenstore?  Either way, the bug should be
fixed.

What does your libxl-fix-reboot.patch patch do? Does it add an
implementation of XEN_DOMCTL_SENDTRIGGER_RESET?