[pve-devel] [PATCH qemu-server 4/5] resume suspended vm on start

[Thomas Lamprecht]

On 3/11/19 3:45 PM, Dominik Csapak wrote:
> if a vm has the 'suspend' lock, we resume with the saved state
> and remove the lock, the saved vmstate and the saved runningmachine
> after the vm started
> 
> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
> ---
>  PVE/QemuServer.pm | 26 +++++++++++++++++++++++++-
>  1 file changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 46620de..baebb62 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -4002,6 +4002,12 @@ sub config_to_command {
>      push @$cmd, '-global', join(',', @$globalFlags)
>  	if scalar(@$globalFlags);
>  
> +    if (my $vmstate = $conf->{vmstate}) {
> +	my $path = PVE::Storage::path($storecfg, $vmstate);

I do not like overwriting variables possible defined in outer scope.
Please use something more unique and telling here, even if all usage can
be seen in context (now).

> +	PVE::Storage::activate_volumes($storecfg, [$vmstate]);
> +	push @$cmd, '-loadstate', $path;
> +    }
> +
>      # add custom args
>      if ($conf->{args}) {
>  	my $aa = PVE::Tools::split_args($conf->{args});
> @@ -5148,7 +5154,8 @@ sub vm_start {
>  
>  	die "you can't start a vm if it's a template\n" if PVE::QemuConfig->is_template($conf);
>  
> -	PVE::QemuConfig->check_lock($conf) if !$skiplock;
> +	PVE::QemuConfig->check_lock($conf)
> +	    if !($skiplock || PVE::QemuConfig->has_lock($conf, 'suspend'));
>  
>  	die "VM $vmid already running\n" if check_running($vmid, undef, $migratedfrom);
>  
> @@ -5214,6 +5221,12 @@ sub vm_start {
>  
>  	PVE::GuestHelpers::exec_hookscript($conf, $vmid, 'pre-start', 1);
>  
> +	if (PVE::QemuConfig->has_lock($conf, 'suspend')) {
> +	    # if we want to resume a suspended vm,
> +	    # we have to use the saved machine type, otherwise it cannot work

maybe just:
# enforce saved machine type to ensure HW compatibility on resume


> +	    $forcemachine = $conf->{runningmachine};
> +	}
> +
>  	my ($cmd, $vollist, $spice_port) = config_to_command($storecfg, $vmid, $conf, $defaults, $forcemachine);
>  
>  	my $migrate_port = 0;
> @@ -5411,6 +5424,17 @@ sub vm_start {
>  		    property => "guest-stats-polling-interval",
>  		    value => 2) if (!defined($conf->{balloon}) || $conf->{balloon});
>  
> +	if (PVE::QemuConfig->has_lock($conf, 'suspend') &&

as you re-use this here 3 times why not just pull it out at the start of the locked context
in it's own $is_suspened (or the like) variable? then this if would also look a bit nicer ;)

> +	    (my $vmstate = $conf->{vmstate}))
> +	{
> +	    delete $conf->{lock};
> +	    delete $conf->{vmstate};
> +	    delete $conf->{runningmachine};

could do:

delete $conf->@{qw(lock vmstate runningmachine)};

but not sure if it's better besides that it's shorter ^^

> +	    PVE::Storage::deactivate_volumes($storecfg, [$vmstate]);
> +	    PVE::Storage::vdisk_free($storecfg, $vmstate);
> +	    PVE::QemuConfig->write_config($vmid, $conf);
> +	}
> +
>  	PVE::GuestHelpers::exec_hookscript($conf, $vmid, 'post-start');
>      });
>  }
>