[PATCH qemu-server 1/1] fix #8030: snapshot: avoid orphaned vmstate volume on failure

[PATCH qemu-server 1/1] fix #8030: snapshot: avoid orphaned vmstate volume on failure

[Michael Köppl]

The state volume was allocated before the running instance was queried
for its machine type, CPU argument and network MTUs. Those queries can
fail (e.g. the 'query-machines' QMP command, which can run into a
timeout while QEMU's is busy [0]).

The helper is called from __snapshot_prepare() before write_config(), so
the config never records the snapshot, snapshot_create() dies before it
gets to its cleanup path. Such a failure leaves behind the just
allocated volume.

Query the running instance first, which makes the allocation the last
step of the function that can fail.

[0] https://bugzilla.proxmox.com/show_bug.cgi?id=8030

Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
---
Tested this pretty much as described in the Bugzilla entry, but using a
Perl script to connect to the QMP socket instead of socat.

 src/PVE/QemuConfig.pm | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/PVE/QemuConfig.pm b/src/PVE/QemuConfig.pm
index 26f0fda2..2ef6c075 100644
--- a/src/PVE/QemuConfig.pm
+++ b/src/PVE/QemuConfig.pm
@@ -238,8 +238,6 @@ sub __snapshot_save_vmstate {
     my $name = "vm-$vmid-state-$snapname";
     $name .= ".raw" if $scfg->{path}; # add filename extension for file base storage
 
-    my $statefile =
-        PVE::Storage::vdisk_alloc($storecfg, $target, $vmid, 'raw', $name, $size * 1024);
     my $runningmachine = PVE::QemuServer::Machine::get_current_qemu_machine($vmid);
 
     # get current QEMU -cpu argument to ensure consistency of custom CPU models
@@ -249,6 +247,11 @@ sub __snapshot_save_vmstate {
 
     my $nets_host_mtu = PVE::QemuServer::Network::get_nets_host_mtu($vmid, $conf);
 
+    # allocate only after querying the running instance, nothing below can fail, so a failed
+    # query cannot leave an orphaned state volume behind
+    my $statefile =
+        PVE::Storage::vdisk_alloc($storecfg, $target, $vmid, 'raw', $name, $size * 1024);
+
     if (!$suspend) {
         $conf = $conf->{snapshots}->{$snapname};
     }
-- 
2.47.3