[pve-devel] [PATCH storage 10/26] plugins: update volname parsing for new naming convention
Wolfgang Bumiller
w.bumiller at proxmox.com
Wed Jul 30 10:53:48 CEST 2025
On Wed, Jul 30, 2025 at 10:37:27AM +0200, Fabian Grünbichler wrote:
> 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?
But this would also make it difficult if we ever want to allow zvols
with file systems. And given that the code falls through to Plugin.pm in
some places (and dir plugins can theoretically have size=0 containers
dirs), it seemed much safer to stick with a scheme that works across
more storage types. Eg. we fall through to Plugin.pm via
`get_next_vm_diskname()` which is the thing that causes the "subvol-ct-"
prefix for "format == subvol". And unlike dir plugins, our ZFS plugin
does not have a distinguishing format suffix for this...
>
> > 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 at 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?
(Yeah... I just left it in for better alignment/documentation :P)
>
> > + | (?<issub> subvol)
> > + | (?<israw> (?<isbase>base))
> > + | (?<issub> (?<isbase>basevol))
>
> the old convention was actually a lot more loose, see below..
IMO the more loose part should be in an `else` in `parse_name_dir`.
This regex is used in other plugins and I don't want this regex to allow
zvols named `💩`.
>
> 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)$!) {
>
> [..]
More information about the pve-devel
mailing list