[pve-devel] [PATCH qemu-server 2/4] snapshot: use explicitly configured vmstate storage

[Wolfgang Bumiller]

On Wed, May 10, 2017 at 12:00:19PM +0200, Fabian Grünbichler wrote:
> Signed-off-by: Fabian Grünbichler <f.gruenbichler at proxmox.com>
> ---
>  PVE/QemuConfig.pm | 30 +++++++++++++++++-------------
>  1 file changed, 17 insertions(+), 13 deletions(-)
> 
> diff --git a/PVE/QemuConfig.pm b/PVE/QemuConfig.pm
> index a2bc4fc..1492480 100644
> --- a/PVE/QemuConfig.pm
> +++ b/PVE/QemuConfig.pm
> @@ -107,28 +107,32 @@ sub __snapshot_save_vmstate {
>  
>      my $snap = $conf->{snapshots}->{$snapname};
>  

Since we iterate through storages twice currently with two functions
which differ only in a single '!' if I see this right, how about
iterating once and storing 2 possible results, something like:

    # first, use explicitly configured storage
    my $target = $conf->{vmstatestorage};
    if (!$target) {
	my ($shared, $local);
	PVE::QemuServer::foreach_storage_used_by_vm($conf, sub {
	    my ($sid) = @_;
	    my $scfg = PVE::Storage::storage_config($storecfg, $sid);
	    my $dst = $scfg->{shared} ? \$shared : \$local;
	    $$dst = $sid if !$$dst || $scfg->{path}; # prefer file based storage
	});
	# 2nd: use shared storage where VM has at least 1 disk
	# 3rd: use local storage where VM has at least 1 disk
	# fall back to local storage
	$target = $shared // $local // 'local';
    }

> -    my $target;
> -
> -    # search shared storage first
> -    PVE::QemuServer::foreach_writable_storage($conf, sub {
> -	my ($sid) = @_;
> -	my $scfg = PVE::Storage::storage_config($storecfg, $sid);
> -	return if !$scfg->{shared};
> -
> -	$target = $sid if !$target || $scfg->{path}; # prefer file based storage
> -    });
> +    # first, use explicitly configured storage
> +    my $target = $conf->{vmstatestorage};
>  
>      if (!$target) {
> -	# now search local storage
> +	# second, use shared storage where VM has at least one disk
>  	PVE::QemuServer::foreach_writable_storage($conf, sub {
>  	    my ($sid) = @_;
>  	    my $scfg = PVE::Storage::storage_config($storecfg, $sid);
> -	    return if $scfg->{shared};
> +	    return if !$scfg->{shared};
>  
> -	    $target = $sid if !$target || $scfg->{path}; # prefer file based storage;
> +	    $target = $sid if !$target || $scfg->{path}; # prefer file based storage
>  	});
> +
> +	if (!$target) {
> +	    # third, use local storage where VM has at least one disk
> +	    PVE::QemuServer::foreach_writable_storage($conf, sub {
> +		my ($sid) = @_;
> +		my $scfg = PVE::Storage::storage_config($storecfg, $sid);
> +		return if $scfg->{shared};
> +
> +		$target = $sid if !$target || $scfg->{path}; # prefer file based storage;
> +	});
> +	}
>      }
>  
> +    # fall back to local storage
>      $target = 'local' if !$target;
>  
>      my $driver_state_size = 500; # assume 32MB is enough to safe all driver state;
> -- 
> 2.1.4