From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 4BCFE1FF185 for ; Mon, 21 Jul 2025 14:11:32 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 64984D1D7; Mon, 21 Jul 2025 14:12:05 +0200 (CEST) From: Fiona Ebner To: pve-devel@lists.proxmox.com Date: Mon, 21 Jul 2025 14:10:47 +0200 Message-ID: <20250721121124.77526-3-f.ebner@proxmox.com> X-Mailer: git-send-email 2.47.2 In-Reply-To: <20250721121124.77526-1-f.ebner@proxmox.com> References: <20250721121124.77526-1-f.ebner@proxmox.com> MIME-Version: 1.0 X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1753099880871 X-SPAM-LEVEL: Spam detection results: 0 AWL -0.027 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: [pve-devel] [PATCH storage 2/9] plugin: add get_formats() method and use it instead of default_format() X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox VE development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" The LVM plugin can only use qcow2 format when the 'snapshot-as-volume-chain' configuration option is set. The format information is currently only recorded statically in the plugin data. This causes issues, for example, restoring a guest volume that uses qcow2 as a format hint on an LVM storage without the option set will fail, because the plugin data indicates that qcow2 is supported. Introduce a dedicated method, so that plugins can indicate what actually is supported according to the storage configuration. The implementation for LVM is done in a separate commit. Remove the now unused default_format() function from Plugin.pm. Signed-off-by: Fiona Ebner --- ApiChangeLog | 8 ++++++ src/PVE/Storage.pm | 19 +++++++------ src/PVE/Storage/Plugin.pm | 59 ++++++++++++++++++++++++++------------- 3 files changed, 59 insertions(+), 27 deletions(-) diff --git a/ApiChangeLog b/ApiChangeLog index 0984afb..d80bfb3 100644 --- a/ApiChangeLog +++ b/ApiChangeLog @@ -49,6 +49,14 @@ Future changes should be documented in here. NOTE: Storages must support using "current" as a special name in `rename_snapshot()` to cheaply convert a snapshot into the current disk state and back. +* Introduce `get_formats()` plugin method + + Get information about the supported formats and default format according to the current storage + configuration. The default implemenation is backwards-compatible with previous behavior and looks + at the definition given in the plugin data, as well as the `format` storage configuration option, + which can override the default format. Must be implemented when the supported formats or default + format depend on the storage configuration. + ## Version 11: * Allow declaring storage features via plugin data diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm index 6ca9f88..1faf893 100755 --- a/src/PVE/Storage.pm +++ b/src/PVE/Storage.pm @@ -857,10 +857,11 @@ my $volname_for_storage = sub { my $scfg = storage_config($cfg, $storeid); - my (undef, $valid_formats) = PVE::Storage::Plugin::default_format($scfg); - my $format_is_valid = grep { $_ eq $format } @$valid_formats; + my $plugin = PVE::Storage::Plugin->lookup($scfg->{type}); + + my $formats = $plugin->get_formats($scfg, $storeid); die "unsupported format '$format' for storage type $scfg->{type}\n" - if !$format_is_valid; + if !$formats->{valid}->{$format}; (my $name_without_extension = $name) =~ s/\.$format$//; @@ -1184,14 +1185,12 @@ sub vdisk_alloc { $vmid = parse_vmid($vmid); - my $defformat = PVE::Storage::Plugin::default_format($scfg); + my $plugin = PVE::Storage::Plugin->lookup($scfg->{type}); - $fmt = $defformat if !$fmt; + $fmt = $plugin->get_formats($scfg, $storeid)->{default} if !$fmt; activate_storage($cfg, $storeid); - my $plugin = PVE::Storage::Plugin->lookup($scfg->{type}); - # lock shared storage return $plugin->cluster_lock_storage( $storeid, @@ -1673,8 +1672,12 @@ sub storage_default_format { my ($cfg, $storeid) = @_; my $scfg = storage_config($cfg, $storeid); + my $plugin = PVE::Storage::Plugin->lookup($scfg->{type}); - return PVE::Storage::Plugin::default_format($scfg); + my $formats = $plugin->get_formats($scfg, $storeid); + + return + wantarray ? ($formats->{default}, [sort keys $formats->{valid}->%*]) : $formats->{default}; } sub vgroup_is_used { diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm index ef04cb1..c3c1b63 100644 --- a/src/PVE/Storage/Plugin.pm +++ b/src/PVE/Storage/Plugin.pm @@ -287,23 +287,6 @@ sub storage_has_feature { return; } -sub default_format { - my ($scfg) = @_; - - my $type = $scfg->{type}; - my $def = $defaultData->{plugindata}->{$type}; - - my $def_format = 'raw'; - my $valid_formats = [$def_format]; - - if (defined($def->{format})) { - $def_format = $scfg->{format} || $def->{format}->[1]; - $valid_formats = [sort keys %{ $def->{format}->[0] }]; - } - - return wantarray ? ($def_format, $valid_formats) : $def_format; -} - PVE::JSONSchema::register_format('pve-storage-path', \&verify_path); sub verify_path { @@ -640,6 +623,44 @@ sub preallocation_cmd_opt { # Storage implementation +=pod + +=head3 get_formats + + my $formats = $plugin->get_formats($scfg); + my $default_format = $formats->{default}; + my $is_valid = $formats->{valid}->{$format} ? 1 : 0; + +Get information about the supported formats and default format according to the current storage +configuration C<$scfg>. The return value is a hash reference with C mapping to the default +format and C mapping to a hash reference, where each supported format is present as a key +mapping to C<1>. For example: + + { + default => 'raw', + valid => { + 'qcow2 => 1, + 'raw' => 1, + }, + } + +=cut + +sub get_formats { + my ($class, $scfg, $storeid) = @_; + + my $type = $scfg->{type}; + my $plugin_data = $defaultData->{plugindata}->{$type}; + + return { default => 'raw', valid => { raw => 1 } } if !defined($plugin_data->{format}); + + return { + default => $scfg->{format} || $plugin_data->{format}->[1], + # copy rather than passing direct reference + valid => { $plugin_data->{format}->[0]->%* }, + }; +} + # called during addition of storage (before the new storage config got written) # die to abort addition if there are (grave) problems # NOTE: runs in a storage config *locked* context @@ -1526,8 +1547,8 @@ sub list_images { my $imagedir = $class->get_subdir($scfg, 'images'); - my ($defFmt, $vaidFmts) = default_format($scfg); - my $fmts = join('|', @$vaidFmts); + my $format_info = $class->get_formats($scfg, $storeid); + my $fmts = join('|', sort keys $format_info->{valid}->%*); my $res = []; -- 2.47.2 _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel