On Wed, 29 Apr 2015 14:43:56 +0200
Markus Armbruster <arm...@redhat.com> wrote:
> Cornelia Huck <cornelia.h...@de.ibm.com> writes:
>
> > From: Xu Wang <gesa...@linux.vnet.ibm.com>
> >
> > A new sysbus device for diag288 watchdog, it monitors the kvm guest
> > status, and take corresponding actions when it detects that the guest
> > is not responding.
> >
> > Signed-off-by: Xu Wang <gesa...@linux.vnet.ibm.com>
> > Reviewed-by: David Hildenbrand <d...@linux.vnet.ibm.com>
> > Signed-off-by: Cornelia Huck <cornelia.h...@de.ibm.com>
> > ---
> > default-configs/s390x-softmmu.mak | 1 +
> > hw/watchdog/Makefile.objs | 1 +
> > hw/watchdog/wdt_diag288.c | 110
> > ++++++++++++++++++++++++++++++++++++++
> > include/hw/watchdog/wdt_diag288.h | 36 +++++++++++++
> > qemu-options.hx | 6 ++-
> > 5 files changed, 152 insertions(+), 2 deletions(-)
> > create mode 100644 hw/watchdog/wdt_diag288.c
> > create mode 100644 include/hw/watchdog/wdt_diag288.h
> > diff --git a/qemu-options.hx b/qemu-options.hx
> > index 319d971..7ef8f50 100644
> > --- a/qemu-options.hx
> > +++ b/qemu-options.hx
> > @@ -3115,7 +3115,7 @@ when the shift value is high (how high depends on the
> > host machine).
> > ETEXI
> >
> > DEF("watchdog", HAS_ARG, QEMU_OPTION_watchdog, \
> > - "-watchdog i6300esb|ib700\n" \
> > + "-watchdog i6300esb|ib700|diag288\n" \
> > " enable virtual hardware watchdog [default=none]\n",
> > QEMU_ARCH_ALL)
> > STEXI
>
> Preexisting: the help text assumes that all these watchdogs are actually
> compiled in. Generally not the case. Instead of adding another one
> that's generally not available, please change the help text in a
> preliminary patch to something like
>
> DEF("watchdog", HAS_ARG, QEMU_OPTION_watchdog, \
> "-watchdog model\n" \
> " enable virtual hardware watchdog [default none]\n",
> QEMU_ARCH_ALL)
> STEXI
> @item -watchdog @var{model}
> @findex -watchdog
> Create a virtual hardware watchdog device. Once enabled (by a guest
> action), the watchdog must be periodically polled by an agent inside
> the guest or else the guest will be restarted.
>
> The @var{model} is the model of hardware watchdog to emulate. Use
> @code{-watchdog help} to list available hardware models. Only one
> watchdog can be enabled for a guest.
Maybe keep "Choose a watchdog for which your guest has drivers."?
>
> The following models may be available:
> @table @option
> @item ib700
> iBASE 700 is a very simple ISA watchdog with a single timer.
> @item i6300esb
> Intel 6300ESB I/O controller hub is a much more featureful PCI-based
> dual-timer watchdog.
> @end table
> ETEXI
I like that.
>
> > @@ -3129,7 +3129,9 @@ The @var{model} is the model of hardware watchdog to
> > emulate. Choices
> > for model are: @code{ib700} (iBASE 700) which is a very simple ISA
> > watchdog with a single timer, or @code{i6300esb} (Intel 6300ESB I/O
> > controller hub) which is a much more featureful PCI-based dual-timer
> > -watchdog. Choose a model for which your guest has drivers.
> > +watchdog. @code{diag288} is a watchdog device only available on s390x
> > +(for now only supported by KVM). Choose a model for which your guest
> > +has drivers.
Hm... let's make that
@item diag288
A virtual watchdog for s390x backed by the diagnose 288 hypercall (currently
KVM only).
?
> >
> > Use @code{-watchdog help} to list available hardware models. Only one
> > watchdog can be enabled for a guest.
>
> Recommend to split this patch: one patch to add the device model, and
> another one to wire up -watchdog.
So we end up with three patches:
- clean up help text
- add device (probably as a plain device)
- wire up -watchdog
Sounds sensible.
