This cleanup improve code reuse, and allows plugins to override list_volumes.

Note: This changes the template_list return value into an array.
---

Changes in v2:

- remove debug statements
- move implementaion to Plugin.pm for max. compatibility with old code
- cleanup regex (use i flag)
- bump APIVER an d APIAGE
- sort result inside volume_list (for all plugins)
- only list supported/enabled content

We need to adopt the template_list call in
- pveam,
- PVE/QemuServer.pm 7316f
- PVE/LXC.pm 1832f


 PVE/Storage.pm                 | 155 ++++-----------------------------
 PVE/Storage/DirPlugin.pm       |   4 +-
 PVE/Storage/Plugin.pm          |  77 ++++++++++++++++
 test/run_test_zfspoolplugin.pl |   2 +-
 4 files changed, 98 insertions(+), 140 deletions(-)

diff --git a/PVE/Storage.pm b/PVE/Storage.pm
index 5925c69..c438374 100755
--- a/PVE/Storage.pm
+++ b/PVE/Storage.pm
@@ -36,11 +36,11 @@ use PVE::Storage::ZFSPlugin;
 use PVE::Storage::DRBDPlugin;
 
 # Storage API version. Icrement it on changes in storage API interface.
-use constant APIVER => 2;
+use constant APIVER => 3;
 # Age is the number of versions we're backward compatible with.
 # This is like having 'current=APIVER' and age='APIAGE' in libtool,
 # see 
https://www.gnu.org/software/libtool/manual/html_node/Libtool-versioning.html
-use constant APIAGE => 1;
+use constant APIAGE => 2;
 
 # load standard plugins
 PVE::Storage::DirPlugin->register();
@@ -769,116 +769,6 @@ sub vdisk_free {
     $rpcenv->fork_worker('imgdel', undef, $authuser, $cleanup_worker);
 }
 
-# lists all files in the snippets directory
-sub snippets_list {
-    my ($cfg, $storeid) = @_;
-
-    my $ids = $cfg->{ids};
-
-    storage_check_enabled($cfg, $storeid) if ($storeid);
-
-    my $res = {};
-
-    foreach my $sid (keys %$ids) {
-       next if $storeid && $storeid ne $sid;
-       next if !storage_check_enabled($cfg, $sid, undef, 1);
-
-       my $scfg = $ids->{$sid};
-       next if !$scfg->{content}->{snippets};
-
-       activate_storage($cfg, $sid);
-
-       if ($scfg->{path}) {
-           my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});
-           my $path = $plugin->get_subdir($scfg, 'snippets');
-
-           foreach my $fn (<$path/*>) {
-               next if -d $fn;
-
-               push @{$res->{$sid}}, {
-                   volid => "$sid:snippets/". basename($fn),
-                   format => 'snippet',
-                   size => -s $fn // 0,
-               };
-           }
-       }
-
-       if ($res->{$sid}) {
-           @{$res->{$sid}} = sort {$a->{volid} cmp $b->{volid} } 
@{$res->{$sid}};
-       }
-    }
-
-    return $res;
-}
-
-#list iso or openvz template ($tt = <iso|vztmpl|backup>)
-sub template_list {
-    my ($cfg, $storeid, $tt) = @_;
-
-    die "unknown template type '$tt'\n"
-       if !($tt eq 'iso' || $tt eq 'vztmpl' || $tt eq 'backup');
-
-    my $ids = $cfg->{ids};
-
-    storage_check_enabled($cfg, $storeid) if ($storeid);
-
-    my $res = {};
-
-    # query the storage
-
-    foreach my $sid (keys %$ids) {
-       next if $storeid && $storeid ne $sid;
-
-       my $scfg = $ids->{$sid};
-       my $type = $scfg->{type};
-
-       next if !storage_check_enabled($cfg, $sid, undef, 1);
-
-       next if $tt eq 'iso' && !$scfg->{content}->{iso};
-       next if $tt eq 'vztmpl' && !$scfg->{content}->{vztmpl};
-       next if $tt eq 'backup' && !$scfg->{content}->{backup};
-
-       activate_storage($cfg, $sid);
-
-       if ($scfg->{path}) {
-           my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});
-
-           my $path = $plugin->get_subdir($scfg, $tt);
-
-           foreach my $fn (<$path/*>) {
-
-               my $info;
-
-               if ($tt eq 'iso') {
-                   next if $fn !~ m!/([^/]+\.[Ii][Ss][Oo])$!;
-
-                   $info = { volid => "$sid:iso/$1", format => 'iso' };
-
-               } elsif ($tt eq 'vztmpl') {
-                   next if $fn !~ m!/([^/]+\.tar\.([gx]z))$!;
-
-                   $info = { volid => "$sid:vztmpl/$1", format => "t$2" };
-
-               } elsif ($tt eq 'backup') {
-                   next if $fn !~ 
m!/([^/]+\.(tar|tar\.gz|tar\.lzo|tgz|vma|vma\.gz|vma\.lzo))$!;
-
-                   $info = { volid => "$sid:backup/$1", format => $2 };
-               }
-
-               $info->{size} = -s $fn // 0;
-
-               push @{$res->{$sid}}, $info;
-           }
-
-       }
-
-       @{$res->{$sid}} = sort {lc($a->{volid}) cmp lc ($b->{volid}) } 
@{$res->{$sid}} if $res->{$sid};
-    }
-
-    return $res;
-}
-
-
 sub vdisk_list {
     my ($cfg, $storeid, $vmid, $vollist) = @_;
 
@@ -923,6 +813,15 @@ sub vdisk_list {
     return $res;
 }
 
+sub template_list {
+    my ($cfg, $storeid, $tt) = @_;
+
+    die "unknown template type '$tt'\n"
+       if !($tt eq 'iso' || $tt eq 'vztmpl' || $tt eq 'backup' || $tt eq 
'snippets');
+
+    return volume_list($cfg, $storeid, undef, $tt);
+}
+
 sub volume_list {
     my ($cfg, $storeid, $vmid, $content) = @_;
 
@@ -932,33 +831,15 @@ sub volume_list {
 
     my $scfg = PVE::Storage::storage_config($cfg, $storeid);
 
-    my $res = [];
-    foreach my $ct (@$cts) {
-       my $data;
-       if ($ct eq 'images') {
-           $data = vdisk_list($cfg, $storeid, $vmid);
-       } elsif ($ct eq 'iso' && !defined($vmid)) {
-           $data = template_list($cfg, $storeid, 'iso');
-       } elsif ($ct eq 'vztmpl'&& !defined($vmid)) {
-           $data = template_list ($cfg, $storeid, 'vztmpl');
-       } elsif ($ct eq 'backup') {
-           $data = template_list ($cfg, $storeid, 'backup');
-           foreach my $item (@{$data->{$storeid}}) {
-               if (defined($vmid)) {
-                   @{$data->{$storeid}} = grep { $_->{volid} =~ 
m/\S+-$vmid-\S+/ } @{$data->{$storeid}};
-               }
-           }
-       } elsif ($ct eq 'snippets') {
-           $data = snippets_list($cfg, $storeid);
-       }
+    $cts = [ grep { defined($scfg->{content}->{$_}) } @$cts ];
+
+    my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});
 
-       next if !$data || !$data->{$storeid};
+    activate_storage($cfg, $storeid);
 
-       foreach my $item (@{$data->{$storeid}}) {
-           $item->{content} = $ct;
-           push @$res, $item;
-       }
-    }
+    my $res = $plugin->list_volumes($storeid, $scfg, $vmid, $cts);
+
+    @$res = sort {lc($a->{volid}) cmp lc ($b->{volid}) } @$res;
 
     return $res;
 }
diff --git a/PVE/Storage/DirPlugin.pm b/PVE/Storage/DirPlugin.pm
index e24ee09..39760a8 100644
--- a/PVE/Storage/DirPlugin.pm
+++ b/PVE/Storage/DirPlugin.pm
@@ -21,7 +21,7 @@ sub plugindata {
                     { images => 1,  rootdir => 1 }],
        format => [ { raw => 1, qcow2 => 1, vmdk => 1, subvol => 1 } , 'raw' ],
     };
-}   
+}
 
 sub properties {
     return {
@@ -114,7 +114,7 @@ sub activate_storage {
            "directory is expected to be a mount point but is not mounted: 
'$mp'\n";
     }
 
-    $class->SUPER::activate_storage($storeid, $scfg, $cache);    
+    $class->SUPER::activate_storage($storeid, $scfg, $cache);
 }
 
 sub check_config {
diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
index 842b4d2..eb2d86a 100644
--- a/PVE/Storage/Plugin.pm
+++ b/PVE/Storage/Plugin.pm
@@ -902,6 +902,83 @@ sub list_images {
     return $res;
 }
 
+# list templates ($tt = <iso|vztmpl|backup|snippets>)
+my $template_list = sub {
+    my ($sid, $path, $tt, $vmid) = @_;
+
+    my $res = [];
+
+    foreach my $fn (<$path/*>) {
+
+       next if -d $fn;
+
+       my $info;
+
+       if ($tt eq 'iso') {
+           next if $fn !~ m!/([^/]+\.iso)$!i;
+
+           $info = { volid => "$sid:iso/$1", format => 'iso' };
+
+       } elsif ($tt eq 'vztmpl') {
+           next if $fn !~ m!/([^/]+\.tar\.([gx]z))$!;
+
+           $info = { volid => "$sid:vztmpl/$1", format => "t$2" };
+
+       } elsif ($tt eq 'backup') {
+           next if $fn !~ 
m!/([^/]+\.(tar|tar\.gz|tar\.lzo|tgz|vma|vma\.gz|vma\.lzo))$!;
+           next if defined($vmid) && $fn !~  m/\S+-$vmid-\S+/;
+
+           $info = { volid => "$sid:backup/$1", format => $2 };
+
+       } elsif ($tt eq 'snippets') {
+
+           $info = {
+               volid => "$sid:snippets/". basename($fn),
+               format => 'snippet',
+           };
+       }
+
+       $info->{size} = -s $fn // 0;
+
+       push @$res, $info;
+    }
+
+    return $res;
+};
+
+sub list_volumes {
+    my ($class, $storeid, $scfg, $vmid, $content_types) = @_;
+
+    my $res = [];
+
+    foreach my $ct (@$content_types) {
+       my $data;
+
+       my $path = $class->get_subdir($scfg, $ct);
+
+       if ($ct eq 'images') {
+           $data = $class->list_images($storeid, $scfg, $vmid);
+       } elsif ($ct eq 'iso' && !defined($vmid)) {
+           $data = $template_list->($storeid, $path, 'iso');
+       } elsif ($ct eq 'vztmpl'&& !defined($vmid)) {
+           $data = $template_list->($storeid, $path, 'vztmpl');
+       } elsif ($ct eq 'backup') {
+           $data = $template_list->($storeid, $path, 'backup', $vmid);
+       } elsif ($ct eq 'snippets') {
+           $data = $template_list->($storeid, $path, 'snippets');
+       }
+
+       next if !$data;
+
+       foreach my $item (@$data) {
+           $item->{content} = $ct;
+           push @$res, $item;
+       }
+    }
+
+    return $res;
+}
+
 sub status {
     my ($class, $storeid, $scfg, $cache) = @_;
 
diff --git a/test/run_test_zfspoolplugin.pl b/test/run_test_zfspoolplugin.pl
index 63b1456..2058508 100755
--- a/test/run_test_zfspoolplugin.pl
+++ b/test/run_test_zfspoolplugin.pl
@@ -309,7 +309,7 @@ my $test15 = sub {
 
     print "\nrun test15 \"template_list and vdisk_list\"\n";
 
-    my $hash = Dumper {};
+    my $hash = Dumper [];
 
     my $res = Dumper PVE::Storage::template_list($cfg, $storagename, "vztmpl");
     if ( $hash ne $res ) {
-- 
2.20.1

_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Reply via email to