[pve-devel] [PATCH v4 pve-storage 3/5] storage: vdisk_free: remove external snapshots

[Fabian Grünbichler]

> Alexandre Derumier via pve-devel <pve-devel at lists.proxmox.com> hat am 11.03.2025 11:28 CET geschrieben:
> 
>  
> _______________________________________________
> pve-devel mailing list
> pve-devel at lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> Signed-off-by: Alexandre Derumier <alexandre.derumier at groupe-cyllene.com>
> ---
>  src/PVE/Storage.pm                 | 18 +++++++++++++++++-
>  src/test/run_test_zfspoolplugin.pl | 18 ++++++++++++++++++
>  2 files changed, 35 insertions(+), 1 deletion(-)
> 
> diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm
> index 79e5c3a..4012905 100755
> --- a/src/PVE/Storage.pm
> +++ b/src/PVE/Storage.pm
> @@ -1052,7 +1052,23 @@ sub vdisk_free {
>  
>  	my (undef, undef, undef, undef, undef, $isBase, $format) =
>  	    $plugin->parse_volname($volname);
> -	$cleanup_worker = $plugin->free_image($storeid, $scfg, $volname, $isBase, $format);
> +
> +        $cleanup_worker = sub {
> +	    #remove external snapshots
> +	    activate_volumes($cfg, [ $volid ]);
> +	    my $snapshots = PVE::Storage::volume_snapshot_info($cfg, $volid);
> +	    for my $snapid (sort { $snapshots->{$b}->{order} <=> $snapshots->{$a}->{order} } keys %$snapshots) {
> +		my $snap = $snapshots->{$snapid};
> +		next if $snapid eq 'current';
> +		next if !$snap->{volid};
> +		next if !$snap->{ext};
> +		my ($snap_storeid, $snap_volname) = parse_volume_id($snap->{volid});
> +		my (undef, undef, undef, undef, undef, $snap_isBase, $snap_format) =
> +		    $plugin->parse_volname($volname);
> +		$plugin->free_image($snap_storeid, $scfg, $snap_volname, $snap_isBase, $snap_format);
> +	    }
> +	    $plugin->free_image($storeid, $scfg, $volname, $isBase, $format);

this is the wrong place to do this, you need to handle this in the cleanup worker returned by the plugin and still execute it here.. also you need to honor saferemove when cleaning up the snapshots

> +	};
>      });
>  
>      return if !$cleanup_worker;
> diff --git a/src/test/run_test_zfspoolplugin.pl b/src/test/run_test_zfspoolplugin.pl
> index 095ccb3..4ff9f22 100755
> --- a/src/test/run_test_zfspoolplugin.pl
> +++ b/src/test/run_test_zfspoolplugin.pl
> @@ -6,12 +6,30 @@ use strict;
>  use warnings;
>  
>  use Data::Dumper qw(Dumper);
> +use Test::MockModule;
> +
>  use PVE::Storage;
>  use PVE::Cluster;
>  use PVE::Tools qw(run_command);
> +use PVE::RPCEnvironment;
>  use Cwd;
>  $Data::Dumper::Sortkeys = 1;
>  
> +my $rpcenv_module;
> +$rpcenv_module = Test::MockModule->new('PVE::RPCEnvironment');
> +$rpcenv_module->mock(
> +    get_user => sub {
> +        return 'root at pam';
> +    },
> +    fork_worker => sub {
> +	my ($self, $dtype, $id, $user, $function, $background) = @_;
> +	$function->(123456);
> +	return '123456';
> +    }
> +);
> +
> +my $rpcenv = PVE::RPCEnvironment->init('pub');
> +

what? why? no explanation?

>  my $verbose = undef;
>  
>  my $storagename = "zfstank99";
> -- 
> 2.39.5