[pve-devel] [PATCH storage v2 1/2] fix #3873: btrfs: check for correct subvolume taking snapshot

[Fiona Ebner]

Am 18.02.25 um 17:28 schrieb Maximiliano Sandoval:
> Suppose we are taking a snapshot of VM 100's disk-0. The
> dir_glob_foreach runs over $path=/subvolume/images/100, lists all
> snapshot names and appends their names to the path of the disk, e.g.
> /subvolume/images/vm-100-disk-0 at SNAP_NAME, but the original directory
> $path might contain a second disk `vm-100-disk-1` which is also listed
> by the dir_glib_foreach.
> 
> This change adds a helper foreach_snapshot_of_subvol which only lists
> snapshots for the current volume.
> 
> The helper is also implemented in the other place where we iterate over
> snapshots.
> 
> Signed-off-by: Maximiliano Sandoval <m.sandoval at proxmox.com>
> ---
> 
> Differences from v1:
>  - Add a helper to run callback on each snapshot.
> 
>  src/PVE/Storage/BTRFSPlugin.pm | 25 +++++++++++++++++++------
>  1 file changed, 19 insertions(+), 6 deletions(-)
> 
> diff --git a/src/PVE/Storage/BTRFSPlugin.pm b/src/PVE/Storage/BTRFSPlugin.pm
> index cadd9d1..d21c831 100644
> --- a/src/PVE/Storage/BTRFSPlugin.pm
> +++ b/src/PVE/Storage/BTRFSPlugin.pm
> @@ -419,6 +419,20 @@ my sub foreach_subvol : prototype($$) {
>      })
>  }
>  
> +# Calls `$code->($volume, $snap_name)` for each snapshot of the subvolume.
> +my sub foreach_snapshot_of_subvol : prototype($$) {

We can just rename foreach_subvol() and the $BTRFS_VOL_REGEX instead of
adding this new wrapper. As already written in the review of v1, the
regex requires a snapshot to be present. This means the regex is a
misnomer and the foreach_subvol() helper already operates only on
snapshots and can also be renamed. It'd be more concise/straightforward
and otherwise, those remain confusing.

> +    my ($subvol, $code) = @_;
> +
> +    my $basename = basename($subvol);
> +    my $dir = dirname($subvol);
> +    foreach_subvol($dir, sub {
> +	my ($volume, $name, $snap) = @_;
> +	return if $name ne $basename;
> +	return if !defined $snap;
> +	$code->($volume, $snap);
> +   });
> +}
> +
>  sub free_image {
>      my ($class, $storeid, $scfg, $volname, $isBase, $_format) = @_;
>