[pve-devel] [PATCH qemu-server] qmreboot: clear reboot request if reboot fails

[Thomas Lamprecht]

On 11/11/19 5:29 PM, Oguz Bektas wrote:
> the reboot request is only cleaned in the vm_start path, so if reboot
> fails for some reason, the request still exists. this causes an
> unintentional reboot when a shutdown/stop/hibernate is called.
> 
> to mitigate, we can just clear the reboot request in case of an error.
> 
> Signed-off-by: Oguz Bektas <o.bektas at proxmox.com>
> ---
>  PVE/QemuServer.pm | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index a50b4e1..6361785 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -5794,15 +5794,22 @@ sub vm_reboot {
>      my ($vmid, $timeout) = @_;
>  
>      PVE::QemuConfig->lock_config($vmid, sub {
> +	eval {
>  
> -	# only reboot if running, as qmeventd starts it again on a stop event
> -	return if !check_running($vmid);
> +	    # only reboot if running, as qmeventd starts it again on a stop event
> +	    return if !check_running($vmid);
>  
> -	create_reboot_request($vmid);
> +	    create_reboot_request($vmid);
>  
> -	my $storecfg = PVE::Storage::config();
> -	_do_vm_stop($storecfg, $vmid, undef, undef, $timeout, 1);
> +	    my $storecfg = PVE::Storage::config();
> +	    _do_vm_stop($storecfg, $vmid, undef, undef, $timeout, 1);
>  
> +	};
> +	if (my $err = $@) {
> +	    # clear reboot request if reboot fails for some reason
> +	    clear_reboot_request($vmid);
> +	    die $err;
> +	}
>     });
>  }
>  
> 

applied, nice catch!
Changed the comment though, as it was it did not really help, I mean
the method name already said that it will clear a reboot request, and
we're clearly in an error path here, so describing the obvious does not
adds value :) Some "why we do this" comment may help more.