[pve-devel] [RFC container] fix #2200: do not forcestop containers when rebooting host

[Thomas Lamprecht]

On 5/7/19 5:19 PM, Oguz Bektas wrote:
> i know, that's why i asked if someone had a better patch. because i
> tried something like this and couldn't get it to work:
> 
> ```
> if (defined($param->{forceStop}) {
>     PVE::LXC::vm_stop($vmid, 1, $timeout)
> } else {
>     PVE::LXC::vm_stop($vmid, 0, $timeout)
> }
> ```
> since i guess we want vm_shutdown to not forcestop by default.
> but it didn't work in my tests. maybe i missed something??

Ahh, now I get what you even want to achieve... What about the following
for pve-manager, which hosts the stop/start all api enpoints called on shutdown
and after boot? Similarly to changes in pve-ha-manager done a while ago:
https://git.proxmox.com/?p=pve-ha-manager.git;a=commitdiff;h=7e92211a2c52e06ff4befaf14ce8c3a93c7c205a

----8<----
diff --git a/PVE/API2/Nodes.pm b/PVE/API2/Nodes.pm
index df47be1f..6dc9e26a 100644
--- a/PVE/API2/Nodes.pm
+++ b/PVE/API2/Nodes.pm
@@ -1678,7 +1678,7 @@ my $create_stop_worker = sub {
        my $timeout =  defined($down_timeout) ? int($down_timeout) : 60;
        print STDERR "Stopping CT $vmid (timeout = $timeout seconds)\n";
        $upid = PVE::API2::LXC::Status->vm_shutdown({node => $nodename, vmid => $vmid,
-                                            timeout => $timeout, forceStop => 1 });
+                                            timeout => $timeout, forceStop => 0 });
     } elsif ($type eq 'qemu') {
        return if !PVE::QemuServer::check_running($vmid, 1);
        my $timeout =  defined($down_timeout) ? int($down_timeout) : 60*3;
--

> 
> maybe it's a better idea to do it like this:
> 
> ```
> $param->{forceStop} = 0 if !defined($param->{forceStop})
> PVE::LXC::vm_stop($vmid, $param->{forceStop}, $timeout
> ```
> 
> but i haven't tested this.
> 
> 
> On Tue, May 07, 2019 at 04:57:51PM +0200, Thomas Lamprecht wrote:
>> On 5/7/19 4:44 PM, Oguz Bektas wrote:
>>> Signed-off-by: Oguz Bektas <o.bektas at proxmox.com>
>>> ---
>>>
>>> so this is quite confusing... i'm not sure if this is a valid fix and
>>> it probably doesn't make much sense... but this was the only way i got
>>> the containers to shutdown gracefully when a reboot/shutdown command
>>> was issued on host. i've tested this on a cluster with all the main
>>> linux distro containers running on it. i noticed on my hardware it
>>> takes maximum 2-3 seconds for a ct to go down when they are abruptly
>>> killed before rebooting the host (never >3s). however with this patch
>>> i noticed the cts are taking more time to shutdown (some ~6s, some
>>> still ~3s).
>>>
>>> can someone test this? maybe offer a better patch ? i tried some other
>>> variants but they didn't work and this one was the most clear one 
>>> (although not very nice looking or logical) that worked.
>>>
>>>  src/PVE/API2/LXC/Status.pm | 6 +++++-
>>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/src/PVE/API2/LXC/Status.pm b/src/PVE/API2/LXC/Status.pm
>>> index 95775fe..5b67498 100644
>>> --- a/src/PVE/API2/LXC/Status.pm
>>> +++ b/src/PVE/API2/LXC/Status.pm
>>> @@ -369,7 +369,11 @@ __PACKAGE__->register_method({
>>>  
>>>  		PVE::LXC::Config->check_lock($conf);
>>>  
>>> -		PVE::LXC::vm_stop($vmid, $param->{forceStop}, $timeout);
>>> +		if ($param->{forceStop}) {
>>> +		    PVE::LXC::vm_stop($vmid, 1, $timeout);
>>> +		} else {
>>> +		    PVE::LXC::vm_stop($vmid, 0, $timeout);
>>> +		}
>>
>> sorry, but that needs to be semantically equivalent, at least if
>> vm_stop doesn't has a "not defined" check for force-stop, and thus
>> would only do so if $force-stop is falsy but defined???
>>
>>>  
>>>  		return;
>>>  	    };
>>>
>>
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>