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

[Chris Rogers <crogers122@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3507 bytes --]

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@kernel.org>
wrote:

> On Mon, 14 Dec 2020, Rich Persaud wrote:
> > (adding xen-devel & toolstack devs)
> >
> > On Dec 14, 2020, at 16:12, Jason Andryuk <jandryuk@gmail.com> wrote:
> > >
> > > On Fri, Dec 11, 2020 at 3:56 PM Chris Rogers <crogers122@gmail.com>
> 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?

[-- Attachment #2: Type: text/html, Size: 4544 bytes --]