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

[Markus Armbruster]

"Daniel P. Berrange" <address@hidden> writes:

> 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 <address@hidden>
>> ---
>>  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

I'll stick it in.  I'll also fix it elsewhere in a separate patch.

> Reviewed-by: Daniel P. Berrange <address@hidden>

Thanks!