* [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