[pve-devel] [PATCH qemu-server v3 3/4] qm: cleanup: detect and handle reboot trigger

[Thomas Lamprecht]

On 06.09.19 14:24, Dominik Csapak wrote:
> if the reboot trigger file was set, start the vm again
> also cleanup the reboot trigger on vm startup, to prevent
> leftover files to trigger a reboot at the next shutdown
> 
> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
> ---
> changes from v2:
> * warn on error
> * add explaining comment
>  PVE/CLI/qm.pm     | 14 ++++++++++++++
>  PVE/QemuServer.pm |  8 ++++++++
>  2 files changed, 22 insertions(+)
> 
> diff --git a/PVE/CLI/qm.pm b/PVE/CLI/qm.pm
> index 3aae23c..dea3deb 100755
> --- a/PVE/CLI/qm.pm
> +++ b/PVE/CLI/qm.pm
> @@ -763,6 +763,7 @@ __PACKAGE__->register_method({
>  	my $vmid = $param->{vmid};
>  	my $clean = $param->{'clean-shutdown'};
>  	my $guest = $param->{'guest-requested'};
> +	my $restart = 0;
>  
>  	# return if we do not have the config anymore
>  	return if !-f PVE::QemuConfig->config_file($vmid);
> @@ -790,10 +791,23 @@ __PACKAGE__->register_method({
>  		PVE::QemuServer::vm_stop_cleanup($storecfg, $vmid, $conf, 0, 0);
>  	    }
>  	    PVE::GuestHelpers::exec_hookscript($conf, $vmid, 'post-stop');
> +
> +	    $restart = eval { PVE::QemuServer::clear_reboot_request($vmid) };
> +	    if (my $err = $@) {
> +		warn $err;
> +	    }

IMO, `warn $@ if $@` is nicer here, we often use it and it's easier and less
to read if nothing else is done with the error message anyway..
But that just as nit.

>  	});
>  
>  	warn "Finished cleanup for $vmid\n";
>  
> +	if ($restart) {
> +	    warn "Restarting VM $vmid\n";
> +	    PVE::API2::Qemu->vm_start({
> +		vmid => $vmid,
> +		node => $nodename,
> +	    });
> +	}
> +
>  	return undef;
>      }});
>  
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 535297d..ba83b10 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -5341,6 +5341,14 @@ sub vm_start {
>  
>  	die "VM $vmid already running\n" if check_running($vmid, undef, $migratedfrom);
>  
> +	# clean up leftover reboot request files
> +	eval {
> +	    clear_reboot_request($vmid);

above single line eval vs split to multiplines in the same patch is a bit weird,
style should be at least consistent in a single patch. But just another nit,

Besides those two nits: Looks OK

> +	};
> +	if (my $err = $@) {
> +	    warn $err;
> +	}
> +
>  	if (!$statefile && scalar(keys %{$conf->{pending}})) {
>  	    vmconfig_apply_pending($vmid, $conf, $storecfg);
>  	    $conf = PVE::QemuConfig->load_config($vmid); # update/reload
>