[pve-devel] [PATCH storage 2/9] plugin: add get_formats() method and use it instead of default_format()

[Fiona Ebner <f.ebner@proxmox.com>]

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 <f.ebner@proxmox.com>
---
 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<default> mapping to the default
+format and C<valid> 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