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

[Fiona Ebner]

Am 31.07.25 um 2:20 PM schrieb 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..

Yes, makes sense to not restrict further than necessary now if we
already have a path forward :)