[pve-devel] applied: [RFC PATCH qemu-server] use 'system_wakeup' to resume suspended vms

[Thomas Lamprecht]

On 6/13/18 1:46 PM, Thomas Lamprecht wrote:
> On 6/13/18 11:17 AM, Dominik Csapak wrote:
>> when a vm is suspended (e.g. autosuspend on windows)
>> we detect that it is not running, display the resume button,
>> but 'cont' does not wakeup the system from suspend
>>
>> with this we can wake up suspended vms
>>
>> [snip]
> 
> applied, but I wonder if we could remove the $nocheck from this
> method...
> 
> nocheck was introduced in 289e0b8564dce494481cd3a5d534b801835f85b6 as
> a band-aid, but it only accomplishes skipping the check_running call
> in the vm_qmp_command (which vm_mon_cmd uses) and avoiding the
> load_conf for some race cases with migration where the config wasn't
> moved yet at this point.
> 
> But as neither cont nor system_wakup will work on an non-running VM
> we could use only vm_mon_cmd once at the end of the locked scope?
> 
> Further nocheck may be removed, the load_conf could be wrapped in
> skiplock, which the migration case uses anyway.
> The qm skiplock param would need to be kept as dummy param for now,
> to keep incoming migration backward compatibility... 
> 
> What do you think? I.e., something like this followup:
> 

Any comments?

> ---8<---
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index c15c71f..ab1f73d 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -2349,7 +2349,7 @@ __PACKAGE__->register_method({
>             vmid => get_standard_option('pve-vmid',
>                                         { completion => \&PVE::QemuServer::complete_vmid_running }),
>             skiplock => get_standard_option('skiplock'),
> -           nocheck => { type => 'boolean', optional => 1 },
> +           nocheck => { type => 'boolean', optional => 1, description => 'deprecated' },
> 
>         },
>      },
> @@ -2371,16 +2371,14 @@ __PACKAGE__->register_method({
>         raise_param_exc({ skiplock => "Only root may use this option." })
>             if $skiplock && $authuser ne 'root at pam';
> 
> -       my $nocheck = extract_param($param, 'nocheck');
> -
> -       die "VM $vmid not running\n" if !PVE::QemuServer::check_running($vmid, $nocheck);
> +       die "VM $vmid not running\n";
> 
>         my $realcmd = sub {
>             my $upid = shift;
> 
>             syslog('info', "resume VM $vmid: $upid\n");
> 
> -           PVE::QemuServer::vm_resume($vmid, $skiplock, $nocheck);
> +           PVE::QemuServer::vm_resume($vmid, $skiplock);
> 
>             return;
>         };
> diff --git a/PVE/CLI/qm.pm b/PVE/CLI/qm.pm
> index c017a59..83a50a2 100755
> --- a/PVE/CLI/qm.pm
> +++ b/PVE/CLI/qm.pm
> @@ -297,7 +297,7 @@ __PACKAGE__->register_method ({
>             } elsif ($line =~ /^resume (\d+)$/) {
>                 my $vmid = $1;
>                 if (PVE::QemuServer::check_running($vmid, 1)) {
> -                   eval { PVE::QemuServer::vm_resume($vmid, 1, 1); };
> +                   eval { PVE::QemuServer::vm_resume($vmid, 1) };
>                     if ($@) {
>                         $tunnel_write->("ERR: resume failed - $@");
>                     } else {
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 277bc58..a93625a 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -5166,29 +5166,22 @@ sub vm_suspend {
>  }
> 
>  sub vm_resume {
> -    my ($vmid, $skiplock, $nocheck) = @_;
> +    my ($vmid, $skiplock) = @_;
> 
>      PVE::QemuConfig->lock_config($vmid, sub {
> 
>         my $res = vm_mon_cmd($vmid, 'query-status');
>         my $resume_cmd = 'cont';
> -
>         if ($res->{status} && $res->{status} eq 'suspended') {
>             $resume_cmd = 'system_wakeup';
>         }
> 
> -       if (!$nocheck) {
> -
> +       if (!$skiplock) {
>             my $conf = PVE::QemuConfig->load_config($vmid);
> -
> -           PVE::QemuConfig->check_lock($conf)
> -               if !($skiplock || PVE::QemuConfig->has_lock($conf, 'backup'));
> -
> -           vm_mon_cmd($vmid, $resume_cmd);
> -
> -       } else {
> -           vm_mon_cmd_nocheck($vmid, $resume_cmd);
> +           PVE::QemuConfig->check_lock($conf) if !PVE::QemuConfig->has_lock($conf, 'backup');
>         }
> +
> +       vm_mon_cmd($vmid, $resume_cmd);
>      });
>  }
> 
> @@ -6299,7 +6292,7 @@ sub qemu_drive_mirror_monitor {
>                         eval { PVE::QemuServer::vm_mon_cmd($vmid, "guest-fsfreeze-thaw"); };
>                     } else {
>                         print "resume vm\n";
> -                       eval {  PVE::QemuServer::vm_resume($vmid, 1, 1); };
> +                       eval { vm_resume($vmid, 1) };
>                     }
> 
>                     last;
>