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

[Alexandre Derumier via pve-devel <pve-devel@lists.proxmox.com>]

[-- Attachment #1: Type: message/rfc822, Size: 9252 bytes --]

From: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
To: pve-devel@lists.proxmox.com
Subject: [PATCH pve-storage 4/5] storage: vdisk_free: remove external snapshots
Date: Tue, 22 Apr 2025 13:51:30 +0200
Message-ID: <20250422115141.808427-10-alexandre.derumier@groupe-cyllene.com>

add a $include_snapshots param to free_image to
remove the whole chain of snapshots when deleting the main image.

Signed-off-by: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
---
 src/PVE/Storage.pm           |  2 +-
 src/PVE/Storage/LVMPlugin.pm | 72 ++++++++++++++++++++++++------------
 src/PVE/Storage/Plugin.pm    | 27 +++++++++++---
 3 files changed, 71 insertions(+), 30 deletions(-)

diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm
index db9d190..55a9a43 100755
--- a/src/PVE/Storage.pm
+++ b/src/PVE/Storage.pm
@@ -1059,7 +1059,7 @@ 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 = $plugin->free_image($storeid, $scfg, $volname, $isBase, $format, 1);
     });
 
     return if !$cleanup_worker;
diff --git a/src/PVE/Storage/LVMPlugin.pm b/src/PVE/Storage/LVMPlugin.pm
index 8ee337a..b20fe98 100644
--- a/src/PVE/Storage/LVMPlugin.pm
+++ b/src/PVE/Storage/LVMPlugin.pm
@@ -463,10 +463,34 @@ sub alloc_snap_image {
 }
 
 sub free_image {
-    my ($class, $storeid, $scfg, $volname, $isBase) = @_;
+    my ($class, $storeid, $scfg, $volname, $isBase, $format, $include_snapshots) = @_;
 
     my $vg = $scfg->{vgname};
 
+    my $name = ($class->parse_volname($volname))[1];
+
+    #activate volumes && snapshot volumes
+    my $path = $class->path($scfg, $volname, $storeid);
+    $path = "\@pve-$name" if $format && $format eq 'qcow2';
+    my $cmd = ['/sbin/lvchange', '-aly', $path];
+    run_command($cmd, errmsg => "can't activate LV '$path' to zero-out its data");
+    $cmd = ['/sbin/lvchange', '--refresh', $path];
+    run_command($cmd, errmsg => "can't refresh LV '$path' to zero-out its data");
+
+    my $volnames = [];
+    if ($include_snapshots) {
+	my $snapshots =  $class->volume_snapshot_info($scfg, $storeid, $volname);
+	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) = PVE::Storage::parse_volume_id($snap->{volid});
+	    push @$volnames, $snap_volname;
+	}
+    }
+    push @$volnames, $volname;
+
     # we need to zero out LVM data for security reasons
     # and to allow thin provisioning
 
@@ -478,40 +502,40 @@ sub free_image {
 	if ($scfg->{saferemove_throughput}) {
 		$throughput = $scfg->{saferemove_throughput};
 	}
-
-	my $cmd = [
+	for my $name (@$volnames) {
+	    my $cmd = [
 		'/usr/bin/cstream',
 		'-i', '/dev/zero',
-		'-o', "/dev/$vg/del-$volname",
+		'-o', "/dev/$vg/del-$name",
 		'-T', '10',
 		'-v', '1',
 		'-b', '1048576',
 		'-t', "$throughput"
-	];
-	eval { run_command($cmd, errmsg => "zero out finished (note: 'No space left on device' is ok here)"); };
-	warn $@ if $@;
-
-	$class->cluster_lock_storage($storeid, $scfg->{shared}, undef, sub {
-	    my $cmd = ['/sbin/lvremove', '-f', "$vg/del-$volname"];
-	    run_command($cmd, errmsg => "lvremove '$vg/del-$volname' error");
-	});
-	print "successfully removed volume $volname ($vg/del-$volname)\n";
+	    ];
+	    eval { run_command($cmd, errmsg => "zero out finished (note: 'No space left on device' is ok here)"); };
+	    warn $@ if $@;
+
+	    $class->cluster_lock_storage($storeid, $scfg->{shared}, undef, sub {
+		my $cmd = ['/sbin/lvremove', '-f', "$vg/del-$name"];
+		run_command($cmd, errmsg => "lvremove '$vg/del-$name' error");
+	    });
+	    print "successfully removed volume $name ($vg/del-$name)\n";
+	}
     };
 
-    my $cmd = ['/sbin/lvchange', '-aly', "$vg/$volname"];
-    run_command($cmd, errmsg => "can't activate LV '$vg/$volname' to zero-out its data");
-    $cmd = ['/sbin/lvchange', '--refresh', "$vg/$volname"];
-    run_command($cmd, errmsg => "can't refresh LV '$vg/$volname' to zero-out its data");
-
     if ($scfg->{saferemove}) {
-	# avoid long running task, so we only rename here
-	$cmd = ['/sbin/lvrename', $vg, $volname, "del-$volname"];
-	run_command($cmd, errmsg => "lvrename '$vg/$volname' error");
+	for my $name (@$volnames) {
+	    # avoid long running task, so we only rename here
+	    my $cmd = ['/sbin/lvrename', $vg, $name, "del-$name"];
+	    run_command($cmd, errmsg => "lvrename '$vg/$name' error");
+	}
 	return $zero_out_worker;
     } else {
-	my $tmpvg = $scfg->{vgname};
-	$cmd = ['/sbin/lvremove', '-f', "$tmpvg/$volname"];
-	run_command($cmd, errmsg => "lvremove '$tmpvg/$volname' error");
+	for my $name (@$volnames) {
+	    my $tmpvg = $scfg->{vgname};
+	    my $cmd = ['/sbin/lvremove', '-f', "$tmpvg/$name"];
+	    run_command($cmd, errmsg => "lvremove '$tmpvg/$name' error");
+	}
     }
 
     return undef;
diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
index 3f83fae..0319ab2 100644
--- a/src/PVE/Storage/Plugin.pm
+++ b/src/PVE/Storage/Plugin.pm
@@ -959,7 +959,7 @@ sub alloc_snap_image {
 }
 
 sub free_image {
-    my ($class, $storeid, $scfg, $volname, $isBase, $format) = @_;
+    my ($class, $storeid, $scfg, $volname, $isBase, $format, $include_snapshots) = @_;
 
     die "cannot remove protected volume '$volname' on '$storeid'\n"
 	if $class->get_volume_attribute($scfg, $storeid, $volname, 'protected');
@@ -975,12 +975,29 @@ sub free_image {
     if (defined($format) && ($format eq 'subvol')) {
 	File::Path::remove_tree($path);
     } else {
-	if (!(-f $path || -l $path)) {
-	    warn "disk image '$path' does not exist\n";
-	    return undef;
+
+	my $volnames = [];
+	if ($include_snapshots) {
+ 	    my $snapshots =  $class->volume_snapshot_info($scfg, $storeid, $volname);
+	    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) = PVE::Storage::parse_volume_id($snap->{volid});
+		push @$volnames, $snap_volname;
+	    }
 	}
+	push @$volnames, $volname;
 
-	unlink($path) || die "unlink '$path' failed - $!\n";
+	for my $name (@$volnames) {
+	    my $path = $class->filesystem_path($scfg, $name);
+	    if (!(-f $path || -l $path)) {
+		warn "disk image '$path' does not exist\n";
+	    } else {
+		unlink($path) || die "unlink '$path' failed - $!\n";
+	    }
+	}
     }
 
     # try to cleanup directory to not clutter storage with empty $vmid dirs if
-- 
2.39.5



[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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