Re: [Qemu-devel] [PATCH v2] watchdog: convert to QemuOpts

[Eric Blake <eblake@redhat.com>]

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

On 05/27/2015 07:09 AM, Paolo Bonzini wrote:
> This makes it possible to specify a watchdog action in a configuration file.
> The previous behavior of "-watchdog" is moved to the (implied) "-watchdog
> model" suboption.  This is already more or less obsolete, since it is possible
> to achieve the same effect with "-device", but "-watchdog-action" does not have
> an equivalent.
> 
> One alternative implementation is possible, namely to add an "action"
> property to each watchdog device.  However, boards often have embedded
> watchdog devices; even if they currently don't, these should call
> watchdog_perform_action() so that they are affected by -watchdog-action.
> (This is listed in our BiteSizedTasks wiki page).
> 
> For these boards, "-watchdog action=foo" has two advantages:
> 
> 1) it is much easier to use than a "-global" option, and can be
> configured globally for all boards.
> 
> 2) it is harder to add a property to a device than it is to just
> s/qemu_system_reset_request/watchdog_perform_action/; in some cases,
> the devices are not even qdev-ified at all.  The chance of the conversion
> happening then would basically be zero if one had to add a property as
> a prerequisite.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> 	tweak brackets in help messages [Eric]
> 	fix missing error message for -watchdog action [Richard]
> ---
>  docs/qdev-device-use.txt |  4 ++++
>  qemu-options.hx          | 37 +++++++++++++++--------------
>  vl.c                     | 61 +++++++++++++++++++++++++++++++++++-------------
>  3 files changed, 69 insertions(+), 33 deletions(-)

>  
>  DEF("watchdog", HAS_ARG, QEMU_OPTION_watchdog, \
> -    "-watchdog i6300esb|ib700\n" \
> -    "                enable virtual hardware watchdog [default=none]\n",
> +    "-watchdog [[model=]i6300esb|ib700]\n" \
> +    "          [,action=reset|shutdown|poweroff|pause|debug|none]\n" \
> +    "                enable virtual hardware watchdog (default: model=none,action=reset)\n",
> +    QEMU_ARCH_ALL)
> +DEF("watchdog-action", HAS_ARG, QEMU_OPTION_watchdog_action, \
> +    "-watchdog-action reset|shutdown|poweroff|pause|debug|none\n" \
> +    "                action when watchdog fires (default: reset)\n",
>      QEMU_ARCH_ALL)
>  STEXI
> -@item -watchdog @var{model}
> +@item -watchdog [[model=]@var{model}][,action=@var{action}]

Bikeshedding here:

Hmm - this says I can supply -watchdog with no arguments.  It also
implies that '-watchdog ,action=...' would work.  Maybe we split it into
two lines:

-watchdog [model=]@var{model}
-watchdog [[model=]@var{model},]action=@var{action}

But that gets more verbose than necessary.  I can live with it as-is.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]