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

[Eric Blake <eblake@redhat.com>]

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

On 05/27/2015 06:35 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 the 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>
> ---
>  docs/qdev-device-use.txt |  4 ++++
>  qemu-options.hx          | 36 ++++++++++++++--------------
>  vl.c                     | 61 +++++++++++++++++++++++++++++++++++-------------
>  3 files changed, 68 insertions(+), 33 deletions(-)
> 

> +++ b/qemu-options.hx
> @@ -3115,12 +3115,19 @@ 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" \
> -    "                enable virtual hardware watchdog [default=none]\n",
> +    "-watchdog [model=]i6300esb|ib700,action=reset|shutdown|poweroff|pause|debug|none\n" \

Should this be

-watchdog [model=]i6300esb|ib700[,action=...none]

to make it obvious that ,action= is optional for back-compat with old usage?

> +    "                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}

same here.

-- 
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 --]