[pve-devel] [PATCH qemu-server v3] Fix #2171: VM statefile was not activated

[Alwin Antreich]

On rollback of a snapshot with state, the storage wasn't activated and
KVM failed to load the state on a storage like RBD. The $statefile
contained a path to the state file/device on rollback of a snapshot.

This patch assigns the statefile of the snapshot with a storage based
URI (<storage>:<volid>), to $conf->{vmstate} so config_to_command can
activate, generate and assign the path to '-loadstate'. Any file/device
based statefile will be added directly to -loadstate.

Signed-off-by: Alwin Antreich <a.antreich at proxmox.com>
---
Note:	V1 -> V2: re-use resume code for rollback, incorporate
		  suggestions of Fabian
	  https://pve.proxmox.com/pipermail/pve-devel/2019-October/039459.html

	V2 -> V3: move config_to_command below command additions,
		  incorporate suggestions of Thomas
	  https://pve.proxmox.com/pipermail/pve-devel/2019-October/039564.html

 PVE/QemuConfig.pm |  3 +--
 PVE/QemuServer.pm | 36 +++++++++++++++++++++++-------------
 2 files changed, 24 insertions(+), 15 deletions(-)

diff --git a/PVE/QemuConfig.pm b/PVE/QemuConfig.pm
index edbf1a7..e9796a3 100644
--- a/PVE/QemuConfig.pm
+++ b/PVE/QemuConfig.pm
@@ -359,8 +359,7 @@ sub __snapshot_rollback_vm_start {
     my ($class, $vmid, $vmstate, $data) = @_;
 
     my $storecfg = PVE::Storage::config();
-    my $statefile = PVE::Storage::path($storecfg, $vmstate);
-    PVE::QemuServer::vm_start($storecfg, $vmid, $statefile, undef, undef, undef, $data->{forcemachine});
+    PVE::QemuServer::vm_start($storecfg, $vmid, $vmstate, undef, undef, undef, $data->{forcemachine});
 }
 
 sub __snapshot_rollback_get_unused {
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 8dda594..b4e1ec8 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -5416,9 +5416,8 @@ sub vm_start {
 	    print "Resuming suspended VM\n";
 	}
 
-	my ($cmd, $vollist, $spice_port) = config_to_command($storecfg, $vmid, $conf, $defaults, $forcemachine);
-
 	my $migrate_uri;
+	my $cmd_ext;
 	if ($statefile) {
 	    if ($statefile eq 'tcp') {
 		my $localip = "localhost";
@@ -5447,8 +5446,8 @@ sub vm_start {
 		my $pfamily = PVE::Tools::get_host_address_family($nodename);
 		my $migrate_port = PVE::Tools::next_migrate_port($pfamily);
 		$migrate_uri = "tcp:${localip}:${migrate_port}";
-		push @$cmd, '-incoming', $migrate_uri;
-		push @$cmd, '-S';
+		push @$cmd_ext, '-incoming', $migrate_uri;
+		push @$cmd_ext, '-S';
 
 	    } elsif ($statefile eq 'unix') {
 		# should be default for secure migrations as a ssh TCP forward
@@ -5459,16 +5458,24 @@ sub vm_start {
 
 		$migrate_uri = "unix:$socket_addr";
 
-		push @$cmd, '-incoming', $migrate_uri;
-		push @$cmd, '-S';
+		push @$cmd_ext, '-incoming', $migrate_uri;
+		push @$cmd_ext, '-S';
 
+	    } elsif (-e $statefile) {
+		push @$cmd_ext, '-loadstate', $statefile;
 	    } else {
-		push @$cmd, '-loadstate', $statefile;
+		# config_to_command takes care of activation and path
+		# generation of storage URIs (storage:vmid/vm-image) and adds
+		# the statefile to -loadstate
+		$conf->{vmstate} = $statefile;
 	    }
 	} elsif ($paused) {
-	    push @$cmd, '-S';
+	    push @$cmd_ext, '-S';
 	}
 
+	my ($cmd, $vollist, $spice_port) = config_to_command($storecfg, $vmid, $conf, $defaults, $forcemachine);
+	push @$cmd, $cmd_ext if $cmd_ext;
+
 	# host pci devices
         for (my $i = 0; $i < $MAX_HOSTPCI_DEVICES; $i++)  {
           my $d = parse_hostpci($conf->{"hostpci$i"});
@@ -5613,12 +5620,15 @@ sub vm_start {
 		    property => "guest-stats-polling-interval",
 		    value => 2) if (!defined($conf->{balloon}) || $conf->{balloon});
 
-	if ($is_suspended && (my $vmstate = $conf->{vmstate})) {
-	    print "Resumed VM, removing state\n";
-	    delete $conf->@{qw(lock vmstate runningmachine)};
+	if (my $vmstate = $conf->{vmstate}) {
+	    # always deactive vmstate volume again
 	    PVE::Storage::deactivate_volumes($storecfg, [$vmstate]);
-	    PVE::Storage::vdisk_free($storecfg, $vmstate);
-	    PVE::QemuConfig->write_config($vmid, $conf);
+	    if ($is_suspended) {
+		print "Resumed VM, removing state\n";
+		delete $conf->@{qw(lock vmstate runningmachine)};
+		PVE::Storage::vdisk_free($storecfg, $vmstate);
+		PVE::QemuConfig->write_config($vmid, $conf);
+	    }
 	}
 
 	PVE::GuestHelpers::exec_hookscript($conf, $vmid, 'post-start');
-- 
2.20.1