Re: [PATCH v2 pve-storage 1/16] plugin: add efi-firmware content type

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

Am 28.09.26 um 7:48 AM schrieb Christian Ludwig:
> Holds custom OVMF code images in the efi-firmware/ subdirectory of a
> storage. File names are restricted to the safe character class.
> 
> Signed-off-by: Christian Ludwig <christian_ludwig@genua.de>
> ---
>  src/PVE/Storage/Plugin.pm | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
> index 4f69f9b..f6ca140 100644
> --- a/src/PVE/Storage/Plugin.pm
> +++ b/src/PVE/Storage/Plugin.pm
> @@ -372,7 +372,7 @@ PVE::JSONSchema::register_format('pve-storage-content', \&verify_content);
>  sub verify_content {
>      my ($ct, $noerr) = @_;
>  
> -    return $ct if $ct eq 'import';
> +    return $ct if $ct eq 'import' || $ct eq 'efi-firmware';
>  
>      my $valid_content = valid_content_types('dir'); # dir includes all other types
>  
> @@ -831,6 +831,8 @@ sub parse_volname {
>          m!^import/(${PVE::Storage::SAFE_CHAR_WITH_WHITESPACE_CLASS_RE}+$PVE::Storage::IMPORT_EXT_RE_1)$!

See my reply to the cover letter regarding format extensions.

>      ) {
>          return ('import', $1, undef, undef, undef, undef, $2);
> +    } elsif ($volname =~ m!^efi-firmware/(${PVE::Storage::SAFE_CHAR_CLASS_RE}+)$!) {
> +        return ('efi-firmware', $1, undef, undef, undef, undef, 'raw');
>      }
>  
>      die "unable to parse directory volume name '$volname'\n";
> @@ -844,6 +846,7 @@ my $vtype_subdirs = {
>      backup => 'dump',
>      snippets => 'snippets',
>      import => 'import',
> +    'efi-firmware' => 'efi-firmware',
>  };
>  
>  sub get_vtype_subdirs {
> @@ -1753,6 +1756,10 @@ my $get_subdir_files = sub {
>                  m!/(${PVE::Storage::SAFE_CHAR_CLASS_RE}+$PVE::Storage::IMPORT_EXT_RE_1)$!i;
>  
>              $info = { volid => "$sid:import/$1", format => "$2" };
> +        } elsif ($tt eq 'efi-firmware') {
> +            next if $fn !~ m!/(${PVE::Storage::SAFE_CHAR_CLASS_RE}+)$!i;

Similarly here, if we go with requiring a specific extension.

> +
> +            $info = { volid => "$sid:efi-firmware/$1", format => 'raw' };
>          }
>  
>          $info->{size} = $st->size;
> @@ -1789,6 +1796,8 @@ sub list_volumes {
>                  $data = $get_subdir_files->($storeid, $path, 'snippets');
>              } elsif ($type eq 'import') {
>                  $data = $get_subdir_files->($storeid, $path, 'import');
> +            } elsif ($type eq 'efi-firmware' && !defined($vmid)) {
> +                $data = $get_subdir_files->($storeid, $path, 'efi-firmware');
>              }
>          }
>  

In volume_{export,import}{,_formats}(), the new content type could also
be added to allow moving such images between nodes. Should be a separate
patch.

There's two hunks in patch 4/16 that should already be in this patch,
where the content type is introduced. Please also add handling to
path_to_volume_id() and complete_content_type() in Storage.pm for
completeness. And while there won't be a usage site yet, it seems like
it might also make sense to cover in template_list(), because it behaves
similarly to 'iso'?

Best Regards,
Fiona