[pve-devel] [PATCH qemu-server] Use crm-command stop to allow shutdown with timeout and hard stop for HA

[Thomas Lamprecht]

On 11/13/19 12:55 PM, Fabian Ebner wrote:
> On 11/13/19 9:55 AM, Thomas Lamprecht wrote:
>> On 11/12/19 11:03 AM, Fabian Ebner wrote:
>>> The minimum value for timeout in vm_shutdown is changed from 0 to 1, since a
>>> value of 0 would trigger a hard stop for HA managed VMs. Like this the API
>>> description stays valid for all cases.
>>>
>>> Signed-off-by: Fabian Ebner <f.ebner at proxmox.com>
>>> ---
>>>
>>> In vm_shutdown we'd like to pass along the timeout parameter to the HA stack,
>>> but the value 0 triggers a special behavior there, namely a hard stop.
>>> So either the description needs to be changed to mention this behavior or
>>> we just don't allow a timeout of 0 in vm_shutdown. Since a shutdown with
>>> timeout 0 doesn't really make sense anyways, I opted for the latter.
>>> It's the same situation for containers.
>>>
>>
>> timeout == 0 just means instant stop, I did not checked this, but as
>> both CTs and VMs allow to pass 0 it really smells like it was done by
>> design.. ^^
>>
>> Also, limiting API value ranges could be seen as API breakage, and that
>> should be avoided if possible..
>>
>> What was the behaviour of passing this for a non-HA VM/CT?
>>
> 
> If I interpreted the code correctly:
> 
> * For 'qm shutdown --timeout=0':
> ** For non-HA managed VMs it leads to (depending on whether guest agent is enabled) a qmp command 'guest_shutdown' or 'system_powerdown'.
> Then (since timeout is 0 this happens immediately) depending on --force we either kill with SIGTERM or die with "got timeout".
> ** For HA managed VMs 'qm shutdown --timeout=0' turns into a vm_stop issuing a qmp command 'quit' and we actually wait 60 seconds for that command.
> 
> 
> * For 'pct shutdown --timeout=0':
> ** For non-HA managed containers we always give an extra 5 seconds of timeout, so 5 seconds here.
> ** For HA managed containers it will result in an immediate kill.
> 
> I mean it probably doesn't really matter, there was a discrepancy between what happens for HA managed and non-HA managed VM/CT already.

The discrepancy between HA and non-HA was not was I meant, just
the non-HA part was relevant to see how the API change of disallowing
0 as timeout could affect peoples use-cases.

> I can send a v2 without the new minima if you like.

Yes, please. And the schema change should have been in a second patch
anyway - it was independent of the HA crm-command adaption, and without
that mixed in I could have already applied that one, just FYI :)
Thanks!