Re: [pve-devel] [RFC container 2/4] fix #4474: lxc api: add overrule-shutdown parameter to stop endpoint

[Friedrich Weber <f.weber@proxmox.com>]

On 01/12/2023 10:57, Friedrich Weber wrote:
> On 17/11/2023 14:09, Wolfgang Bumiller wrote:
> [...]
>>>  	    return PVE::LXC::Config->lock_config($vmid, $lockcmd);
>>
>> ^ Here we lock first, then fork the worker, then do `vm_stop` with the
>> config lock inherited.
>>
>> This means that creating multiple shutdown tasks before using one with
>> override=true could cause the override task to cancel the *first* ongoing
>> shutdown task, then move on to the `lock_config` call - in the meantime
>> a second shutdown task acquires this very lock and performs another
>> long-running shutdown, causing the `override` parameter to be
>> ineffective.
> 
> Just to make sure I understand correctly, the scenario is (please
> correct me if I'm wrong):
> 
> * shutdown task #1 has the lock and starts long-running shutdown
> * stop API handler with override kills shutdown task #1, but does not
> acquire the lock yet
> * shutdown task #2 starts, acquires the lock and starts long-running
> shutdown
> * stop task waits for the lock => override flag was ineffective

Discussed this with Wolfgang off-list, posting here for completeness. I
suppose the scenario I sketched is technically possible, but unlikely to
occur in practice (the stop API handler will usually acquire the lock
before shutdown task #2 can).

Wolfgang actually sketched a slightly different scenario, which is
reproducible with containers pretty easily:

* shutdown task #1 has the lock and starts long-running shutdown
* API handler for shutdown task #2 waits for the lock (there is no task yet)
* API handler for stop task #3 (with overrule-shutdown) kills shutdown
task #1, but does not acquire the lock yet
* API handler for shutdown task #2 acquires the lock and runs another
long-running shutdown
* API handler for stop task #3 waits for the lock => overrule-shutdown
flag was ineffective

As pointed out by Wolfgang this happens because container shutdown
currently uses lock-then-fork. VM shutdown, on the other hand, uses
fork-then-lock, so the above can't happen (the stop task with
overrule-shutdown kills both shutdown tasks).

In the next version I'll send a separate patch that switches the
ordering as suggested by Wolfgang.