[pve-devel] [RFC storage] Find iso and vztmpl in subdirs
Dominic Jäger
d.jaeger at proxmox.com
Mon Dec 2 12:45:55 CET 2019
Each content type has a predefined location in a storage where they can be found.
iso and vztmpl files can now be uploaded and found in subdirectories of those
locations, too.
There are some additional tests that show what some functions of Plugin.pm do.
They test for example #2467 (duplicate volumes).
Signed-off-by: Dominic Jäger <d.jaeger at proxmox.com>
---
Thanks to Stefan for the feedback about the tests.
get_file_properties is a mix of parse_volname and what was in get_subdir_files.
We could probably change parse_volname to return subdirectories and replace
get_file_properties with it or (as it was in get_subdir_files) put it back into
get_subdir_files_rec.
In the API content list call we see a volid, that is
storage:vtype/volname
For example
my-storage:iso/file.iso
As volname could not have a slash / it was clear that it separates vtype and
volname. However, the volname can now contain those / which leads to
my-storage:iso/some/sub/directories/file.iso
IMO it's a bit confusing that "iso" is a vtype here but also part of the
path
storage_root/template/iso/some/sub/directories/file.iso
but unfortunately I haven't had a better idea yet.
PVE/API2/Storage/Status.pm | 10 +-
PVE/Storage/Plugin.pm | 86 +++++++++++++----
test/Makefile | 5 +-
test/run_path_tests.pl | 190 +++++++++++++++++++++++++++++++++++++
4 files changed, 266 insertions(+), 25 deletions(-)
create mode 100755 test/run_path_tests.pl
diff --git a/PVE/API2/Storage/Status.pm b/PVE/API2/Storage/Status.pm
index 1a856fb..dccdcd7 100644
--- a/PVE/API2/Storage/Status.pm
+++ b/PVE/API2/Storage/Status.pm
@@ -403,25 +403,23 @@ __PACKAGE__->register_method ({
my $filename = $param->{filename};
chomp $filename;
- $filename =~ s/^.*[\/\\]//;
- $filename =~ s/[^-a-zA-Z0-9_.]/_/g;
+ $filename =~ s/^[\/|\\]*//; # files are uploaded relative to storage path
+ $filename =~ s/[^-a-zA-Z0-9_.\/\\]/_/g;
my $path;
-
if ($content eq 'iso') {
- if ($filename !~ m![^/]+$PVE::Storage::iso_extension_re$!) {
+ if ($filename !~ m!$PVE::Storage::iso_extension_re$!) {
raise_param_exc({ filename => "missing '.iso' or '.img' extension" });
}
$path = PVE::Storage::get_iso_dir($cfg, $param->{storage});
} elsif ($content eq 'vztmpl') {
- if ($filename !~ m![^/]+\.tar\.[gx]z$!) {
+ if ($filename !~ m!\.tar\.[gx]z$!) {
raise_param_exc({ filename => "missing '.tar.gz' or '.tar.xz' extension" });
}
$path = PVE::Storage::get_vztmpl_dir($cfg, $param->{storage});
} else {
raise_param_exc({ content => "upload content type '$content' not allowed" });
}
-
die "storage '$param->{storage}' does not support '$content' content\n"
if !$scfg->{content}->{$content};
diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
index d0f1df6..885b08f 100644
--- a/PVE/Storage/Plugin.pm
+++ b/PVE/Storage/Plugin.pm
@@ -404,6 +404,9 @@ sub parse_name_dir {
die "unable to parse volume filename '$name'\n";
}
+# for iso and vztmpl returns
+# [0] format
+# [1] notdir (=basename+suffix; no directories)
sub parse_volname {
my ($class, $volname) = @_;
@@ -417,9 +420,9 @@ sub parse_volname {
my ($vmid, $name) = ($1, $2);
my (undef, $format, $isBase) = parse_name_dir($name);
return ('images', $name, $vmid, undef, undef, $isBase, $format);
- } elsif ($volname =~ m!^iso/([^/]+$PVE::Storage::iso_extension_re)$!) {
+ } elsif ($volname =~ m!^iso/(?:.+/)*([^/]+$PVE::Storage::iso_extension_re)$!) {
return ('iso', $1);
- } elsif ($volname =~ m!^vztmpl/([^/]+\.tar\.[gx]z)$!) {
+ } elsif ($volname =~ m!^vztmpl/(?:.+/)*([^/]+\.tar\.[gx]z)$!) {
return ('vztmpl', $1);
} elsif ($volname =~ m!^rootdir/(\d+)$!) {
return ('rootdir', $1, $1);
@@ -474,7 +477,14 @@ sub filesystem_path {
$dir .= "/$vmid" if $vtype eq 'images';
- my $path = "$dir/$name";
+ my $path;
+ if ($vtype eq 'iso' || $vtype eq 'vztmpl') {
+ # remove vtype => file name relative to storage path
+ $volname =~ m!^$vtype/(.+)!;
+ $path = "$dir/$1";
+ } else {
+ $path = "$dir/$name";
+ }
return wantarray ? ($path, $vmid, $vtype) : $path;
}
@@ -889,9 +899,60 @@ sub list_images {
return $res;
}
-# list templates ($tt = <iso|vztmpl|backup|snippets>)
+
+# retvals if file name fits $tt
+# [0] volname = file name relative to $storage_path = right side of volid
+# [1] format
+# undef otherwise
+sub get_file_properties {
+ my ($fn, $storage_path, $tt) = @_;
+ if ($tt eq 'iso') {
+ if ($fn =~ m!$storage_path/(.+$PVE::Storage::iso_extension_re)$!i) {
+ return ["$tt/$1", $tt];
+ } else {
+ return undef;
+ }
+ } elsif ($tt eq 'vztmpl') {
+ if ($fn =~ m!$storage_path/(.+\.tar\.([gx]z))$!) {
+ return ["$tt/$1", "t$2"];
+ } else {
+ return undef;
+ }
+ } else {
+ die "Invalid parameter tt=$tt";
+ }
+}
+
+# $tt = <iso|vztmpl>
+sub get_subdir_files_rec {
+ my ($sid, $path, $tt, $vmid, $storage_path) = @_;
+ die "Invalid parameter" if not ($tt eq 'iso' || $tt eq 'vztmpl');
+ # Each call needs to know the original storage path
+ if (!defined $storage_path) {
+ $storage_path = $path;
+ }
+
+ my @result = ();
+
+ if (-d $path) {
+ foreach my $fn (<$path/*>) {
+ push @result, get_subdir_files_rec($sid, $fn, $tt, $vmid, $storage_path);
+ }
+ } else {
+ my $properties = get_file_properties($path, $storage_path, $tt);
+ if ($properties) {
+ my $file = { volid => "$sid:$$properties[0]", format => $$properties[1] };
+ $file->{size} = -s $path // 0;
+ push @result, $file;
+ }
+ }
+ return @result;
+}
+
+# list templates ($tt = <backup|snippets>)
my $get_subdir_files = sub {
my ($sid, $path, $tt, $vmid) = @_;
+ die "Invalid parameter" if ($tt eq 'iso' || $tt eq 'vztmpl');
my $res = [];
@@ -900,18 +961,7 @@ my $get_subdir_files = sub {
next if -d $fn;
my $info;
-
- if ($tt eq 'iso') {
- next if $fn !~ m!/([^/]+$PVE::Storage::iso_extension_re)$!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') {
+ if ($tt eq 'backup') {
next if defined($vmid) && $fn !~ m/\S+-$vmid-\S+/;
next if $fn !~ m!/([^/]+\.(tar|tar\.gz|tar\.lzo|tgz|vma|vma\.gz|vma\.lzo))$!;
@@ -947,9 +997,9 @@ sub list_volumes {
my $path = $class->get_subdir($scfg, $type);
if ($type eq 'iso' && !defined($vmid)) {
- $data = $get_subdir_files->($storeid, $path, 'iso');
+ $data = [get_subdir_files_rec($storeid, $path, 'iso')];
} elsif ($type eq 'vztmpl'&& !defined($vmid)) {
- $data = $get_subdir_files->($storeid, $path, 'vztmpl');
+ $data = [get_subdir_files_rec($storeid, $path, 'vztmpl')];
} elsif ($type eq 'backup') {
$data = $get_subdir_files->($storeid, $path, 'backup', $vmid);
} elsif ($type eq 'snippets') {
diff --git a/test/Makefile b/test/Makefile
index 833a597..525acf7 100644
--- a/test/Makefile
+++ b/test/Makefile
@@ -1,6 +1,6 @@
all: test
-test: test_zfspoolplugin test_disklist test_bwlimit
+test: test_zfspoolplugin test_disklist test_bwlimit test_paths
test_zfspoolplugin: run_test_zfspoolplugin.pl
./run_test_zfspoolplugin.pl
@@ -10,3 +10,6 @@ test_disklist: run_disk_tests.pl
test_bwlimit: run_bwlimit_tests.pl
./run_bwlimit_tests.pl
+
+test_paths: run_path_tests.pl
+ ./run_path_tests.pl
diff --git a/test/run_path_tests.pl b/test/run_path_tests.pl
new file mode 100755
index 0000000..638c939
--- /dev/null
+++ b/test/run_path_tests.pl
@@ -0,0 +1,190 @@
+#!/usr/bin/perl
+use strict;
+use warnings;
+
+use lib ('.', '..');
+use Data::Dumper qw(Dumper);
+
+use Test::More tests => 37;
+use Test::MockModule;
+use File::Temp;
+use File::Path qw(make_path);
+use PVE::Storage::Plugin;
+
+my $dir_plugin = 'PVE::Storage::DirPlugin';
+my $nfs_plugin = 'PVE::Storage::NFSPlugin';
+my $basename = "test";
+my $subdirectories = "some/more/subdirectories/";
+
+my $iso_vtype = "iso";
+my $iso_suffix = ".iso";
+my $iso_notdir = "$basename$iso_suffix";
+my $iso_volname = "$iso_vtype/$iso_notdir";
+my $iso_volname_with_subdirs = "$iso_vtype/$subdirectories$iso_notdir";
+my $repeating_iso_subdir = "a/template/iso/b/template/iso/c/template/iso/";
+
+my $vztmpl_vtype = "vztmpl";
+my $vztmpl_suffix = ".tar.gz";
+my $vztmpl_notdir = "$basename$vztmpl_suffix";
+my $vztmpl_volname = "$vztmpl_vtype/$vztmpl_notdir";
+my $vztmpl_volname_with_subdirs = "$vztmpl_vtype/$subdirectories$vztmpl_notdir";
+my $repeating_vztmpl_subdir = "a/template/cache/b/template/cache/c/template/cache/";
+
+my $image_vtype = "images";
+my $vmid = "100";
+my $image_basename = "vm-100-disk-0";
+my $raw_image_format = "raw";
+my $raw_image_notdir = "$image_basename.$raw_image_format";
+my $raw_image_volname = "$vmid/$raw_image_notdir";
+my $qcow_image_format = "qcow2";
+my $qcow_image_notdir = "$image_basename.$qcow_image_format";
+my $qcow_image_volname = "$vmid/$qcow_image_notdir";
+my $vmdk_image_format = "vmdk";
+my $vmdk_image_notdir = "$image_basename.$vmdk_image_format";
+my $vmdk_image_volname = "$vmid/$vmdk_image_notdir";
+{
+ use PVE::Storage;
+ my $vtype_index = 0;
+ my $notdir_index = 1;
+ my $format_index = 6;
+
+ is (($dir_plugin->parse_volname($iso_volname))[$vtype_index],
+ $iso_vtype, "parse_volname: vtype for iso");
+ is (($dir_plugin->parse_volname($iso_volname))[$notdir_index],
+ $iso_notdir, "parse_volname: notdir for iso");
+ is (($dir_plugin->parse_volname($iso_volname_with_subdirs))[$vtype_index],
+ $iso_vtype, "parse_volname: vtype for iso in subdir");
+ is (($dir_plugin->parse_volname($iso_volname_with_subdirs))[$notdir_index],
+ $iso_notdir, "parse_volname: notdir for iso in subdir");
+
+ is (($dir_plugin->parse_volname($vztmpl_volname))[$vtype_index],
+ $vztmpl_vtype, "parse_volname: vtype for vztmpl");
+ is (($dir_plugin->parse_volname($vztmpl_volname))[$notdir_index],
+ $vztmpl_notdir, "parse_volname: notdir for vztmpl");
+ is (($dir_plugin->parse_volname($vztmpl_volname_with_subdirs))[$vtype_index],
+ $vztmpl_vtype, "parse_volname: vtype for vztmpl in subdir");
+ is (($dir_plugin->parse_volname($vztmpl_volname_with_subdirs))[$notdir_index],
+ $vztmpl_notdir, "parse_volname: notdir for vztmpl in subdir");
+
+ is (($dir_plugin->parse_volname($raw_image_volname))[$vtype_index],
+ $image_vtype, "parse_volname: vtype for raw image");
+ is (($dir_plugin->parse_volname($raw_image_volname))[$notdir_index],
+ $raw_image_notdir, "parse_volname: notdir for raw image");
+ is (($dir_plugin->parse_volname($raw_image_volname))[$format_index],
+ $raw_image_format, "parse_volname: format for raw image");
+
+ is (($dir_plugin->parse_volname($qcow_image_volname))[$vtype_index],
+ $image_vtype, "parse_volname: vtype for qcow image");
+ is (($dir_plugin->parse_volname($qcow_image_volname))[$notdir_index],
+ $qcow_image_notdir, "parse_volname: notdir for qcow image");
+ is (($dir_plugin->parse_volname($qcow_image_volname))[$format_index],
+ $qcow_image_format, "parse_volname: format for qcow image");
+
+ is (($dir_plugin->parse_volname($vmdk_image_volname))[$vtype_index],
+ $image_vtype, "parse_volname: vtype for vmdk image");
+ is (($dir_plugin->parse_volname($vmdk_image_volname))[$notdir_index],
+ $vmdk_image_notdir, "parse_volname: notdir for vmdk image");
+ is (($dir_plugin->parse_volname($vmdk_image_volname))[$format_index],
+ $vmdk_image_format, "parse_volname: format for vmdk image");
+}
+{
+ my $scfg = { path => '/some/path' };
+ is ($dir_plugin->get_subdir($scfg, "iso"), "$scfg->{path}/template/iso", "get_subdir for iso" );
+ is ($dir_plugin->get_subdir($scfg, "vztmpl"), "$scfg->{path}/template/cache", "get_subdir for vztmpl");
+ is ($dir_plugin->get_subdir($scfg, "backup"), "$scfg->{path}/dump", "get_subdir for backup");
+ is ($dir_plugin->get_subdir($scfg, "images"), "$scfg->{path}/images", "get_subdir for images");
+ is ($dir_plugin->get_subdir($scfg, "rootdir"), "$scfg->{path}/private", "get_subdir for rootdir");
+}
+{
+ my $scfg = { path => '/some/path' };
+ is ($dir_plugin->filesystem_path($scfg, $iso_volname, undef),
+ "$scfg->{path}/template/$iso_volname",
+ "filesystem_path for iso");
+ is ($dir_plugin->filesystem_path($scfg, $iso_volname_with_subdirs, undef),
+ "$scfg->{path}/template/$iso_volname_with_subdirs",
+ "filesystem_path for iso in subdirs");
+
+ is ($dir_plugin->filesystem_path($scfg, $vztmpl_volname, undef),
+ "$scfg->{path}/template/cache/$vztmpl_notdir",
+ "filesystem_path for vztmpl");
+ is ($dir_plugin->filesystem_path($scfg, $vztmpl_volname_with_subdirs, undef),
+ "$scfg->{path}/template/cache/$subdirectories$vztmpl_notdir",
+ "filesystem_path for vztmpl in subdirs");
+
+ is ($dir_plugin->filesystem_path($scfg, $raw_image_volname, undef),
+ "$scfg->{path}/images/$raw_image_volname",
+ "filesystem_path for raw images");
+ is ($dir_plugin->filesystem_path($scfg, $qcow_image_volname, undef),
+ "$scfg->{path}/images/$qcow_image_volname",
+ "filesystem_path for qcow image");
+ is ($dir_plugin->filesystem_path($scfg, $vmdk_image_volname, undef),
+ "$scfg->{path}/images/$vmdk_image_volname",
+ "filesystem_path for vmdk image");
+}
+
+# @expected_volnames may be unsorted
+# $required_vtype is a reference to an array
+sub test_list_volumes {
+ my ($scfg, $required_vtype, @expected_volnames) = @_;
+ my $storage_id = "sid";
+ my $volumes_ref = $dir_plugin->list_volumes($storage_id, $scfg, undef, $required_vtype);
+ my @received = map {my $i = $_->{volid}; $i =~ s/$storage_id://g; $i } @$volumes_ref;
+ my @x = sort @received;
+ my @y = sort @expected_volnames;
+ ok ( @x ~~ @y, "list_volumes with required vtype [@$required_vtype]") || diag "Expected\n", Dumper (@y), "but received\n",
+ Dumper(@x);
+}
+{
+ my $storage_dir = File::Temp->newdir();
+ my @paths = (
+ "$storage_dir/template/cache",
+ "$storage_dir/template/cache/$subdirectories",
+ "$storage_dir/template/cache/$repeating_iso_subdir",
+ "$storage_dir/template/cache/$repeating_vztmpl_subdir",
+ "$storage_dir/template/iso",
+ "$storage_dir/template/iso/$subdirectories",
+ "$storage_dir/template/iso/$repeating_iso_subdir",
+ "$storage_dir/template/iso/$repeating_vztmpl_subdir",
+ "$storage_dir/images/$vmid");
+
+ foreach my $path (@paths) {
+ make_path($path) || BAIL_OUT "Could not create directory $path!";
+ IO::File->new("$path/$iso_notdir", "w");
+ IO::File->new("$path/$iso_notdir", "w");
+ IO::File->new("$path/$raw_image_notdir", "w");
+ IO::File->new("$path/$qcow_image_notdir", "w");
+ IO::File->new("$path/$vmdk_image_notdir", "w");
+ IO::File->new("$path/$vztmpl_notdir", "w");
+ IO::File->new("$path/garbage", "w");
+ }
+# print "Content of temporary directory is:\n", `tree $storage_dir`;
+ {
+ my $scfg = { path => "$storage_dir" };
+ test_list_volumes($scfg, ["$iso_vtype"], (
+ "$iso_vtype/$iso_notdir",
+ "$iso_vtype/$subdirectories$iso_notdir",
+ "$iso_vtype/$repeating_vztmpl_subdir$iso_notdir",
+ "$iso_vtype/$repeating_iso_subdir$iso_notdir"));
+ test_list_volumes($scfg, [$vztmpl_vtype], (
+ "$vztmpl_vtype/$vztmpl_notdir",
+ "$vztmpl_vtype/$subdirectories$vztmpl_notdir",
+ "$vztmpl_vtype/$repeating_vztmpl_subdir$vztmpl_notdir",
+ "$vztmpl_vtype/$repeating_iso_subdir$vztmpl_notdir"));
+ }
+ {
+ my $scfg = { path => "$storage_dir", type => "dir" };
+ my $pluginMock = Test::MockModule->new('PVE::Cluster');
+ $pluginMock->redefine('get_vmlist' => sub { return undef });
+ my @expected_volnames = ($qcow_image_volname, $raw_image_volname, $vmdk_image_volname);
+ note "Tests for undefined VM type => KVM/Qemu";
+ test_list_volumes($scfg, ["images"], @expected_volnames);
+ test_list_volumes($scfg, ["rootdir"], ());
+ test_list_volumes($scfg, ["images", "rootdir"], @expected_volnames);
+
+ note "Tests for VM type lxc";
+ $pluginMock->redefine('get_vmlist' => sub { return {ids => {$vmid => { type => "lxc" }}}});
+ test_list_volumes($scfg, ["images"], ());
+ test_list_volumes($scfg, ["rootdir"], @expected_volnames);
+ test_list_volumes($scfg, ["images", "rootdir"], @expected_volnames);
+ }
+}
--
2.20.1
More information about the pve-devel
mailing list