From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <pve-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
	by lore.proxmox.com (Postfix) with ESMTPS id 84F7F1FF172
	for <inbox@lore.proxmox.com>; Tue,  1 Apr 2025 15:51:25 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
	by firstgate.proxmox.com (Proxmox) with ESMTP id 0A1F233D42;
	Tue,  1 Apr 2025 15:51:16 +0200 (CEST)
Date: Tue, 1 Apr 2025 15:50:41 +0200 (CEST)
From: =?UTF-8?Q?Fabian_Gr=C3=BCnbichler?= <f.gruenbichler@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Message-ID: <438825170.3976.1743515441173@webmail.proxmox.com>
In-Reply-To: <mailman.944.1741688961.293.pve-devel@lists.proxmox.com>
References: <20250311102905.2680524-1-alexandre.derumier@groupe-cyllene.com>
 <mailman.944.1741688961.293.pve-devel@lists.proxmox.com>
MIME-Version: 1.0
X-Priority: 3
Importance: Normal
X-Mailer: Open-Xchange Mailer v7.10.6-Rev75
X-Originating-Client: open-xchange-appsuite
X-SPAM-LEVEL: Spam detection results:  0
 AWL 0.046 Adjusted score from AWL reputation of From: address
 BAYES_00                 -1.9 Bayes spam probability is 0 to 1%
 DMARC_MISSING             0.1 Missing DMARC policy
 KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
 SPF_HELO_NONE           0.001 SPF: HELO does not publish an SPF Record
 SPF_PASS               -0.001 SPF: sender matches SPF record
Subject: Re: [pve-devel] [PATCH v4 pve-storage 3/5] storage: vdisk_free:
 remove external snapshots
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: pve-devel-bounces@lists.proxmox.com
Sender: "pve-devel" <pve-devel-bounces@lists.proxmox.com>


> Alexandre Derumier via pve-devel <pve-devel@lists.proxmox.com> hat am 11.03.2025 11:28 CET geschrieben:
> 
>  
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> Signed-off-by: Alexandre Derumier <alexandre.derumier@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@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


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel