public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH qemu-server] fix #2788: do not resume vms after backup if they were paused before
@ 2021-01-20 10:07 Dominik Csapak
  2021-01-20 10:54 ` Fabian Ebner
  0 siblings, 1 reply; 3+ messages in thread
From: Dominik Csapak @ 2021-01-20 10:07 UTC (permalink / raw)
  To: pve-devel

by checking if the vm is paused at the beginning and skipping the resume
now we also skip the qga freeze/thaw (which cannot work if the vm is
paused)

moved the 'vm_is_paused' sub from the api to PVE/QemuServer.pm so it is
available everywhere we need it.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 PVE/API2/Qemu.pm         | 14 ++------------
 PVE/QemuServer.pm        | 10 ++++++++++
 PVE/VZDump/QemuServer.pm |  7 +++++--
 3 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index e2d2d67..623c998 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -2402,16 +2402,6 @@ __PACKAGE__->register_method({
 	return $rpcenv->fork_worker('qmreset', $vmid, $authuser, $realcmd);
     }});
 
-my sub vm_is_paused {
-    my ($vmid) = @_;
-    my $qmpstatus = eval {
-	PVE::QemuConfig::assert_config_exists_on_node($vmid);
-	mon_cmd($vmid, "query-status");
-    };
-    warn "$@\n" if $@;
-    return $qmpstatus && $qmpstatus->{status} eq "paused";
-}
-
 __PACKAGE__->register_method({
     name => 'vm_shutdown',
     path => '{vmid}/status/shutdown',
@@ -2480,7 +2470,7 @@ __PACKAGE__->register_method({
 	#
 	# checking the qmp status here to get feedback to the gui/cli/api
 	# and the status query should not take too long
-	if (vm_is_paused($vmid)) {
+	if (PVE::QemuServer::vm_is_paused($vmid)) {
 	    if ($param->{forceStop}) {
 		warn "VM is paused - stop instead of shutdown\n";
 		$shutdown = 0;
@@ -2556,7 +2546,7 @@ __PACKAGE__->register_method({
 	my $node = extract_param($param, 'node');
 	my $vmid = extract_param($param, 'vmid');
 
-	die "VM is paused - cannot shutdown\n" if vm_is_paused($vmid);
+	die "VM is paused - cannot shutdown\n" if PVE::QemuServer::vm_is_paused($vmid);
 
 	die "VM $vmid not running\n" if !PVE::QemuServer::check_running($vmid);
 
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 54278e5..d7ee05f 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -7382,4 +7382,14 @@ sub complete_migration_storage {
     return $res;
 }
 
+sub vm_is_paused {
+    my ($vmid) = @_;
+    my $qmpstatus = eval {
+	PVE::QemuConfig::assert_config_exists_on_node($vmid);
+	mon_cmd($vmid, "query-status");
+    };
+    warn "$@\n" if $@;
+    return $qmpstatus && $qmpstatus->{status} eq "paused";
+}
+
 1;
diff --git a/PVE/VZDump/QemuServer.pm b/PVE/VZDump/QemuServer.pm
index b322701..c62142d 100644
--- a/PVE/VZDump/QemuServer.pm
+++ b/PVE/VZDump/QemuServer.pm
@@ -62,8 +62,11 @@ sub prepare {
 	if defined($conf->{name});
 
     $self->{vm_was_running} = 1;
+    $self->{vm_was_paused} = 0;
     if (!PVE::QemuServer::check_running($vmid)) {
 	$self->{vm_was_running} = 0;
+    } elsif (PVE::QemuServer::vm_is_paused($vmid)) {
+	$self->{vm_was_paused} = 1;
     }
 
     $task->{hostname} = $conf->{name};
@@ -794,7 +797,7 @@ sub _get_task_devlist {
 
 sub qga_fs_freeze {
     my ($self, $task, $vmid) = @_;
-    return if !$self->{vmlist}->{$vmid}->{agent} || $task->{mode} eq 'stop' || !$self->{vm_was_running};
+    return if !$self->{vmlist}->{$vmid}->{agent} || $task->{mode} eq 'stop' || !$self->{vm_was_running} || $self->{vm_was_paused};
 
     if (!PVE::QemuServer::qga_check_running($vmid, 1)) {
 	$self->loginfo("skipping guest-agent 'fs-freeze', agent configured but not running?");
@@ -872,7 +875,7 @@ sub register_qmeventd_handle {
 sub resume_vm_after_job_start {
     my ($self, $task, $vmid) = @_;
 
-    return if !$self->{vm_was_running};
+    return if !$self->{vm_was_running} || $self->{vm_was_paused};
 
     if (my $stoptime = $task->{vmstoptime}) {
 	my $delay = time() - $task->{vmstoptime};
-- 
2.20.1





^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [pve-devel] [PATCH qemu-server] fix #2788: do not resume vms after backup if they were paused before
  2021-01-20 10:07 [pve-devel] [PATCH qemu-server] fix #2788: do not resume vms after backup if they were paused before Dominik Csapak
@ 2021-01-20 10:54 ` Fabian Ebner
  2021-01-20 11:38   ` Dominik Csapak
  0 siblings, 1 reply; 3+ messages in thread
From: Fabian Ebner @ 2021-01-20 10:54 UTC (permalink / raw)
  To: pve-devel, d.csapak

Works for snapshot mode, but with suspend mode it still resumes the VM 
afterwards. For stop mode, we error out before stopping the VM. Should 
we error out before suspending too or can we somehow go back from 
suspended to paused?

On 20.01.21 11:07, Dominik Csapak wrote:
> by checking if the vm is paused at the beginning and skipping the resume
> now we also skip the qga freeze/thaw (which cannot work if the vm is
> paused)
> 
> moved the 'vm_is_paused' sub from the api to PVE/QemuServer.pm so it is
> available everywhere we need it.
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>   PVE/API2/Qemu.pm         | 14 ++------------
>   PVE/QemuServer.pm        | 10 ++++++++++
>   PVE/VZDump/QemuServer.pm |  7 +++++--
>   3 files changed, 17 insertions(+), 14 deletions(-)
> 
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index e2d2d67..623c998 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -2402,16 +2402,6 @@ __PACKAGE__->register_method({
>   	return $rpcenv->fork_worker('qmreset', $vmid, $authuser, $realcmd);
>       }});
>   
> -my sub vm_is_paused {
> -    my ($vmid) = @_;
> -    my $qmpstatus = eval {
> -	PVE::QemuConfig::assert_config_exists_on_node($vmid);
> -	mon_cmd($vmid, "query-status");
> -    };
> -    warn "$@\n" if $@;
> -    return $qmpstatus && $qmpstatus->{status} eq "paused";
> -}
> -
>   __PACKAGE__->register_method({
>       name => 'vm_shutdown',
>       path => '{vmid}/status/shutdown',
> @@ -2480,7 +2470,7 @@ __PACKAGE__->register_method({
>   	#
>   	# checking the qmp status here to get feedback to the gui/cli/api
>   	# and the status query should not take too long
> -	if (vm_is_paused($vmid)) {
> +	if (PVE::QemuServer::vm_is_paused($vmid)) {
>   	    if ($param->{forceStop}) {
>   		warn "VM is paused - stop instead of shutdown\n";
>   		$shutdown = 0;
> @@ -2556,7 +2546,7 @@ __PACKAGE__->register_method({
>   	my $node = extract_param($param, 'node');
>   	my $vmid = extract_param($param, 'vmid');
>   
> -	die "VM is paused - cannot shutdown\n" if vm_is_paused($vmid);
> +	die "VM is paused - cannot shutdown\n" if PVE::QemuServer::vm_is_paused($vmid);
>   
>   	die "VM $vmid not running\n" if !PVE::QemuServer::check_running($vmid);
>   
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 54278e5..d7ee05f 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -7382,4 +7382,14 @@ sub complete_migration_storage {
>       return $res;
>   }
>   
> +sub vm_is_paused {
> +    my ($vmid) = @_;
> +    my $qmpstatus = eval {
> +	PVE::QemuConfig::assert_config_exists_on_node($vmid);
> +	mon_cmd($vmid, "query-status");
> +    };
> +    warn "$@\n" if $@;
> +    return $qmpstatus && $qmpstatus->{status} eq "paused";
> +}
> +
>   1;
> diff --git a/PVE/VZDump/QemuServer.pm b/PVE/VZDump/QemuServer.pm
> index b322701..c62142d 100644
> --- a/PVE/VZDump/QemuServer.pm
> +++ b/PVE/VZDump/QemuServer.pm
> @@ -62,8 +62,11 @@ sub prepare {
>   	if defined($conf->{name});
>   
>       $self->{vm_was_running} = 1;
> +    $self->{vm_was_paused} = 0;
>       if (!PVE::QemuServer::check_running($vmid)) {
>   	$self->{vm_was_running} = 0;
> +    } elsif (PVE::QemuServer::vm_is_paused($vmid)) {
> +	$self->{vm_was_paused} = 1;
>       }
>   
>       $task->{hostname} = $conf->{name};
> @@ -794,7 +797,7 @@ sub _get_task_devlist {
>   
>   sub qga_fs_freeze {
>       my ($self, $task, $vmid) = @_;
> -    return if !$self->{vmlist}->{$vmid}->{agent} || $task->{mode} eq 'stop' || !$self->{vm_was_running};
> +    return if !$self->{vmlist}->{$vmid}->{agent} || $task->{mode} eq 'stop' || !$self->{vm_was_running} || $self->{vm_was_paused};
>   
>       if (!PVE::QemuServer::qga_check_running($vmid, 1)) {
>   	$self->loginfo("skipping guest-agent 'fs-freeze', agent configured but not running?");
> @@ -872,7 +875,7 @@ sub register_qmeventd_handle {
>   sub resume_vm_after_job_start {
>       my ($self, $task, $vmid) = @_;
>   
> -    return if !$self->{vm_was_running};
> +    return if !$self->{vm_was_running} || $self->{vm_was_paused};
>   
>       if (my $stoptime = $task->{vmstoptime}) {
>   	my $delay = time() - $task->{vmstoptime};
> 




^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [pve-devel] [PATCH qemu-server] fix #2788: do not resume vms after backup if they were paused before
  2021-01-20 10:54 ` Fabian Ebner
@ 2021-01-20 11:38   ` Dominik Csapak
  0 siblings, 0 replies; 3+ messages in thread
From: Dominik Csapak @ 2021-01-20 11:38 UTC (permalink / raw)
  To: Fabian Ebner, pve-devel

On 1/20/21 11:54 AM, Fabian Ebner wrote:
> Works for snapshot mode, but with suspend mode it still resumes the VM 
> afterwards. For stop mode, we error out before stopping the VM. Should 
> we error out before suspending too or can we somehow go back from 
> suspended to paused?
> 

thanks, i missed that...

for vms, paused == suspended (not to disk), so that should be no 
problem, i'll send a v2 later today

> On 20.01.21 11:07, Dominik Csapak wrote:
>> by checking if the vm is paused at the beginning and skipping the resume
>> now we also skip the qga freeze/thaw (which cannot work if the vm is
>> paused)
>>
>> moved the 'vm_is_paused' sub from the api to PVE/QemuServer.pm so it is
>> available everywhere we need it.
>>
>> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
>> ---
>>   PVE/API2/Qemu.pm         | 14 ++------------
>>   PVE/QemuServer.pm        | 10 ++++++++++
>>   PVE/VZDump/QemuServer.pm |  7 +++++--
>>   3 files changed, 17 insertions(+), 14 deletions(-)
>>
>> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
>> index e2d2d67..623c998 100644
>> --- a/PVE/API2/Qemu.pm
>> +++ b/PVE/API2/Qemu.pm
>> @@ -2402,16 +2402,6 @@ __PACKAGE__->register_method({
>>       return $rpcenv->fork_worker('qmreset', $vmid, $authuser, $realcmd);
>>       }});
>> -my sub vm_is_paused {
>> -    my ($vmid) = @_;
>> -    my $qmpstatus = eval {
>> -    PVE::QemuConfig::assert_config_exists_on_node($vmid);
>> -    mon_cmd($vmid, "query-status");
>> -    };
>> -    warn "$@\n" if $@;
>> -    return $qmpstatus && $qmpstatus->{status} eq "paused";
>> -}
>> -
>>   __PACKAGE__->register_method({
>>       name => 'vm_shutdown',
>>       path => '{vmid}/status/shutdown',
>> @@ -2480,7 +2470,7 @@ __PACKAGE__->register_method({
>>       #
>>       # checking the qmp status here to get feedback to the gui/cli/api
>>       # and the status query should not take too long
>> -    if (vm_is_paused($vmid)) {
>> +    if (PVE::QemuServer::vm_is_paused($vmid)) {
>>           if ($param->{forceStop}) {
>>           warn "VM is paused - stop instead of shutdown\n";
>>           $shutdown = 0;
>> @@ -2556,7 +2546,7 @@ __PACKAGE__->register_method({
>>       my $node = extract_param($param, 'node');
>>       my $vmid = extract_param($param, 'vmid');
>> -    die "VM is paused - cannot shutdown\n" if vm_is_paused($vmid);
>> +    die "VM is paused - cannot shutdown\n" if 
>> PVE::QemuServer::vm_is_paused($vmid);
>>       die "VM $vmid not running\n" if 
>> !PVE::QemuServer::check_running($vmid);
>> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
>> index 54278e5..d7ee05f 100644
>> --- a/PVE/QemuServer.pm
>> +++ b/PVE/QemuServer.pm
>> @@ -7382,4 +7382,14 @@ sub complete_migration_storage {
>>       return $res;
>>   }
>> +sub vm_is_paused {
>> +    my ($vmid) = @_;
>> +    my $qmpstatus = eval {
>> +    PVE::QemuConfig::assert_config_exists_on_node($vmid);
>> +    mon_cmd($vmid, "query-status");
>> +    };
>> +    warn "$@\n" if $@;
>> +    return $qmpstatus && $qmpstatus->{status} eq "paused";
>> +}
>> +
>>   1;
>> diff --git a/PVE/VZDump/QemuServer.pm b/PVE/VZDump/QemuServer.pm
>> index b322701..c62142d 100644
>> --- a/PVE/VZDump/QemuServer.pm
>> +++ b/PVE/VZDump/QemuServer.pm
>> @@ -62,8 +62,11 @@ sub prepare {
>>       if defined($conf->{name});
>>       $self->{vm_was_running} = 1;
>> +    $self->{vm_was_paused} = 0;
>>       if (!PVE::QemuServer::check_running($vmid)) {
>>       $self->{vm_was_running} = 0;
>> +    } elsif (PVE::QemuServer::vm_is_paused($vmid)) {
>> +    $self->{vm_was_paused} = 1;
>>       }
>>       $task->{hostname} = $conf->{name};
>> @@ -794,7 +797,7 @@ sub _get_task_devlist {
>>   sub qga_fs_freeze {
>>       my ($self, $task, $vmid) = @_;
>> -    return if !$self->{vmlist}->{$vmid}->{agent} || $task->{mode} eq 
>> 'stop' || !$self->{vm_was_running};
>> +    return if !$self->{vmlist}->{$vmid}->{agent} || $task->{mode} eq 
>> 'stop' || !$self->{vm_was_running} || $self->{vm_was_paused};
>>       if (!PVE::QemuServer::qga_check_running($vmid, 1)) {
>>       $self->loginfo("skipping guest-agent 'fs-freeze', agent 
>> configured but not running?");
>> @@ -872,7 +875,7 @@ sub register_qmeventd_handle {
>>   sub resume_vm_after_job_start {
>>       my ($self, $task, $vmid) = @_;
>> -    return if !$self->{vm_was_running};
>> +    return if !$self->{vm_was_running} || $self->{vm_was_paused};
>>       if (my $stoptime = $task->{vmstoptime}) {
>>       my $delay = time() - $task->{vmstoptime};
>>





^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-01-20 11:38 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-20 10:07 [pve-devel] [PATCH qemu-server] fix #2788: do not resume vms after backup if they were paused before Dominik Csapak
2021-01-20 10:54 ` Fabian Ebner
2021-01-20 11:38   ` Dominik Csapak

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal