public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Elias Huhsovitz <e.huhsovitz@proxmox.com>
To: pve-devel@lists.proxmox.com
Cc: Elias Huhsovitz <e.huhsovitz@proxmox.com>
Subject: [PATCH storage 3/3] test: plugin: add unit tests for volname_for_format
Date: Wed, 12 Aug 2026 17:32:52 +0200	[thread overview]
Message-ID: <20260812153252.222298-4-e.huhsovitz@proxmox.com> (raw)
In-Reply-To: <20260812153252.222298-1-e.huhsovitz@proxmox.com>

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





      parent reply	other threads:[~2026-08-12 15:33 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260812153252.222298-4-e.huhsovitz@proxmox.com \
    --to=e.huhsovitz@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal