From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id 0F06E1FF0E5 for ; Wed, 12 Aug 2026 17:33:16 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 70E99215C1; Wed, 12 Aug 2026 17:33:02 +0200 (CEST) From: Elias Huhsovitz To: pve-devel@lists.proxmox.com Subject: [PATCH storage 2/3] plugin: generalize volname_for_format across storage plugins Date: Wed, 12 Aug 2026 17:32:51 +0200 Message-ID: <20260812153252.222298-3-e.huhsovitz@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260812153252.222298-1-e.huhsovitz@proxmox.com> References: <20260812153252.222298-1-e.huhsovitz@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1786548761818 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.834 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: BNM7IKWWLMCVOK4QIPOUP2VOIUD7CY2J X-Message-ID-Hash: BNM7IKWWLMCVOK4QIPOUP2VOIUD7CY2J X-MailFrom: e.huhsovitz@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: Elias Huhsovitz X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: 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 --- 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 . + +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