Re: [Qemu-devel] [PATCH v2] watchdog: Allow setting action on the fly

[Michal Privoznik]

On 09/05/2017 10:36 PM, Eric Blake wrote:
> On 09/05/2017 08:22 AM, 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.
>>
>> At the same time, drop local redefinition of the actions eum in
> 
> s/eum/enum/
> 
>> watchdog.h in favour of the ones defined in schema.
>>
>> Inspired by: https://bugzilla.redhat.com/show_bug.cgi?id=1447169
>>
>> Signed-off-by: Michal Privoznik <address@hidden>
>> ---
> 
>> @@ -77,27 +80,17 @@ int select_watchdog(const char *p)
>>  
>>  int select_watchdog_action(const char *p)
>>  {
> 
>> +    int action;
>>  
>> +    action = qapi_enum_parse(WatchdogExpirationAction_lookup, p,
>> +                             WATCHDOG_EXPIRATION_ACTION__MAX, -1, NULL);
> 
> Semantic conflict; you need to rebase now that the qapi enum lookup code
> has landed (see commit ebf677c8 and parents)


D'oh. Okay.

> 
>> +
>> +    case WATCHDOG_EXPIRATION_ACTION__MAX:
>> +        /* keep gcc happy */
>> +        break;
> 
> Could use g_assert_not_reached() here instead.
> 
>> +++ b/qapi-schema.json
>> @@ -6539,3 +6539,12 @@
>>  # Since 2.9
>>  ##
>>  { 'command': 'query-vm-generation-id', 'returns': 'GuidInfo' }
>> +
>> +##
>> +# @watchdog-set-action:
>> +#
>> +# Set watchdog action
>> +#
>> +# Since 2.11
>> +##
>> +{ 'command': 'watchdog-set-action', 'data' : {'action': 
>> 'WatchdogExpirationAction'} }
> 
> Can a machine have more than one watchdog device, in which case you'd
> want a device name to select which watchdog gets which action?  But the
> command-line version that you are replacing seems to be global, so I
> guess you're okay with this interface.
> 

Yeah, I don't think that a guest can have more than one watchdog:

/qemu.git $ ./x86_64-softmmu/qemu-system-x86_64 \
-watchdog ib700 -watchdog i6300esb
qemu-system-x86_64: -watchdog i6300esb: only one watchdog option may be
given

Also, would it make sense? I mean, what would be the benefit of having
two or more watchdogs?

Michal