Re: spapr watchdog vs watchdog_perform_action() / QMP watchdog-set-action

[David Gibson]

On Sat, Jan 27, 2024 at 01:08:02PM +0000, Peter Maydell wrote:
> On Fri, 26 Jan 2024 at 20:49, Markus Armbruster <armbru@redhat.com> wrote:
> >
> > Peter Maydell <peter.maydell@linaro.org> writes:
> >
> > > Hi; one of the "bitesized tasks" we have listed is to convert
> > > watchdog timers which directly call qemu_system_reset_request() on
> > > watchdog timeout to call watchdog_perform_action() instead. This
> > > means they honour the QMP commands that let the user specifiy
> > > the behaviour on watchdog expiry:
> > > https://www.qemu.org/docs/master/interop/qemu-qmp-ref.html#qapidoc-141
> > > https://www.qemu.org/docs/master/interop/qemu-qmp-ref.html#qapidoc-129
> > > (choices include reset, power off the system, do nothing, etc).
> > >
> > > There are only a few remaining watchdogs that don't use the
> > > watchdog_perform_action() function. In most cases the change
> > > is obvious and easy: just make them do that instead of calling
> > > qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET).
> > >
> > > However, the hw/watchdog/spapr_watchdog.c case is trickier. As
> > > far as I can tell from the sources, this is a watchdog set up via
> > > a hypercall, and the guest makes a choice of "power off, restart,
> > > or dump and restart" for its on-expiry action.
> > >
> > > What should this watchdog's interaction with the watchdog-set-action
> > > QMP command be? If the user says "do X" and the guest says "do Y",
> > > which do we do? (With the current code, we always honour what
> > > the guest asks for and ignore what the user asks for.)
> >
> > Gut reaction: when the user says "do X", the guest should not get a say.
> > But one of the values of X could be "whatever the guest says".

That would also be my inclination.

> Mmm. Slightly awkwardly, we don't currently distinguish between
> "action is reset because the user never expressed a preference"
> and "action is reset because the user specifically asked for that",
> but I guess in theory we could make that distinction. (Conveniently
> there is no QMP action for "query current watchdog-action state",
> so we don't need to worry about reflecting that distinction in the
> QMP interface if we make it.)

I think that change is necessary in order to accomodate this sort of
watchdog with guest-progammable behaviour (which is part of the PAPR
spec, so we shouldn't just ignore it).

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature