* [PATCH storage 0/3] generalize volname_for_format across storage plugins
@ 2026-08-12 15:32 Elias Huhsovitz
2026-08-12 15:32 ` [PATCH storage 1/3] plugin: refactor format parsing in parse_volname and parse_name_dir Elias Huhsovitz
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Elias Huhsovitz @ 2026-08-12 15:32 UTC (permalink / raw)
To: pve-devel; +Cc: Elias Huhsovitz
Previously, only the LVM plugin explicitly validated that a requested
volume format matched the provided volume name. This inconsistency
allowed other storage backends to silently accept mismatched names and
formats, leading to confusing states (as was previously mentioned in
bug #7811).
Centralize volname_for_format() in the base PVE::Storage::Plugin.
This design allows individual plugins to override specific parsing
behavior while sharing the core validation logic.
The new validation method operates in two modes:
* Lax mode (default): Names without an extension are
automatically adapted to the requested format. Names that
explicitly spell out a contradicting format are rejected.
* Strict mode: Any format mismatch results in an error.
To preserve backwards compatibility, only the default lax
mode is currently used in alloc_image() and rename_volume().
Should strict volume name guidelines be needed in the
future, the strict parameter can be enabled at the call
sites.
Potential Pitfalls
------------------
Because this introduces stricter validation checks, there is a chance
it may break existing systems, scripts, or automation that previously
relied on passing mismatched volume names and formats without
triggering an error.
Patch Overview
--------------
Elias Huhsovitz (3):
plugin: refactor format parsing in parse_volname and parse_name_dir
plugin: generalize volname_for_format across storage plugins
test: plugin: add unit tests for volname_for_format
src/PVE/Storage/BTRFSPlugin.pm | 6 +-
src/PVE/Storage/LVMPlugin.pm | 39 +-
src/PVE/Storage/LvmThinPlugin.pm | 18 +-
src/PVE/Storage/Plugin.pm | 99 +++-
src/PVE/Storage/RBDPlugin.pm | 22 +-
src/PVE/Storage/ZFSPlugin.pm | 2 +
src/PVE/Storage/ZFSPoolPlugin.pm | 37 ++
src/test/run_plugin_tests.pl | 1 +
src/test/volname_for_format_test.pm | 813 ++++++++++++++++++++++++++++
9 files changed, 1005 insertions(+), 32 deletions(-)
create mode 100644 src/test/volname_for_format_test.pm
--
2.47.3
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH storage 1/3] plugin: refactor format parsing in parse_volname and parse_name_dir
2026-08-12 15:32 [PATCH storage 0/3] generalize volname_for_format across storage plugins Elias Huhsovitz
@ 2026-08-12 15:32 ` Elias Huhsovitz
2026-08-12 15:32 ` [PATCH storage 2/3] plugin: generalize volname_for_format across storage plugins Elias Huhsovitz
2026-08-12 15:32 ` [PATCH storage 3/3] test: plugin: add unit tests for volname_for_format Elias Huhsovitz
2 siblings, 0 replies; 4+ messages in thread
From: Elias Huhsovitz @ 2026-08-12 15:32 UTC (permalink / raw)
To: pve-devel; +Cc: Elias Huhsovitz
Previously, `parse_volname` in several plugins (LVM, LvmThin, RBD)
hardcoded the returned format to 'raw', ignoring any format extension
present in the volume name. This prevented the detection of mismatched
formats, such as passing a `.qcow2` name to a raw-only storage.
Update `parse_volname` in `LVMPlugin`, `LvmThinPlugin`, and `RBDPlugin`
to dynamically extract the format from the volume name extension (e.g.,
`.raw`, `.qcow2`, `.vmdk`, `.subvol`), falling back to 'raw' if no known
extension is present.
Ajust `parse_name_dir` in `Plugin.pm` to allow parsing of names without
an extension, returning 'raw' as the default format. This prepares the
base class for extension-less volume names used by block storages.
Signed-off-by: Elias Huhsovitz <e.huhsovitz@proxmox.com>
---
src/PVE/Storage/LVMPlugin.pm | 2 +-
src/PVE/Storage/LvmThinPlugin.pm | 6 +++++-
src/PVE/Storage/Plugin.pm | 2 ++
src/PVE/Storage/RBDPlugin.pm | 8 +++++++-
4 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/src/PVE/Storage/LVMPlugin.pm b/src/PVE/Storage/LVMPlugin.pm
index 67ba0f7..0f2ef66 100644
--- a/src/PVE/Storage/LVMPlugin.pm
+++ b/src/PVE/Storage/LVMPlugin.pm
@@ -526,7 +526,7 @@ sub parse_volname {
if ($volname =~ m/^(vm-(\d+)-\S+)$/) {
my $name = $1;
my $vmid = $2;
- my $format = $volname =~ m/\.qcow2$/ ? 'qcow2' : 'raw';
+ my $format = ($volname =~ m/\.(raw|qcow2|vmdk|subvol)$/) ? $1 : 'raw';
return ('images', $name, $vmid, undef, undef, undef, $format);
}
diff --git a/src/PVE/Storage/LvmThinPlugin.pm b/src/PVE/Storage/LvmThinPlugin.pm
index cadf343..d44e04d 100644
--- a/src/PVE/Storage/LvmThinPlugin.pm
+++ b/src/PVE/Storage/LvmThinPlugin.pm
@@ -66,7 +66,11 @@ sub parse_volname {
PVE::Storage::Plugin::parse_lvm_name($volname);
if ($volname =~ m/^((vm|base)-(\d+)-\S+)$/) {
- return ('images', $1, $3, undef, undef, $2 eq 'base', 'raw');
+ my $name = $1;
+ my $isbase = $2 eq 'base';
+ my $vmid = $3;
+ my $format = ($volname =~ m/\.(raw|qcow2|vmdk|subvol)$/) ? $1 : 'raw';
+ return ('images', $name, $vmid, undef, undef, $isbase, $format);
}
die "unable to parse lvm volume name '$volname'\n";
diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
index 4f69f9b..c5046a4 100644
--- a/src/PVE/Storage/Plugin.pm
+++ b/src/PVE/Storage/Plugin.pm
@@ -789,6 +789,8 @@ sub parse_name_dir {
return ($1, $4, $isbase); # (name, format, isBase)
} elsif ($name =~ m!^snap-.*\.qcow2$!) {
die "'$name' is a snapshot filename, not a volume!\n";
+ } elsif ($name =~ m!^((base-)?[^/\s]+)$!) {
+ return ($1, 'raw', $2); # (name, format, isBase)
} elsif ($name =~ m!^((base-)?[^/\s]+\.(raw|qcow2|vmdk|subvol))$!) {
return ($1, $3, $2); # (name ,format, isBase)
}
diff --git a/src/PVE/Storage/RBDPlugin.pm b/src/PVE/Storage/RBDPlugin.pm
index b537425..8ebb999 100644
--- a/src/PVE/Storage/RBDPlugin.pm
+++ b/src/PVE/Storage/RBDPlugin.pm
@@ -510,7 +510,13 @@ sub parse_volname {
my ($class, $volname) = @_;
if ($volname =~ m/^((base-(\d+)-\S+)\/)?((base)?(vm)?-(\d+)-\S+)$/) {
- return ('images', $4, $7, $2, $3, $5, 'raw');
+ my $basename = $2;
+ my $basevmid = $3;
+ my $name = $4;
+ my $isbase = $5;
+ my $vmid = $7;
+ my $format = ($volname =~ m/\.(raw|qcow2|vmdk|subvol)$/) ? $1 : 'raw';
+ return ('images', $name, $vmid, $basename, $basevmid, $isbase, $format);
}
die "unable to parse rbd volume name '$volname'\n";
--
2.47.3
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH storage 2/3] plugin: generalize volname_for_format across storage plugins
2026-08-12 15:32 [PATCH storage 0/3] generalize volname_for_format across storage plugins Elias Huhsovitz
2026-08-12 15:32 ` [PATCH storage 1/3] plugin: refactor format parsing in parse_volname and parse_name_dir Elias Huhsovitz
@ 2026-08-12 15:32 ` Elias Huhsovitz
2026-08-12 15:32 ` [PATCH storage 3/3] test: plugin: add unit tests for volname_for_format Elias Huhsovitz
2 siblings, 0 replies; 4+ messages in thread
From: Elias Huhsovitz @ 2026-08-12 15:32 UTC (permalink / raw)
To: pve-devel; +Cc: Elias Huhsovitz
Move `volname_for_format()` from `LVMPlugin` into `Plugin` to make it
reusable for all storage backends. This function reconciles the
requested format with the volume name and ensures they match.
Introduce `get_parsed_format()` as an extension point. Subclasses whose
volume names encode the format differently (e.g., LVM, ZFS) override
this method to derive the format via `parse_volname()`.
Add `FORMAT_EXTENSION` constants to affected plugins, enabling the
generation of useful format mismatch suggestions.
Call `volname_for_format()` in `alloc_image()` and `rename_volume()`
for the `Plugin`, `BTRFS`, `LVM`, `LvmThin`, `RBD`, `ZFS`, and `ZFSPool`
backends.
In `ZFSPoolPlugin`, override `volname_for_format()` and
`volname_with_format()` to handle its unique prefix-based naming scheme
(e.g., `vm-` vs `subvol-`).
Signed-off-by: Elias Huhsovitz <e.huhsovitz@proxmox.com>
---
src/PVE/Storage/BTRFSPlugin.pm | 6 +-
src/PVE/Storage/LVMPlugin.pm | 37 ++++++------
src/PVE/Storage/LvmThinPlugin.pm | 12 ++++
src/PVE/Storage/Plugin.pm | 97 ++++++++++++++++++++++++++++++--
src/PVE/Storage/RBDPlugin.pm | 14 +++++
src/PVE/Storage/ZFSPlugin.pm | 2 +
src/PVE/Storage/ZFSPoolPlugin.pm | 37 ++++++++++++
7 files changed, 176 insertions(+), 29 deletions(-)
diff --git a/src/PVE/Storage/BTRFSPlugin.pm b/src/PVE/Storage/BTRFSPlugin.pm
index fb47aa0..fa28592 100644
--- a/src/PVE/Storage/BTRFSPlugin.pm
+++ b/src/PVE/Storage/BTRFSPlugin.pm
@@ -333,10 +333,7 @@ sub alloc_image {
$name = $class->find_free_diskname($storeid, $scfg, $vmid, $fmt, 1) if !$name;
- my (undef, $tmpfmt) = PVE::Storage::Plugin::parse_name_dir($name);
-
- die "illegal name '$name' - wrong extension for format ('$tmpfmt != '$fmt')\n"
- if $tmpfmt ne $fmt;
+ $name = $class->volname_for_format($name, $fmt, 0);
# End copy from Plugin.pm
@@ -974,6 +971,7 @@ sub rename_volume {
$target_volname = $class->find_free_diskname($storeid, $scfg, $target_vmid, $format, 1)
if !$target_volname;
+ $target_volname = $class->volname_for_format($target_volname, $format, 0);
$target_volname = "$target_vmid/$target_volname";
my $basedir = $class->get_subdir($scfg, 'images');
diff --git a/src/PVE/Storage/LVMPlugin.pm b/src/PVE/Storage/LVMPlugin.pm
index 0f2ef66..6ed613d 100644
--- a/src/PVE/Storage/LVMPlugin.pm
+++ b/src/PVE/Storage/LVMPlugin.pm
@@ -16,6 +16,16 @@ use PVE::Storage::Plugin;
use base qw(PVE::Storage::Plugin);
+use constant FORMAT_EXTENSION => {
+ raw => '',
+ qcow2 => 'qcow2',
+};
+
+use constant EXTENSION_FORMAT => {
+ '' => 'raw',
+ 'qcow2' => 'qcow2',
+};
+
# lvm helper functions
my $ignore_no_medium_warnings = sub {
@@ -738,27 +748,10 @@ my sub alloc_lvm_image {
}
-# Only 'qcow2' names carry an extension, like the ones find_free_diskname() generates, so add it
-# for callers passing a fixed name, like cloud-init drives. A name that already spells out another
-# format stays an error, it states an intent that the requested format contradicts.
-my sub volname_for_format {
- my ($class, $name, $fmt) = @_;
-
- return $name if $fmt ne 'raw' && $fmt ne 'qcow2'; # alloc_lvm_image() reports unsupported ones
+sub get_parsed_format {
+ my ($class, $name) = @_;
- my $name_fmt = ($class->parse_volname($name))[6];
- return $name if $name_fmt eq $fmt;
-
- if ($fmt eq 'raw') {
- my $suggested_name = $name =~ s/\.\Q$name_fmt\E$//r;
- die "volume name '$name' does not match requested format '$fmt'"
- . " (did you mean '$suggested_name'?)\n";
- }
-
- my $adapted_name = "$name.$fmt";
- warn "volume name '$name' is missing the '.$fmt' extension - allocating '$adapted_name'\n";
-
- return $adapted_name;
+ return ($class->parse_volname($name))[6];
}
sub alloc_image {
@@ -767,7 +760,7 @@ sub alloc_image {
$name = $class->find_free_diskname($storeid, $scfg, $vmid, $fmt)
if !$name;
- $name = volname_for_format($class, $name, $fmt);
+ $name = $class->volname_for_format($name, $fmt, 0);
alloc_lvm_image($class, $storeid, $scfg, $vmid, $fmt, $name, $size);
@@ -1542,6 +1535,8 @@ sub rename_volume {
$target_volname = $class->find_free_diskname($storeid, $scfg, $target_vmid, $format)
if !$target_volname;
+ $target_volname = $class->volname_for_format($target_volname, $format, 0);
+
my $vg = $scfg->{vgname};
my $lvs = lvm_list_volumes($vg);
die "target volume '${target_volname}' already exists\n"
diff --git a/src/PVE/Storage/LvmThinPlugin.pm b/src/PVE/Storage/LvmThinPlugin.pm
index d44e04d..0c25973 100644
--- a/src/PVE/Storage/LvmThinPlugin.pm
+++ b/src/PVE/Storage/LvmThinPlugin.pm
@@ -25,6 +25,10 @@ use PVE::Storage::LVMPlugin;
use base qw(PVE::Storage::LVMPlugin);
+use constant FORMAT_EXTENSION => {
+ raw => '',
+};
+
sub type {
return 'lvmthin';
}
@@ -100,6 +104,12 @@ my $set_lv_autoactivation = sub {
warn "could not set autoactivation: $@" if $@;
};
+sub get_parsed_format {
+ my ($class, $name) = @_;
+
+ return ($class->parse_volname($name))[6];
+}
+
sub alloc_image {
my ($class, $storeid, $scfg, $vmid, $fmt, $name, $size) = @_;
@@ -117,6 +127,8 @@ sub alloc_image {
$name = $class->find_free_diskname($storeid, $scfg, $vmid)
if !$name;
+ $name = $class->volname_for_format($name, $fmt, 0);
+
my $cmd = [
'/sbin/lvcreate',
'-aly',
diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
index c5046a4..d596397 100644
--- a/src/PVE/Storage/Plugin.pm
+++ b/src/PVE/Storage/Plugin.pm
@@ -27,6 +27,13 @@ use constant COMPRESSOR_RE => join('|', KNOWN_COMPRESSION_FORMATS);
use constant LOG_EXT => ".log";
use constant NOTES_EXT => ".notes";
+use constant FORMAT_EXTENSION => {
+ raw => 'raw',
+ qcow2 => 'qcow2',
+ vmdk => 'vmdk',
+ subvol => 'subvol',
+};
+
our @COMMON_TAR_FLAGS = qw(
--one-file-system
-p --sparse --numeric-owner --acls
@@ -838,6 +845,89 @@ sub parse_volname {
die "unable to parse directory volume name '$volname'\n";
}
+=item get_parsed_format($class, $name)
+
+Return the disk format encoded in the given volume name.
+
+This is an extension point. Subclasses whose volume names encode the format
+differently override this method. LVM and ZFS, for example, derive the format
+via C<parse_volname> .
+
+returns: the format indicated by the name.
+
+=cut
+
+sub get_parsed_format {
+ my ($class, $name) = @_;
+
+ return (parse_name_dir($name))[1];
+}
+
+sub is_valid_format {
+ my ($class, $fmt) = @_;
+
+ return defined($class->FORMAT_EXTENSION->{$fmt});
+}
+
+=item volname_for_format($class, $name, $fmt, $strict)
+
+Reconcile the requested format with the volume name and return the name to
+allocate.
+
+Dies immediately if C<$fmt> is not a valid format for this plugin.
+
+If the name already encodes the requested format, returns the name unchanged.
+
+If C<$strict> is true, any format mismatch dies with a suggestion for the
+corrected name.
+
+If C<$strict> is false (lax mode), a name without a file extension is adapted
+to the requested format and a warning is emitted. A name that already carries
+a file extension dies with a suggestion, because the extension indicates an
+explicit format choice that contradicts the request.
+
+returns: the name to allocate
+
+=cut
+
+sub volname_for_format {
+ my ($class, $name, $fmt, $strict) = @_;
+
+ die "unsupported format '$fmt'\n" if !($class->is_valid_format($fmt));
+
+ my $parsed_volname_fmt = $class->get_parsed_format($name);
+
+ return $name if $parsed_volname_fmt eq $fmt;
+
+ my $suggestion = $class->volname_with_format($name, $fmt);
+
+ if ($strict || $name =~ /\.[^.]+$/) {
+ die "illegal name $name - volume name does not match requested format "
+ . "'$fmt' (did you mean '$suggestion'?)\n";
+ }
+
+ warn "volume name '$name' is missing the '.$fmt' extension - allocating '$suggestion'\n";
+
+ return $suggestion;
+}
+
+sub get_format_extension {
+ my ($class, $fmt) = @_;
+
+ return $class->FORMAT_EXTENSION->{$fmt};
+}
+
+sub volname_with_format {
+ my ($class, $name, $fmt) = @_;
+
+ my $correct_fmt_ext = $class->get_format_extension($fmt);
+
+ $name =~ s/\.[^.]+$//;
+
+ return $name if !$correct_fmt_ext;
+ return "$name.$correct_fmt_ext";
+}
+
my $vtype_subdirs = {
images => 'images',
rootdir => 'private',
@@ -1058,10 +1148,7 @@ sub alloc_image {
$name = $class->find_free_diskname($storeid, $scfg, $vmid, $fmt, 1) if !$name;
- my (undef, $tmpfmt) = parse_name_dir($name);
-
- die "illegal name '$name' - wrong extension for format ('$tmpfmt != '$fmt')\n"
- if $tmpfmt ne $fmt;
+ $name = $class->volname_for_format($name, $fmt, 0);
my $path = "$imagedir/$name";
@@ -2391,6 +2478,8 @@ sub rename_volume {
$target_volname = $class->find_free_diskname($storeid, $scfg, $target_vmid, $format, 1)
if !$target_volname;
+ $target_volname = $class->volname_for_format($target_volname, $format, 0);
+
my $basedir = $class->get_subdir($scfg, 'images');
mkpath "${basedir}/${target_vmid}";
diff --git a/src/PVE/Storage/RBDPlugin.pm b/src/PVE/Storage/RBDPlugin.pm
index 8ebb999..95b65d6 100644
--- a/src/PVE/Storage/RBDPlugin.pm
+++ b/src/PVE/Storage/RBDPlugin.pm
@@ -22,6 +22,10 @@ use PVE::Storage::Common;
use base qw(PVE::Storage::Plugin);
+use constant FORMAT_EXTENSION => {
+ raw => '',
+};
+
my $get_parent_image_name = sub {
my ($parent) = @_;
return undef if !$parent;
@@ -522,6 +526,12 @@ sub parse_volname {
die "unable to parse rbd volume name '$volname'\n";
}
+sub get_parsed_format {
+ my ($class, $name) = @_;
+
+ return ($class->parse_volname($name))[6];
+}
+
sub path {
my ($class, $scfg, $volname, $storeid, $snapname) = @_;
@@ -715,6 +725,8 @@ sub alloc_image {
$name = $class->find_free_diskname($storeid, $scfg, $vmid) if !$name;
+ $name = $class->volname_for_format($name, $fmt, 0);
+
my @options = (
'--image-format', 2, '--size', int(($size + 1023) / 1024),
);
@@ -1087,6 +1099,8 @@ sub rename_volume {
$target_volname = $class->find_free_diskname($storeid, $scfg, $target_vmid, $format)
if !$target_volname;
+ $target_volname = $class->volname_for_format($target_volname, $format, 0);
+
die "target volume '${target_volname}' already exists\n"
if rbd_volume_exists($scfg, $storeid, $target_volname);
diff --git a/src/PVE/Storage/ZFSPlugin.pm b/src/PVE/Storage/ZFSPlugin.pm
index 74e0a08..bfae513 100644
--- a/src/PVE/Storage/ZFSPlugin.pm
+++ b/src/PVE/Storage/ZFSPlugin.pm
@@ -366,6 +366,8 @@ sub alloc_image {
$volname = $class->find_free_diskname($storeid, $scfg, $vmid, $fmt) if !$volname;
+ $volname = $class->volname_for_format($volname, $fmt, 0);
+
$class->zfs_create_zvol($scfg, $volname, $size);
my $guid = $class->zfs_create_lu($scfg, $volname);
diff --git a/src/PVE/Storage/ZFSPoolPlugin.pm b/src/PVE/Storage/ZFSPoolPlugin.pm
index 55f0bbc..4fecebb 100644
--- a/src/PVE/Storage/ZFSPoolPlugin.pm
+++ b/src/PVE/Storage/ZFSPoolPlugin.pm
@@ -150,6 +150,37 @@ sub parse_volname {
die "unable to parse zfs volume name '$volname'\n";
}
+sub get_parsed_format {
+ my ($class, $name) = @_;
+
+ return ($class->parse_volname($name))[6];
+}
+
+# ZFS volume names always encode their format in the name prefix (vm- for raw
+# zvols, subvol- for subvolumes). Any mismatch therefore spells out a
+# contradicting format and is an error, regardless of $strict.
+sub volname_for_format {
+ my ($class, $name, $fmt, $strict) = @_;
+
+ my $name_fmt = $class->get_parsed_format($name);
+ return $name if $name_fmt eq $fmt;
+
+ my $suggestion = $class->volname_with_format($name, $name_fmt, $fmt);
+
+ die "illegal name $name - volume name does not match requested format "
+ . "'$fmt' (did you mean '$suggestion'?)\n";
+}
+
+sub volname_with_format {
+ my ($class, $name, $name_fmt, $fmt) = @_;
+
+ if ($fmt eq 'subvol') {
+ return $name =~ s/^(?:vm|base|basevol)-/subvol-/r;
+ }
+
+ return $name =~ s/^(?:subvol|basevol|base)-/vm-/r;
+}
+
# virtual zfs methods (subclass can overwrite them)
sub on_add_hook {
@@ -273,6 +304,8 @@ sub alloc_image {
$volname = $class->find_free_diskname($storeid, $scfg, $vmid, $fmt)
if !$volname;
+ $volname = $class->volname_for_format($volname, $fmt, 0);
+
$class->zfs_create_zvol($scfg, $volname, $size);
$class->zfs_wait_for_zvol_link($scfg, $volname);
@@ -283,6 +316,8 @@ sub alloc_image {
$volname = $class->find_free_diskname($storeid, $scfg, $vmid, $fmt)
if !$volname;
+ $volname = $class->volname_for_format($volname, $fmt, 0);
+
die "illegal name '$volname' - should be 'subvol-$vmid-*'\n"
if $volname !~ m/^subvol-$vmid-/;
@@ -960,6 +995,8 @@ sub rename_volume {
$target_volname = $class->find_free_diskname($storeid, $scfg, $target_vmid, $format)
if !$target_volname;
+ $target_volname = $class->volname_for_format($target_volname, $format, 0);
+
my $pool = $scfg->{pool};
my $source_zfspath = "${pool}/${source_image}";
my $target_zfspath = "${pool}/${target_volname}";
--
2.47.3
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH storage 3/3] test: plugin: add unit tests for volname_for_format
2026-08-12 15:32 [PATCH storage 0/3] generalize volname_for_format across storage plugins Elias Huhsovitz
2026-08-12 15:32 ` [PATCH storage 1/3] plugin: refactor format parsing in parse_volname and parse_name_dir Elias Huhsovitz
2026-08-12 15:32 ` [PATCH storage 2/3] plugin: generalize volname_for_format across storage plugins Elias Huhsovitz
@ 2026-08-12 15:32 ` Elias Huhsovitz
2 siblings, 0 replies; 4+ messages in thread
From: Elias Huhsovitz @ 2026-08-12 15:32 UTC (permalink / raw)
To: pve-devel; +Cc: Elias Huhsovitz
Add unit tests for `volname_for_format()` across all touched storage
plugins.
The test suite covers:
- Matching format and volume-name combinations.
- Mismatched names and their suggested corrections.
- Base-image prefixes and multi-dot names.
- Non-format suffixes and invalid names that must propagate parser
errors.
- Unsupported formats, ensuring they are rejected immediately.
Signed-off-by: Elias Huhsovitz <e.huhsovitz@proxmox.com>
---
src/test/run_plugin_tests.pl | 1 +
src/test/volname_for_format_test.pm | 813 ++++++++++++++++++++++++++++
2 files changed, 814 insertions(+)
create mode 100644 src/test/volname_for_format_test.pm
diff --git a/src/test/run_plugin_tests.pl b/src/test/run_plugin_tests.pl
index 8bce9d3..692872a 100755
--- a/src/test/run_plugin_tests.pl
+++ b/src/test/run_plugin_tests.pl
@@ -12,6 +12,7 @@ my $harness = TAP::Harness->new({ verbosity => -1 });
my $res = $harness->runtests(
"archive_info_test.pm",
"parse_volname_test.pm",
+ "volname_for_format_test.pm",
"list_volumes_test.pm",
"path_to_volume_id_test.pm",
"get_subdir_test.pm",
diff --git a/src/test/volname_for_format_test.pm b/src/test/volname_for_format_test.pm
new file mode 100644
index 0000000..7e0ed63
--- /dev/null
+++ b/src/test/volname_for_format_test.pm
@@ -0,0 +1,813 @@
+package PVE::Storage::TestVolnameForFormat;
+
+use v5.36;
+
+# These modules belong to pve-manager.
+# Stub them to prevent compilation failures when testing isolated storage plugins.
+BEGIN {
+ $INC{'PVE/Storage.pm'} = '/dev/null';
+ $INC{'PVE/GuestImport/OVF.pm'} = '/dev/null';
+
+ package PVE::Storage;
+ 1;
+
+ package PVE::GuestImport::OVF;
+ 1;
+}
+
+use lib qw(..);
+
+use Test::More;
+
+use PVE::Storage::Plugin;
+use PVE::Storage::LVMPlugin;
+use PVE::Storage::LvmThinPlugin;
+use PVE::Storage::RBDPlugin;
+use PVE::Storage::ZFSPoolPlugin;
+use PVE::Storage::ZFSPlugin;
+use PVE::Storage::BTRFSPlugin;
+
+# Each case exercises volname_for_format() and declares exactly one of:
+# expect_pass - must not die and must return the input name unchanged
+# expect_return - must not die and must return this adapted name
+# expect_suggestion - must die and suggest this corrected name
+# expect_error - must die with an error matching this regex
+#
+# The 'strict' flag selects the mode: 1 = strict (old verify_volname_format
+# behavior), 0 = lax (default). A case without 'strict' verifies that the
+# default mode is lax.
+my $tests = [
+ # ======================================================================
+ # PVE::Storage::Plugin (base / dir-like behavior)
+ # Every format carries a file extension, so a mismatch always spells out a
+ # contradicting format and dies even in lax mode. parse_name_dir rejects
+ # extension-less names before any adaptation could happen.
+ # ======================================================================
+
+ # --- strict ---
+ {
+ desc => 'volname_for_format_strict_PluginMatchingRawFormat_Succeeds',
+ class => 'PVE::Storage::Plugin',
+ name => 'vm-100-disk-0.raw',
+ fmt => 'raw',
+ strict => 1,
+ expect_pass => 1,
+ },
+ {
+ desc => 'volname_for_format_strict_PluginMatchingQcow2Format_Succeeds',
+ class => 'PVE::Storage::Plugin',
+ name => 'vm-100-disk-0.qcow2',
+ fmt => 'qcow2',
+ strict => 1,
+ expect_pass => 1,
+ },
+ {
+ desc => 'volname_for_format_strict_PluginMatchingSubvolFormat_Succeeds',
+ class => 'PVE::Storage::Plugin',
+ name => 'subvol-100-disk-0.subvol',
+ fmt => 'subvol',
+ strict => 1,
+ expect_pass => 1,
+ },
+ {
+ desc => 'volname_for_format_strict_PluginMatchingVmdkFormat_Succeeds',
+ class => 'PVE::Storage::Plugin',
+ name => 'vm-100-disk-0.vmdk',
+ fmt => 'vmdk',
+ strict => 1,
+ expect_pass => 1,
+ },
+ {
+ desc => 'volname_for_format_strict_PluginMatchingBaseQcow2Format_Succeeds',
+ class => 'PVE::Storage::Plugin',
+ name => 'base-100-disk-0.qcow2',
+ fmt => 'qcow2',
+ strict => 1,
+ expect_pass => 1,
+ },
+ {
+ desc =>
+ 'volname_for_format_strict_PluginMismatchedRawNameWithQcow2Format_ThrowsSuggestionError',
+ class => 'PVE::Storage::Plugin',
+ name => 'vm-100-disk-0.raw',
+ fmt => 'qcow2',
+ strict => 1,
+ expect_suggestion => 'vm-100-disk-0.qcow2',
+ },
+ {
+ desc =>
+ 'volname_for_format_strict_PluginMismatchedQcow2NameWithRawFormat_ThrowsSuggestionError',
+ class => 'PVE::Storage::Plugin',
+ name => 'vm-100-disk-0.qcow2',
+ fmt => 'raw',
+ strict => 1,
+ expect_suggestion => 'vm-100-disk-0.raw',
+ },
+ {
+ desc =>
+ 'volname_for_format_strict_PluginMismatchedQcow2NameWithSubvolFormat_ThrowsSuggestionError',
+ class => 'PVE::Storage::Plugin',
+ name => 'vm-100-disk-0.qcow2',
+ fmt => 'subvol',
+ strict => 1,
+ expect_suggestion => 'vm-100-disk-0.subvol',
+ },
+ {
+ desc =>
+ 'volname_for_format_strict_PluginMismatchedBaseQcow2NameWithRawFormat_ThrowsSuggestionError',
+ class => 'PVE::Storage::Plugin',
+ name => 'base-100-disk-0.qcow2',
+ fmt => 'raw',
+ strict => 1,
+ expect_suggestion => 'base-100-disk-0.raw',
+ },
+ {
+ desc =>
+ 'volname_for_format_strict_PluginMismatchedMultiDotNameWithRawFormat_ThrowsSuggestionError',
+ class => 'PVE::Storage::Plugin',
+ name => 'vm-100-disk-0.backup.qcow2',
+ fmt => 'raw',
+ strict => 1,
+ expect_suggestion => 'vm-100-disk-0.backup.raw',
+ },
+ # --- lax ---
+ {
+ desc => 'volname_for_format_lax_PluginMatchingRawFormat_Succeeds',
+ class => 'PVE::Storage::Plugin',
+ name => 'vm-100-disk-0.raw',
+ fmt => 'raw',
+ strict => 0,
+ expect_pass => 1,
+ },
+ {
+ desc =>
+ 'volname_for_format_lax_PluginMismatchedRawNameWithQcow2Format_ThrowsSuggestionError',
+ class => 'PVE::Storage::Plugin',
+ name => 'vm-100-disk-0.raw',
+ fmt => 'qcow2',
+ strict => 0,
+ expect_suggestion => 'vm-100-disk-0.qcow2',
+ },
+ {
+ desc => 'volname_for_format_lax_PluginExtensionlessNameWithQcow2Format_AdaptsName',
+ class => 'PVE::Storage::Plugin',
+ name => 'vm-100-cloudinit',
+ fmt => 'qcow2',
+ strict => 0,
+ expect_return => 'vm-100-cloudinit.qcow2',
+ },
+ # ======================================================================
+ # PVE::Storage::LVMPlugin
+ # Raw volumes have no extension, so a raw-parsed name does not spell out a
+ # format and lax mode adapts it. A .qcow2 name does spell out a format and
+ # stays an error in both modes.
+ # ======================================================================
+
+ # --- strict ---
+ {
+ desc => 'volname_for_format_strict_LvmMatchingRawFormat_Succeeds',
+ class => 'PVE::Storage::LVMPlugin',
+ name => 'vm-100-disk-0',
+ fmt => 'raw',
+ strict => 1,
+ expect_pass => 1,
+ },
+ {
+ desc => 'volname_for_format_strict_LvmMatchingQcow2Format_Succeeds',
+ class => 'PVE::Storage::LVMPlugin',
+ name => 'vm-100-disk-0.qcow2',
+ fmt => 'qcow2',
+ strict => 1,
+ expect_pass => 1,
+ },
+ {
+ desc =>
+ 'volname_for_format_strict_LvmMismatchedRawNameWithQcow2Format_ThrowsSuggestionError',
+ class => 'PVE::Storage::LVMPlugin',
+ name => 'vm-100-disk-0',
+ fmt => 'qcow2',
+ strict => 1,
+ expect_suggestion => 'vm-100-disk-0.qcow2',
+ },
+ {
+ desc =>
+ 'volname_for_format_strict_LvmMismatchedQcow2NameWithRawFormat_ThrowsSuggestionError',
+ class => 'PVE::Storage::LVMPlugin',
+ name => 'vm-100-disk-0.qcow2',
+ fmt => 'raw',
+ strict => 1,
+ expect_suggestion => 'vm-100-disk-0',
+ },
+ {
+ desc => 'volname_for_format_strict_LvmNonFormatSuffix_Succeeds',
+ class => 'PVE::Storage::LVMPlugin',
+ name => 'vm-100-disk-0.disk',
+ fmt => 'raw',
+ strict => 1,
+ expect_pass => 1,
+ },
+ {
+ desc => 'volname_for_format_strict_LvmInvalidName_ThrowsParsingError',
+ class => 'PVE::Storage::LVMPlugin',
+ name => 'invalid-name',
+ fmt => 'raw',
+ strict => 1,
+ expect_error => qr/unable to parse lvm volume name/,
+ },
+ {
+ desc => 'volname_for_format_strict_LvmRawSuffixWithRawFormat_Succeeds',
+ class => 'PVE::Storage::LVMPlugin',
+ name => 'vm-100-disk-0.raw',
+ fmt => 'raw',
+ strict => 1,
+ expect_pass => 1,
+ },
+
+ # --- lax ---
+ {
+ desc => 'volname_for_format_lax_LvmMatchingRawFormat_Succeeds',
+ class => 'PVE::Storage::LVMPlugin',
+ name => 'vm-100-disk-0',
+ fmt => 'raw',
+ strict => 0,
+ expect_pass => 1,
+ },
+ {
+ desc => 'volname_for_format_lax_LvmMatchingQcow2Format_Succeeds',
+ class => 'PVE::Storage::LVMPlugin',
+ name => 'vm-100-disk-0.qcow2',
+ fmt => 'qcow2',
+ strict => 0,
+ expect_pass => 1,
+ },
+ {
+ desc => 'volname_for_format_lax_LvmExtensionlessNameWithQcow2Format_AdaptsName',
+ class => 'PVE::Storage::LVMPlugin',
+ name => 'vm-100-cloudinit',
+ fmt => 'qcow2',
+ strict => 0,
+ expect_return => 'vm-100-cloudinit.qcow2',
+ },
+ {
+ desc =>
+ 'volname_for_format_lax_LvmMismatchedQcow2NameWithRawFormat_ThrowsSuggestionError',
+ class => 'PVE::Storage::LVMPlugin',
+ name => 'vm-100-disk-0.qcow2',
+ fmt => 'raw',
+ strict => 0,
+ expect_suggestion => 'vm-100-disk-0',
+ },
+ {
+ desc => 'volname_for_format_lax_LvmRawSuffixWithRawFormat_Succeeds',
+ class => 'PVE::Storage::LVMPlugin',
+ name => 'vm-100-disk-0.raw',
+ fmt => 'raw',
+ strict => 0,
+ expect_pass => 1,
+ },
+ {
+ # .raw is not a real LVM extension (raw has none), so it is treated as a
+ # stray suffix, stripped, and replaced by the requested extension.
+ desc => 'volname_for_format_lax_LvmRawSuffixWithQcow2Format_AdaptsName',
+ class => 'PVE::Storage::LVMPlugin',
+ name => 'vm-100-disk-0.raw',
+ fmt => 'qcow2',
+ strict => 0,
+ expect_suggestion => 'vm-100-disk-0.qcow2',
+ },
+ {
+ desc => 'volname_for_format_lax_LvmNonFormatSuffixWithRawFormat_Succeeds',
+ class => 'PVE::Storage::LVMPlugin',
+ name => 'vm-100-disk-0.disk',
+ fmt => 'raw',
+ strict => 0,
+ expect_pass => 1,
+ },
+ {
+ desc => 'volname_for_format_lax_LvmNonFormatSuffixWithQcow2Format_AdaptsName',
+ class => 'PVE::Storage::LVMPlugin',
+ name => 'vm-100-disk-0.disk',
+ fmt => 'qcow2',
+ strict => 0,
+ expect_suggestion => 'vm-100-disk-0.qcow2',
+ },
+ {
+ desc => 'volname_for_format_lax_LvmInvalidName_ThrowsParsingError',
+ class => 'PVE::Storage::LVMPlugin',
+ name => 'invalid-name',
+ fmt => 'raw',
+ strict => 0,
+ expect_error => qr/unable to parse lvm volume name/,
+ },
+ {
+ desc => 'volname_for_format_lax_LvmUnsupportedFormat_ThrowsError',
+ class => 'PVE::Storage::LVMPlugin',
+ name => 'vm-100-disk-0',
+ fmt => 'vmdk',
+ strict => 0,
+ expect_error => qr/unsupported format 'vmdk'/,
+ },
+
+ # ======================================================================
+ # PVE::Storage::LvmThinPlugin
+ # ======================================================================
+
+ # --- strict ---
+ {
+ desc => 'volname_for_format_strict_LvmThinMatchingRawFormat_Succeeds',
+ class => 'PVE::Storage::LvmThinPlugin',
+ name => 'vm-100-disk-0',
+ fmt => 'raw',
+ strict => 1,
+ expect_pass => 1,
+ },
+ {
+ desc =>
+ 'volname_for_format_strict_LvmThinMismatchedQcow2NameWithRawFormat_ThrowsSuggestionError',
+ class => 'PVE::Storage::LvmThinPlugin',
+ name => 'vm-100-disk-0.qcow2',
+ fmt => 'raw',
+ strict => 1,
+ expect_suggestion => 'vm-100-disk-0',
+ },
+ {
+ desc => 'volname_for_format_strict_LvmThinRawSuffixWithRawFormat_Succeeds',
+ class => 'PVE::Storage::LvmThinPlugin',
+ name => 'vm-100-disk-0.raw',
+ fmt => 'raw',
+ strict => 1,
+ expect_pass => 1,
+ },
+ {
+ desc => 'volname_for_format_strict_LvmThinNonFormatSuffix_Succeeds',
+ class => 'PVE::Storage::LvmThinPlugin',
+ name => 'vm-100-disk-0.disk',
+ fmt => 'raw',
+ strict => 1,
+ expect_pass => 1,
+ },
+ {
+ desc => 'volname_for_format_strict_LvmThinInvalidName_ThrowsParsingError',
+ class => 'PVE::Storage::LvmThinPlugin',
+ name => 'invalid-name',
+ fmt => 'raw',
+ strict => 1,
+ expect_error => qr/unable to parse lvm volume name/,
+ },
+ # --- lax ---
+ {
+ desc => 'volname_for_format_lax_LvmThinMatchingRawFormat_Succeeds',
+ class => 'PVE::Storage::LvmThinPlugin',
+ name => 'vm-100-disk-0',
+ fmt => 'raw',
+ strict => 0,
+ expect_pass => 1,
+ },
+ {
+ desc =>
+ 'volname_for_format_lax_LvmThinMismatchedQcow2NameWithRawFormat_ThrowsSuggestionError',
+ class => 'PVE::Storage::LvmThinPlugin',
+ name => 'vm-100-disk-0.qcow2',
+ fmt => 'raw',
+ strict => 0,
+ expect_suggestion => 'vm-100-disk-0',
+ },
+ {
+ desc => 'volname_for_format_lax_LvmThinRawSuffixWithRawFormat_Succeeds',
+ class => 'PVE::Storage::LvmThinPlugin',
+ name => 'vm-100-disk-0.raw',
+ fmt => 'raw',
+ strict => 0,
+ expect_pass => 1,
+ },
+ {
+ desc => 'volname_for_format_lax_LvmThinInvalidName_ThrowsParsingError',
+ class => 'PVE::Storage::LvmThinPlugin',
+ name => 'invalid-name',
+ fmt => 'raw',
+ strict => 0,
+ expect_error => qr/unable to parse lvm volume name/,
+ },
+
+ # ======================================================================
+ # PVE::Storage::RBDPlugin
+ # ======================================================================
+
+ # --- strict ---
+ {
+ desc => 'volname_for_format_strict_RbdMatchingRawFormat_Succeeds',
+ class => 'PVE::Storage::RBDPlugin',
+ name => 'vm-100-disk-0',
+ fmt => 'raw',
+ strict => 1,
+ expect_pass => 1,
+ },
+ {
+ desc =>
+ 'volname_for_format_strict_RbdMismatchedQcow2NameWithRawFormat_ThrowsSuggestionError',
+ class => 'PVE::Storage::RBDPlugin',
+ name => 'vm-100-disk-0.qcow2',
+ fmt => 'raw',
+ strict => 1,
+ expect_suggestion => 'vm-100-disk-0',
+ },
+ {
+ desc => 'volname_for_format_strict_RbdRawSuffixWithRawFormat_Succeeds',
+ class => 'PVE::Storage::RBDPlugin',
+ name => 'vm-100-disk-0.raw',
+ fmt => 'raw',
+ strict => 1,
+ expect_pass => 1,
+ },
+ {
+ desc => 'volname_for_format_strict_RbdNonFormatSuffix_Succeeds',
+ class => 'PVE::Storage::RBDPlugin',
+ name => 'vm-100-disk-0.disk',
+ fmt => 'raw',
+ strict => 1,
+ expect_pass => 1,
+ },
+ {
+ desc => 'volname_for_format_strict_RbdInvalidName_ThrowsParsingError',
+ class => 'PVE::Storage::RBDPlugin',
+ name => 'invalid-name',
+ fmt => 'raw',
+ strict => 1,
+ expect_error => qr/unable to parse rbd volume name/,
+ },
+
+ # --- lax ---
+ {
+ desc => 'volname_for_format_lax_RbdMatchingRawFormat_Succeeds',
+ class => 'PVE::Storage::RBDPlugin',
+ name => 'vm-100-disk-0',
+ fmt => 'raw',
+ strict => 0,
+ expect_pass => 1,
+ },
+ {
+ desc =>
+ 'volname_for_format_lax_RbdMismatchedQcow2NameWithRawFormat_ThrowsSuggestionError',
+ class => 'PVE::Storage::RBDPlugin',
+ name => 'vm-100-disk-0.qcow2',
+ fmt => 'raw',
+ strict => 0,
+ expect_suggestion => 'vm-100-disk-0',
+ },
+ {
+ desc => 'volname_for_format_lax_RbdInvalidName_ThrowsParsingError',
+ class => 'PVE::Storage::RBDPlugin',
+ name => 'invalid-name',
+ fmt => 'raw',
+ strict => 0,
+ expect_error => qr/unable to parse rbd volume name/,
+ },
+
+ # ======================================================================
+ # PVE::Storage::ZFSPoolPlugin
+ # The name prefix always spells out the format, so any mismatch is an error
+ # in both modes.
+ # ======================================================================
+
+ # --- strict ---
+ {
+ desc => 'volname_for_format_strict_ZfsPoolMatchingRawFormat_Succeeds',
+ class => 'PVE::Storage::ZFSPoolPlugin',
+ name => 'vm-100-disk-0',
+ fmt => 'raw',
+ strict => 1,
+ expect_pass => 1,
+ },
+ {
+ desc => 'volname_for_format_strict_ZfsPoolMatchingSubvolFormat_Succeeds',
+ class => 'PVE::Storage::ZFSPoolPlugin',
+ name => 'subvol-100-disk-0',
+ fmt => 'subvol',
+ strict => 1,
+ expect_pass => 1,
+ },
+ {
+ desc => 'volname_for_format_strict_ZfsPoolMatchingBaseRawFormat_Succeeds',
+ class => 'PVE::Storage::ZFSPoolPlugin',
+ name => 'base-100-disk-0',
+ fmt => 'raw',
+ strict => 1,
+ expect_pass => 1,
+ },
+ {
+ desc => 'volname_for_format_strict_ZfsPoolMatchingBasevolSubvolFormat_Succeeds',
+ class => 'PVE::Storage::ZFSPoolPlugin',
+ name => 'basevol-100-disk-0',
+ fmt => 'subvol',
+ strict => 1,
+ expect_pass => 1,
+ },
+ {
+ desc =>
+ 'volname_for_format_strict_ZfsPoolMismatchedRawNameWithSubvolFormat_ThrowsSuggestionError',
+ class => 'PVE::Storage::ZFSPoolPlugin',
+ name => 'vm-100-disk-0',
+ fmt => 'subvol',
+ strict => 1,
+ expect_suggestion => 'subvol-100-disk-0',
+ },
+ {
+ desc =>
+ 'volname_for_format_strict_ZfsPoolMismatchedSubvolNameWithRawFormat_ThrowsSuggestionError',
+ class => 'PVE::Storage::ZFSPoolPlugin',
+ name => 'subvol-100-disk-0',
+ fmt => 'raw',
+ strict => 1,
+ expect_suggestion => 'vm-100-disk-0',
+ },
+ {
+ desc =>
+ 'volname_for_format_strict_ZfsPoolMismatchedBaseNameWithSubvolFormat_ThrowsSuggestionError',
+ class => 'PVE::Storage::ZFSPoolPlugin',
+ name => 'base-100-disk-0',
+ fmt => 'subvol',
+ strict => 1,
+ expect_suggestion => 'subvol-100-disk-0',
+ },
+ {
+ desc =>
+ 'volname_for_format_strict_ZfsPoolMismatchedBasevolNameWithRawFormat_ThrowsSuggestionError',
+ class => 'PVE::Storage::ZFSPoolPlugin',
+ name => 'basevol-100-disk-0',
+ fmt => 'raw',
+ strict => 1,
+ expect_suggestion => 'vm-100-disk-0',
+ },
+ {
+ desc => 'volname_for_format_strict_ZfsPoolInvalidName_ThrowsParsingError',
+ class => 'PVE::Storage::ZFSPoolPlugin',
+ name => 'invalid-name',
+ fmt => 'raw',
+ strict => 1,
+ expect_error => qr/unable to parse zfs volume name/,
+ },
+
+ # --- lax ---
+ {
+ desc => 'volname_for_format_lax_ZfsPoolMatchingRawFormat_Succeeds',
+ class => 'PVE::Storage::ZFSPoolPlugin',
+ name => 'vm-100-disk-0',
+ fmt => 'raw',
+ strict => 0,
+ expect_pass => 1,
+ },
+ {
+ # The prefix spells out the format, so lax mode still rejects the
+ # mismatch.
+ desc =>
+ 'volname_for_format_lax_ZfsPoolMismatchedRawNameWithSubvolFormat_ThrowsSuggestionError',
+ class => 'PVE::Storage::ZFSPoolPlugin',
+ name => 'vm-100-disk-0',
+ fmt => 'subvol',
+ strict => 0,
+ expect_suggestion => 'subvol-100-disk-0',
+ },
+ {
+ desc =>
+ 'volname_for_format_lax_ZfsPoolMismatchedSubvolNameWithRawFormat_ThrowsSuggestionError',
+ class => 'PVE::Storage::ZFSPoolPlugin',
+ name => 'subvol-100-disk-0',
+ fmt => 'raw',
+ strict => 0,
+ expect_suggestion => 'vm-100-disk-0',
+ },
+
+ # ======================================================================
+ # PVE::Storage::ZFSPlugin
+ # ======================================================================
+
+ # --- strict ---
+ {
+ desc => 'volname_for_format_strict_ZfsMatchingRawFormat_Succeeds',
+ class => 'PVE::Storage::ZFSPlugin',
+ name => 'vm-100-disk-0',
+ fmt => 'raw',
+ strict => 1,
+ expect_pass => 1,
+ },
+ {
+ desc => 'volname_for_format_strict_ZfsMatchingSubvolFormat_Succeeds',
+ class => 'PVE::Storage::ZFSPlugin',
+ name => 'subvol-100-disk-0',
+ fmt => 'subvol',
+ strict => 1,
+ expect_pass => 1,
+ },
+ {
+ desc =>
+ 'volname_for_format_strict_ZfsMismatchedRawNameWithSubvolFormat_ThrowsSuggestionError',
+ class => 'PVE::Storage::ZFSPlugin',
+ name => 'vm-100-disk-0',
+ fmt => 'subvol',
+ strict => 1,
+ expect_suggestion => 'subvol-100-disk-0',
+ },
+
+ # --- lax ---
+ {
+ desc => 'volname_for_format_lax_ZfsMatchingRawFormat_Succeeds',
+ class => 'PVE::Storage::ZFSPlugin',
+ name => 'vm-100-disk-0',
+ fmt => 'raw',
+ strict => 0,
+ expect_pass => 1,
+ },
+ {
+ desc =>
+ 'volname_for_format_lax_ZfsMismatchedRawNameWithSubvolFormat_ThrowsSuggestionError',
+ class => 'PVE::Storage::ZFSPlugin',
+ name => 'vm-100-disk-0',
+ fmt => 'subvol',
+ strict => 0,
+ expect_suggestion => 'subvol-100-disk-0',
+ },
+
+ # ======================================================================
+ # PVE::Storage::BTRFSPlugin
+ # All formats carry an extension, so like the base plugin a mismatch always
+ # spells out a contradicting format.
+ # ======================================================================
+
+ # --- strict ---
+ {
+ desc => 'volname_for_format_strict_BtrfsMatchingRawFormat_Succeeds',
+ class => 'PVE::Storage::BTRFSPlugin',
+ name => 'vm-100-disk-0.raw',
+ fmt => 'raw',
+ strict => 1,
+ expect_pass => 1,
+ },
+ {
+ desc => 'volname_for_format_strict_BtrfsMatchingSubvolFormat_Succeeds',
+ class => 'PVE::Storage::BTRFSPlugin',
+ name => 'vm-100-disk-0.subvol',
+ fmt => 'subvol',
+ strict => 1,
+ expect_pass => 1,
+ },
+ {
+ desc => 'volname_for_format_strict_BtrfsMatchingQcow2Format_Succeeds',
+ class => 'PVE::Storage::BTRFSPlugin',
+ name => 'vm-100-disk-0.qcow2',
+ fmt => 'qcow2',
+ strict => 1,
+ expect_pass => 1,
+ },
+ {
+ desc =>
+ 'volname_for_format_strict_BtrfsMismatchedRawNameWithSubvolFormat_ThrowsSuggestionError',
+ class => 'PVE::Storage::BTRFSPlugin',
+ name => 'vm-100-disk-0.raw',
+ fmt => 'subvol',
+ strict => 1,
+ expect_suggestion => 'vm-100-disk-0.subvol',
+ },
+ {
+ desc =>
+ 'volname_for_format_strict_BtrfsMismatchedSubvolNameWithRawFormat_ThrowsSuggestionError',
+ class => 'PVE::Storage::BTRFSPlugin',
+ name => 'vm-100-disk-0.subvol',
+ fmt => 'raw',
+ strict => 1,
+ expect_suggestion => 'vm-100-disk-0.raw',
+ },
+ {
+ desc =>
+ 'volname_for_format_strict_BtrfsMismatchedQcow2NameWithRawFormat_ThrowsSuggestionError',
+ class => 'PVE::Storage::BTRFSPlugin',
+ name => 'vm-100-disk-0.qcow2',
+ fmt => 'raw',
+ strict => 1,
+ expect_suggestion => 'vm-100-disk-0.raw',
+ },
+ {
+ desc => 'volname_for_format_strict_BtrfsMatchingSubvolPrefixFormat_Succeeds',
+ class => 'PVE::Storage::BTRFSPlugin',
+ name => 'subvol-100-disk-0.subvol',
+ fmt => 'subvol',
+ strict => 1,
+ expect_pass => 1,
+ },
+
+ # --- lax ---
+ {
+ desc => 'volname_for_format_lax_BtrfsMatchingRawFormat_Succeeds',
+ class => 'PVE::Storage::BTRFSPlugin',
+ name => 'vm-100-disk-0.raw',
+ fmt => 'raw',
+ strict => 0,
+ expect_pass => 1,
+ },
+ {
+ desc => 'volname_for_format_lax_BtrfsMatchingSubvolFormat_Succeeds',
+ class => 'PVE::Storage::BTRFSPlugin',
+ name => 'vm-100-disk-0.subvol',
+ fmt => 'subvol',
+ strict => 0,
+ expect_pass => 1,
+ },
+ {
+ desc =>
+ 'volname_for_format_lax_BtrfsMismatchedRawNameWithSubvolFormat_ThrowsSuggestionError',
+ class => 'PVE::Storage::BTRFSPlugin',
+ name => 'vm-100-disk-0.raw',
+ fmt => 'subvol',
+ strict => 0,
+ expect_suggestion => 'vm-100-disk-0.subvol',
+ },
+ {
+ desc => 'volname_for_format_lax_BtrfsExtensionlessNameWithSubvolFormat_AdaptsName',
+ class => 'PVE::Storage::BTRFSPlugin',
+ name => 'vm-100-cloudinit',
+ fmt => 'subvol',
+ strict => 0,
+ expect_return => 'vm-100-cloudinit.subvol',
+ },
+];
+
+sub assert_pass {
+ my ($desc, $result, $err, $expected_name) = @_;
+ if ($err) {
+ fail($desc);
+ diag("Unexpected error: $err");
+ } elsif (!defined($result) || $result ne $expected_name) {
+ fail($desc);
+ diag("Returned '" . ($result // 'undef') . "' but expected '$expected_name'");
+ } else {
+ pass($desc);
+ }
+}
+
+sub assert_return {
+ my ($desc, $result, $err, $expected_return) = @_;
+ if ($err) {
+ fail($desc);
+ diag("Unexpected error: $err");
+ } elsif (!defined($result) || $result ne $expected_return) {
+ fail($desc);
+ diag("Returned '" . ($result // 'undef') . "' but expected '$expected_return'");
+ } else {
+ pass($desc);
+ }
+}
+
+sub assert_suggestion {
+ my ($desc, $name, $fmt, $suggestion, $err) = @_;
+ my $expected_msg = "illegal name $name - volume name does not match requested format "
+ . "'$fmt' (did you mean '$suggestion'?)";
+
+ like($err, qr/\Q$expected_msg\E/, $desc);
+ if (!$err || index($err, $expected_msg) == -1) {
+ diag("Got: " . ($err || 'no error'));
+ }
+}
+
+sub assert_error {
+ my ($desc, $err, $pattern) = @_;
+ like($err, $pattern, $desc);
+ if (!$err || $err !~ $pattern) {
+ diag("Got: " . ($err || 'no error'));
+ }
+}
+
+sub run_case($case) {
+ my $desc = $case->{desc};
+ my $strict = $case->{strict};
+
+ my $result;
+ my $err = '';
+ eval { $result = $case->{class}->volname_for_format($case->{name}, $case->{fmt}, $strict); };
+ $err = $@ if $@;
+
+ if ($case->{expect_pass}) {
+ assert_pass($desc, $result, $err, $case->{name});
+ } elsif (defined($case->{expect_return})) {
+ assert_return($desc, $result, $err, $case->{expect_return});
+ } elsif (defined($case->{expect_suggestion})) {
+ assert_suggestion($desc, $case->{name}, $case->{fmt}, $case->{expect_suggestion}, $err);
+ } elsif (defined($case->{expect_error})) {
+ assert_error($desc, $err, $case->{expect_error});
+ } else {
+ fail($desc);
+ diag('Test case has no expectation set');
+ }
+}
+
+sub main {
+ plan(tests => scalar($tests->@*));
+
+ for my $case ($tests->@*) {
+ run_case($case);
+ }
+
+ done_testing();
+}
+
+main();
+
+1;
--
2.47.3
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-08-12 15:33 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-12 15:32 [PATCH storage 0/3] generalize volname_for_format across storage plugins Elias Huhsovitz
2026-08-12 15:32 ` [PATCH storage 1/3] plugin: refactor format parsing in parse_volname and parse_name_dir Elias Huhsovitz
2026-08-12 15:32 ` [PATCH storage 2/3] plugin: generalize volname_for_format across storage plugins Elias Huhsovitz
2026-08-12 15:32 ` [PATCH storage 3/3] test: plugin: add unit tests for volname_for_format Elias Huhsovitz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox