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

[Wolfgang Bumiller]

On Thu, Feb 04, 2016 at 04:05:11PM +0100, Fabian Grünbichler wrote:
> 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.
> 
> Add a call to snapshot_commit in its own eval block to the
> error handling code, because we want to cleanup and unlock
> the config file in case of freeze or volume_snapshot
> errors. This needs to be done before attempting to call
> snapshot_delete.
> 
> Move the cleanup snapshot_delete call into its own eval
> block, because we want to die with the original $err even
> if the snapshot_delete fails for whatever reason.
> ---
> Comment: this should ease recovery for those affected by the
> hanging lxc-freeze issue. Previously, if the issue occured
> one would have to manually unfreeze or force stop the container
> and remove the lock in the config file. The first subsequent
> snapshot (backup) would still fail on the first attempt because
> the config file would reference a snapshot that was not created
> on the storage level. Now, unfreezing the container and cleaning
> up the config file should happen automatically if an error
> occurs (for example, if a hanging lxc-freeze is killed by the
> administrator).

So basically without this the you end up in a frozen+locked state with a
garbage snapshot entry in the config. Now the only case left to deal
with is when something adds a lock between the commit() and delete()
calls (since no flock and no {lock} is held between them).
Maybe add a parameter to both these functions to change the behavior to
not remove the {lock} in commit() and remove instead of checking it in
delete().

>  src/PVE/LXC.pm | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
> index f761f33..c5c6607 100644
> --- a/src/PVE/LXC.pm
> +++ b/src/PVE/LXC.pm
> @@ -1827,8 +1827,8 @@ sub snapshot_create {
>      
>      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']);
>  	};
>  
> @@ -1847,7 +1847,10 @@ sub snapshot_create {
>      }
>      
>      if ($err) {
> -	snapshot_delete($vmid, $snapname, 1);
> +	eval { &$snapshot_commit($vmid, $snapname); };
> +	warn $@ if $@;
> +	eval { snapshot_delete($vmid, $snapname, 1); };
> +	warn $@ if $@;
>  	die "$err\n";
>      }
>  }
> -- 
> 2.1.4
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>