[pve-devel] [PATCH storage 3/4] plugin: extend snapshot name parsing to legacy volnames

[Fabian Grünbichler]

On July 31, 2025 2:09 pm, Fiona Ebner wrote:
> Am 31.07.25 um 1:15 PM schrieb Fabian Grünbichler:
>> otherwise a volume like `100/oldstyle-100-disk-0.qcow2` can be snapshotted, but
>> the snapshot file is treated as a volume instead of a snapshot afterwards.
>> 
>> this also avoids issues with volnames with `vm-` in their names, similar to the
>> LVM fix for underscores.
>> 
>> Co-authored-by: Shannon Sterz <s.sterz at proxmox.com>
>> Signed-off-by: Fabian Grünbichler <f.gruenbichler at proxmox.com>
>> ---
>>  src/PVE/Storage/Plugin.pm | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>> 
>> diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
>> index affe7b0..db05e0e 100644
>> --- a/src/PVE/Storage/Plugin.pm
>> +++ b/src/PVE/Storage/Plugin.pm
>> @@ -702,9 +702,9 @@ sub cluster_lock_storage {
>>  }
>>  
>>  my sub parse_snap_name {
>> -    my ($name) = @_;
>> +    my ($filename, $volname) = @_;
>>  
>> -    if ($name =~ m/^snap-(.*)-vm(.*)$/) {
>> +    if ($filename =~ m/^snap-(.*)-\Q$volname\E$/) {
>>          return $1;
>>      }
>>  }
>> @@ -715,7 +715,7 @@ sub parse_name_dir {
>>      if ($name =~ m!^((vm-|base-|subvol-)(\d+)-[^/\s]+\.(raw|qcow2|vmdk|subvol))$!) {
>>          my $isbase = $2 eq 'base-' ? $2 : undef;
>>          return ($1, $4, $isbase); # (name, format, isBase)
>> -    } elsif (parse_snap_name($name)) {
>> +    } elsif ($name =~ m!^snap-.*\.qcow2$!) {
> 
> Should we reserve this schema for all formats and not just qcow2 while
> we're at it? For example, would keep open the possibility with TPM state
> snapshots as separate files or something similar that might pop up in
> the future.

I mean, we don't create such volumes anyway, it's only custom ones and
for those we collide with existing ones in any case. the vsplit patch
series should provide us with clean namespaces for the new vtypes, and
then we can simply only ever allocate using those..

> 
>>          die "'$name' is a snapshot filename, not a volume!\n";
>>      } elsif ($name =~ m!^((base-)?[^/\s]+\.(raw|qcow2|vmdk|subvol))$!) {
>>          warn "this volume name `$name` is deprecated, please use (base-/vm-/subvol-)-NNN- as prefix\n";
>> @@ -1753,7 +1753,7 @@ sub volume_snapshot_info {
>>  
>>          my $name = basename($path);
>>  
>> -        if (my $snapname = parse_snap_name($name)) {
>> +        if (my $snapname = parse_snap_name($name, basename($volname))) {
>>              return $snapname;
>>          } elsif ($name eq basename($volname)) {
>>              return 'current';
> 
>