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

[Markus Armbruster]

Michal Privoznik <address@hidden> writes:

> 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>
[...]
>>> +++ 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

-watchdog is a thin wrapper around -device, and all it adds is this
error.  It's quite redundant.  I recommend using -device instead.

"-device i6300esb -device i6300esb" works.  It's just a PCI device after
all.

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

None.  All it would accomplish is complicating things.