Re: [PATCH V4 0/5] Introduce Advanced Watch Dog module

[Paolo Bonzini <pbonzini@redhat.com>]

On 04/03/20 09:06, Zhang, Chen wrote:
>> Hi Eric and Paolo, Can you give some comments about this series?
>>
>>
> No news for a while...
> We already have some users(Cloud Service Provider) try to use is module in their product.
> But they also need to follow the Qemu upstream code.

My main comment about this series is that it's not clear why it is
needed and how to use it.  The documentation includes a demo, but no
description of what is an awd_node, a notification_node and an
opt_script.  I can more or less understand the notification_node and
opt_script role from the documentation, but not entirely because, for
example, the two-host demo has hardcoded IP addresses without saying
which host is which IP address.

The documentation does not describe the protocol, which is absolutely
necessary, and does not describe _why_ the protocol was designed like
that.  Without such documentation it's not clear if, for example, the
watchdog protocol could be implemented as QMP commands (e.g.
start-watchdog, stop-watchdog, notify-watchdog).  Another possibility
could be to use the systemd watchdog protocol, which consists of
essentially three commands (WATCHDOG=1, WATCHDOG=trigger,
WATCHDOG_USEC=...) which are transmitted as datagrams.  Documentation is
important for reviewers to judge the merits of the protocol without (or
before) diving into the code.

In the demo, the opt_script mechanism is currently using the "human"
monitor as opposed to QMP.  The human monitor interface is not stable
and not meant for consumption by management interface.  It is not clear
if this is just a sample usage, and in practice the notification_node
would be outside of QEMU, or not.  In general I would prefer to have the
script as an optional feature, and report the triggering of the watchdog
via QMP events.

Paolo