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

Fabian Grünbichler
2025-07-30T08:36:38Z

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+)
> + -
> + (? [^/\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)$!) {

[..]