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.eb...@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?

>  PVE/API2/Qemu.pm | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index c31dd1d..16a7a04 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -2111,7 +2111,7 @@ __PACKAGE__->register_method({
>  
>               print "Requesting HA stop for VM $vmid\n";
>  
> -             my $cmd = ['ha-manager', 'set',  "vm:$vmid", '--state', 
> 'stopped'];
> +             my $cmd = ['ha-manager', 'crm-command', 'stop',  "vm:$vmid", 
> '0'];
>               PVE::Tools::run_command($cmd);
>               return;
>           };
> @@ -2204,7 +2204,7 @@ __PACKAGE__->register_method({
>           timeout => {
>               description => "Wait maximal timeout seconds.",
>               type => 'integer',
> -             minimum => 0,
> +             minimum => 1,
>               optional => 1,
>           },
>           forceStop => {
> @@ -2267,12 +2267,13 @@ __PACKAGE__->register_method({
>  
>       if (PVE::HA::Config::vm_is_ha_managed($vmid) && $rpcenv->{type} ne 
> 'ha') {
>  
> +         my $timeout = $param->{timeout} // 60;
>           my $hacmd = sub {
>               my $upid = shift;
>  
>               print "Requesting HA stop for VM $vmid\n";
>  
> -             my $cmd = ['ha-manager', 'set', "vm:$vmid", '--state', 
> 'stopped'];
> +             my $cmd = ['ha-manager', 'crm-command', 'stop', "vm:$vmid", 
> "$timeout"];
>               PVE::Tools::run_command($cmd);
>               return;
>           };
> 


_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Reply via email to