Re: [pve-devel] [PATCH storage 10/26] plugins: update volname parsing for new naming convention

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

On July 29, 2025 1:15 pm, Wolfgang Bumiller wrote:
> - ESXi, ISCSIDirect, ISCSI:
>   Volumes are always vm volumes.
> - LVM:
>   New volumes use a `vol-vm-` or `vol-ct-` prefix.
> - Dir based, LvmThin, RBD:
>   Like LVM, but for base images a `base-` prefix is added
>   *additionally*, instead of *replacing* the `vm-` portion like we
>   used to.
> - ZFS:
>   VMs: `vol-vm-` prefix.
>   Containers: `subvol-ct-` prefix.

we could get rid of this historic anomaly while we're at it, and make
the new vtype just encoded as vol-ct for consistency - that those are
always filesystem datasets, and the vol-vm ones always zvols is an
implementation detail then?

>   Both also get an optional additional `base-` prefix for base
>   volumes.
> 
> Note: all new base- prefix come in front of the `(sub)vol-*` prefixes.
> 
> Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
> ---
>  src/PVE/Storage/ESXiPlugin.pm        |  2 +-
>  src/PVE/Storage/ISCSIDirectPlugin.pm |  2 +-
>  src/PVE/Storage/ISCSIPlugin.pm       |  2 +-
>  src/PVE/Storage/LVMPlugin.pm         | 19 ++++++++---
>  src/PVE/Storage/LvmThinPlugin.pm     | 13 ++++++++
>  src/PVE/Storage/Plugin.pm            | 48 +++++++++++++++++++++-------
>  src/PVE/Storage/RBDPlugin.pm         | 45 ++++++++++++++++++++++++--
>  src/PVE/Storage/ZFSPoolPlugin.pm     | 34 ++++++++++++++++----
>  8 files changed, 138 insertions(+), 27 deletions(-)
> 
> diff --git a/src/PVE/Storage/ESXiPlugin.pm b/src/PVE/Storage/ESXiPlugin.pm
> index eeb6a48..ea2f8f9 100644
> --- a/src/PVE/Storage/ESXiPlugin.pm
> +++ b/src/PVE/Storage/ESXiPlugin.pm
> @@ -421,7 +421,7 @@ sub parse_volname {
>  
>      my $format = 'raw';
>      $format = 'vmdk' if $volname =~ /\.vmdk$/;
> -    return ('images', $volname, 0, undef, undef, undef, $format);
> +    return ('vm-vol', $volname, 0, undef, undef, undef, $format);
>  }
>  
>  sub list_images {
> diff --git a/src/PVE/Storage/ISCSIDirectPlugin.pm b/src/PVE/Storage/ISCSIDirectPlugin.pm
> index 069a41f..f5b466e 100644
> --- a/src/PVE/Storage/ISCSIDirectPlugin.pm
> +++ b/src/PVE/Storage/ISCSIDirectPlugin.pm
> @@ -87,7 +87,7 @@ sub parse_volname {
>      my ($class, $volname) = @_;
>  
>      if ($volname =~ m/^lun(\d+)$/) {
> -        return ('images', $1, undef, undef, undef, undef, 'raw');
> +        return ('vm-vol', $1, undef, undef, undef, undef, 'raw');
>      }
>  
>      die "unable to parse iscsi volume name '$volname'\n";
> diff --git a/src/PVE/Storage/ISCSIPlugin.pm b/src/PVE/Storage/ISCSIPlugin.pm
> index 7b30955..4875a1f 100644
> --- a/src/PVE/Storage/ISCSIPlugin.pm
> +++ b/src/PVE/Storage/ISCSIPlugin.pm
> @@ -371,7 +371,7 @@ sub parse_volname {
>      my ($class, $volname) = @_;
>  
>      if ($volname =~ m!^\d+\.\d+\.\d+\.([^/\s]+)$!) {
> -        return ('images', $1, undef, undef, undef, undef, 'raw');
> +        return ('vm-vol', $1, undef, undef, undef, undef, 'raw');
>      }
>  
>      die "unable to parse iscsi volume name '$volname'\n";
> diff --git a/src/PVE/Storage/LVMPlugin.pm b/src/PVE/Storage/LVMPlugin.pm
> index 6694cf2..9b88c6a 100644
> --- a/src/PVE/Storage/LVMPlugin.pm
> +++ b/src/PVE/Storage/LVMPlugin.pm
> @@ -452,11 +452,22 @@ sub parse_volname {
>  
>      PVE::Storage::Plugin::parse_lvm_name($volname);
>  
> -    if ($volname =~ m/^(vm-(\d+)-\S+)$/) {
> -        my $name = $1;
> -        my $vmid = $2;
> +    if (
> +        $volname =~ m!^(?<name>
> +        (
> +            # New style volumes have a vtype:
> +              vol-(?<vtype>vm|ct)
> +
> +            # Old style:
> +            | vm
> +        )
> +        -(?<vmid>\d+)-\S+
> +        )$!xn
> +    ) {
> +        my ($name, $vmid, $vtype) = @+{qw(name vmid vtype)};
> +        $vtype = $vtype ? "$vtype-vol" : 'images';
>          my $format = $volname =~ m/\.qcow2$/ ? 'qcow2' : 'raw';
> -        return ('images', $name, $vmid, undef, undef, undef, $format);
> +        return ($vtype, $name, $vmid, undef, undef, undef, $format);
>      }
>  
>      die "unable to parse lvm volume name '$volname'\n";
> diff --git a/src/PVE/Storage/LvmThinPlugin.pm b/src/PVE/Storage/LvmThinPlugin.pm
> index cdf0fd0..751bd7b 100644
> --- a/src/PVE/Storage/LvmThinPlugin.pm
> +++ b/src/PVE/Storage/LvmThinPlugin.pm
> @@ -77,6 +77,19 @@ sub parse_volname {
>  
>      PVE::Storage::Plugin::parse_lvm_name($volname);
>  
> +    # New naming convention:
> +    if (
> +        $volname =~ m/^(?<name>
> +        (?<isbase>base-)?
> +        (?:vol-(?<vtype> vm|ct))-
> +        (?<vmid>\d+)-
> +        \S+)$/xn
> +    ) {
> +        my ($name, $vmid, $vtype, $isbase) = @+{qw(name vmid vtype isbase)};
> +        return ($vtype . '-vol', $name, $vmid, undef, undef, !!$isbase, 'raw');
> +    }
> +
> +    # Old naming convention:
>      if ($volname =~ m/^((vm|base)-(\d+)-\S+)$/) {
>          return ('images', $1, $3, undef, undef, $2 eq 'base', 'raw');
>      }
> diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
> index ae9d673..77bcfc7 100644
> --- a/src/PVE/Storage/Plugin.pm
> +++ b/src/PVE/Storage/Plugin.pm
> @@ -711,19 +711,46 @@ sub cluster_lock_storage {
>  my sub parse_snap_name {
>      my ($name) = @_;
>  
> +    if ($name =~ m/^snap-(.*)-vol-(?:vm|ct)(.*)$/) {
> +        return $1;
> +    }
> +
>      if ($name =~ m/^snap-(.*)-vm(.*)$/) {
>          return $1;
>      }
> +
> +    return;
>  }
>  
> +our $DISK_NAME_REGEX = qr!
> +    (?<name>
> +        (
> +            # New convention: 'base-' prefix, 'vol-vm' and 'subvol-ct'.
> +              (?<israw> (?<isbase>base-)? vol-    (?<vtype> vm|ct))
> +            | (?<issub> (?<isbase>base-)? subvol- (?<vtype> ct))
> +
> +            # Old convention: "vm" vs "base" and "subvol" vs "basevol"
> +            | (?<israw> vm)

`israw` is a weird identifier here. it's also not used anywhere, so
maybe we could drop it for now and think of a better name if we ever
need it?

> +            | (?<issub> subvol)
> +            | (?<israw> (?<isbase>base))
> +            | (?<issub> (?<isbase>basevol))

the old convention was actually a lot more loose, see below..

we probably should add a warning to pve8to9 for volumes on builtin
storage types that don't match this strict scheme at least?

> +        )
> +        -
> +        (?<vmid>\d+)
> +        -
> +        (?<label>[^/\s]+)
> +    )
> +!xn;
> +
>  sub parse_name_dir {
> -    my $name = shift;
> +    my ($name) = @_;
>  
> -    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)
> +    if ($name =~ m!^$DISK_NAME_REGEX\.(?<format>raw|qcow2|vmdk|subvol)$!xn) {
> +        my ($name, $israw, $issub, $isbase, $vmid, $vtype, $label, $format) =
> +            @+{qw(name israw issub isbase vmid vtype label format)};
> +        return ($name, $format, !!$isbase, $vtype ? "$vtype-vol" : "images");
>      } elsif ($name =~ m!^((base-)?[^/\s]+\.(raw|qcow2|vmdk|subvol))$!) {
> -        warn "this volume name `$name` is not supported anymore\n" if !parse_snap_name($name);
> +        die "this volume name `$name` is not supported anymore\n";

as discussed off list, this regressed recently and needs to be fixed -
we only want to skip snapshot volumes, and allow other things for the
legacy types..

>      }
>  
>      die "unable to parse volume filename '$name'\n";
> @@ -733,15 +760,14 @@ sub parse_volname {
>      my ($class, $volname) = @_;
>  
>      if ($volname =~ m!^(\d+)/(\S+)/(\d+)/(\S+)$!) {
> -        my ($basedvmid, $basename) = ($1, $2);
> +        my ($basedvmid, $basename, $vmid, $name) = ($1, $2, $3, $4);
>          parse_name_dir($basename);
> -        my ($vmid, $name) = ($3, $4);
> -        my (undef, $format, $isBase) = parse_name_dir($name);
> -        return ('images', $name, $vmid, $basename, $basedvmid, $isBase, $format);
> +        my (undef, $format, $isBase, $vtype) = parse_name_dir($name);
> +        return ($vtype, $name, $vmid, $basename, $basedvmid, $isBase, $format);
>      } elsif ($volname =~ m!^(\d+)/(\S+)$!) {
>          my ($vmid, $name) = ($1, $2);
> -        my (undef, $format, $isBase) = parse_name_dir($name);
> -        return ('images', $name, $vmid, undef, undef, $isBase, $format);
> +        my (undef, $format, $isBase, $vtype) = parse_name_dir($name);
> +        return ($vtype, $name, $vmid, undef, undef, $isBase, $format);
>      } elsif ($volname =~ m!^iso/([^/]+$PVE::Storage::ISO_EXT_RE_0)$!) {
>          return ('iso', $1, undef, undef, undef, undef, 'raw');
>      } elsif ($volname =~ m!^vztmpl/([^/]+$PVE::Storage::VZTMPL_EXT_RE_1)$!) {

[..]


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