[pve-devel] [PATCH v3 container] Improve error handling in snapshot_create

[Fabian Grünbichler]

Set unfreeze before trying to freeze, otherwise an aborted
or failed lxc-freeze will not be reversed by our error
handling, leaving the container in a (partially) frozen
state.

Make snapshot_create failure handling more resembling
to the QemuServer codebase and prepare for future code
convergence:
* use $drivehash parameter in snapshot_delete to bypass
check_lock() and delete config lock
* call $snapshot_commit last, it's only needed now if
there were no errors
---
Changes to v2: drop snapshot_abort in favor of code more
similar to QemuServer's

 src/PVE/LXC.pm | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
index 6a3489a..897a283 100644
--- a/src/PVE/LXC.pm
+++ b/src/PVE/LXC.pm
@@ -1833,11 +1833,13 @@ sub snapshot_create {
     my $running = check_running($vmid);
     
     my $unfreeze = 0;
-    
+
+    my $drivehash = {};
+
     eval {
 	if ($running) {
-	    PVE::Tools::run_command(['/usr/bin/lxc-freeze', '-n', $vmid]);
 	    $unfreeze = 1;
+	    PVE::Tools::run_command(['/usr/bin/lxc-freeze', '-n', $vmid]);
 	    PVE::Tools::run_command(['/bin/sync']);
 	};
 
@@ -1846,7 +1848,7 @@ sub snapshot_create {
 	my $volid = $rootinfo->{volume};
 
 	PVE::Storage::volume_snapshot($storecfg, $volid, $snapname);
-	&$snapshot_commit($vmid, $snapname);
+	$drivehash->{rootfs} = 1;
     };
     my $err = $@;
     
@@ -1856,13 +1858,17 @@ sub snapshot_create {
     }
     
     if ($err) {
-	snapshot_delete($vmid, $snapname, 1);
+	eval { snapshot_delete($vmid, $snapname, 1, $drivehash); };
+	warn "$@\n" if $@;
 	die "$err\n";
     }
+
+    &$snapshot_commit($vmid, $snapname);
 }
 
+# Note: $drivehash is only set when called from snapshot_create.
 sub snapshot_delete {
-    my ($vmid, $snapname, $force) = @_;
+    my ($vmid, $snapname, $force, $drivehash) = @_;
 
     my $snap;
 
@@ -1877,7 +1883,9 @@ sub snapshot_delete {
 
 	$snap = $conf->{snapshots}->{$snapname};
 
-	check_lock($conf);
+	if (!$drivehash) {
+	    check_lock($conf);
+	}
 
 	die "snapshot '$snapname' does not exist\n" if !defined($snap);
 
@@ -1905,7 +1913,13 @@ sub snapshot_delete {
 
     my $del_snap =  sub {
 
-	check_lock($conf);
+	$conf = load_config($vmid);
+
+	if ($drivehash) {
+	    delete $conf->{lock};
+	} else {
+	    check_lock($conf);
+	}
 
 	my $parent = $conf->{snapshots}->{$snapname}->{parent};
 	foreach my $snapkey (keys %{$conf->{snapshots}}) {
-- 
2.1.4