Re: [Qemu-devel] [PATCH v5 3/3] watchdog: Allow setting action on the fly

["Daniel P. Berrange" <berrange@redhat.com>]

On Thu, Sep 07, 2017 at 10:05:26AM +0200, Michal Privoznik wrote:
> Currently, the only time that users can set watchdog action is at
> the start as all we expose is this -watchdog-action command line
> argument. This is suboptimal when users want to plug the device
> later via monitor. Alternatively, they might want to change the
> action for already existing device on the fly.
> 
> Inspired by: https://bugzilla.redhat.com/show_bug.cgi?id=1447169
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  hw/watchdog/watchdog.c | 8 +++++++-
>  qapi-schema.json       | 9 +++++++++
>  2 files changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/watchdog/watchdog.c b/hw/watchdog/watchdog.c
> index 0d3eeed187..670114ecfe 100644
> --- a/hw/watchdog/watchdog.c
> +++ b/hw/watchdog/watchdog.c
> @@ -29,6 +29,7 @@
>  #include "qapi-event.h"
>  #include "hw/nmi.h"
>  #include "qemu/help_option.h"
> +#include "qmp-commands.h"
>  
>  static WatchdogAction watchdog_action = WATCHDOG_ACTION_RESET;
>  static QLIST_HEAD(watchdog_list, WatchdogTimerModel) watchdog_list;
> @@ -85,7 +86,7 @@ int select_watchdog_action(const char *p)
>      g_free(qapi_value);
>      if (action < 0)
>          return -1;
> -    watchdog_action = action;
> +    qmp_watchdog_set_action(action, &error_abort);
>      return 0;
>  }
>  
> @@ -142,3 +143,8 @@ void watchdog_perform_action(void)
>          assert(0);
>      }
>  }
> +
> +void qmp_watchdog_set_action(WatchdogAction action, Error **errp)
> +{
> +    watchdog_action = action;
> +}
> diff --git a/qapi-schema.json b/qapi-schema.json
> index f3af2cb851..f5db401838 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3143,3 +3143,12 @@
>  # Since 2.9
>  ##
>  { 'command': 'query-vm-generation-id', 'returns': 'GuidInfo' }
> +
> +##
> +# @watchdog-set-action:
> +#
> +# Set watchdog action
> +#
> +# Since 2.11

Not sure how much it matters, but the standard syntax has a ':' following
the 'Since' word

I do notice that query-vm-generation-id violates this rule though, which
I what I guess you copied ?

> +##
> +{ 'command': 'watchdog-set-action', 'data' : {'action': 'WatchdogAction'} }
> --

Assuming the ':' is added in next posting (or by the committer), then

Reviewed-by: Daniel P. Berrange <berrange@redhat.com>

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|