[pve-devel] [PATCH storage 13/26] plugin, btrfs: update list_images and list_volumes

[Wolfgang Bumiller <w.bumiller@proxmox.com>]

`list_images()` now takes a vtype.

If it is not set, we act like we did previously by listing *all*
images. This now includes the vm-vol and ct-vol types ones.

If a new vtype is set (vm-vol or ct-vol), then we list only those.

For "images" we list both 'vm-vol', for "rootdir" we list both
"ct-vol" and the "rootdir" type.

NOTE: Previously the "rootdir" `content-dirs` option did not take
effect, which is why the `list_images()` implementation for `rootdir`
uses the `images` subdir.
This means that `list_images()` in particular lists all/the same
untyped images regardless of whether `images` or `rootdir` is used as
a type. For `list_volumes()`, the previous strategy of using the
existing VMs to decide the volume-type will be used.

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 src/PVE/Storage/BTRFSPlugin.pm |  44 ++++++++++-
 src/PVE/Storage/Plugin.pm      | 139 +++++++++++++++++++++++++++++----
 2 files changed, 163 insertions(+), 20 deletions(-)

diff --git a/src/PVE/Storage/BTRFSPlugin.pm b/src/PVE/Storage/BTRFSPlugin.pm
index d1c0cf9..585489c 100644
--- a/src/PVE/Storage/BTRFSPlugin.pm
+++ b/src/PVE/Storage/BTRFSPlugin.pm
@@ -645,13 +645,40 @@ sub volume_has_feature {
 }
 
 sub list_images {
-    my ($class, $storeid, $scfg, $vmid, $vollist, $cache) = @_;
-    my $imagedir = $class->get_subdir($scfg, 'images');
+    my ($class, $storeid, $scfg, $vmid, $vollist, $cache, $content_type) = @_;
+
+    my @image_dirs;
+    if (defined($content_type)) {
+        if ($content_type eq 'images') {
+            @image_dirs = (
+                [undef, $class->get_subdir($scfg, 'images')],
+                ['vm-vol', $class->get_subdir($scfg, 'vm-vol')],
+            );
+        } elsif ($content_type eq 'rootdir') {
+            # In the legacy case, the 'rootdir' `content-dir` option did not take
+            # effect, so use the 'images' dir for it, as that is what it used to return!
+            @image_dirs = (
+                [undef, $class->get_subdir($scfg, 'images')],
+                ['ct-vol', $class->get_subdir($scfg, 'ct-vol')],
+            );
+        } else {
+            @image_dirs = [$content_type, $class->get_subdir($scfg, $content_type)];
+        }
+    } else {
+        @image_dirs = (
+            [undef, $class->get_subdir($scfg, 'images')],
+            ['vm-vol', $class->get_subdir($scfg, 'vm-vol')],
+            ['ct-vol', $class->get_subdir($scfg, 'ct-vol')],
+        );
+    }
 
     my $res = [];
 
     # Copied from Plugin.pm, with file_size_info calls adapted:
-    foreach my $fn (<$imagedir/[0-9][0-9]*/*>) {
+    my $current_type;
+    my $code = sub {
+        my ($fn) = @_;
+
         # different to in Plugin.pm the regex below also excludes '@' as valid file name
         next if $fn !~ m@^(/.+/(\d+)/([^/\@.]+(?:\.(qcow2|vmdk|subvol))?))$@;
         $fn = $1; # untaint
@@ -701,9 +728,20 @@ sub list_images {
             parent => $parent,
         };
 
+        # Only add vtype if it is not 'images'...
+        $info->{vtype} = $current_type if defined($current_type);
+
         $info->{ctime} = $ctime if $ctime;
 
         push @$res, $info;
+    };
+
+    my %dedup;
+    for my $dir_entry (@image_dirs) {
+        ($current_type, my $dir) = @$dir_entry;
+        next if $dedup{$dir};
+        $dedup{$dir} = 1;
+        PVE::Storage::Plugin::foreach_guest_file($dir, $code);
     }
 
     return $res;
diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
index 047b2fc..660045d 100644
--- a/src/PVE/Storage/Plugin.pm
+++ b/src/PVE/Storage/Plugin.pm
@@ -10,8 +10,9 @@ use File::chdir;
 use File::Path;
 use File::Basename;
 use File::stat qw();
+use IO::Dir;
 
-use PVE::Tools qw(run_command);
+use PVE::Tools qw(dir_glob_foreach run_command);
 use PVE::JSONSchema qw(get_standard_option register_standard_option);
 use PVE::Cluster qw(cfs_register_file);
 
@@ -1567,36 +1568,91 @@ sub volume_has_feature {
 
     return undef;
 }
+#
+# Given an volume directory, this iterates over vmid directories and recurses
+# once to the files inside.
+#
+# In other words, this is `glob($dir/[0-9][0-9]*/*)`.
+my $MAX_VMID;
+
+sub foreach_guest_file : prototype($$) {
+    my ($dir, $code) = @_;
+
+    $MAX_VMID = get_standard_option("pve-vmid")->{maximum} if !defined($MAX_VMID);
+
+    dir_glob_foreach(
+        $dir,
+        qr/\d+/,
+        sub {
+            my ($vmid) = @_;
+            $vmid = int($vmid);
+            return if $vmid < 100 || $vmid > $MAX_VMID;
+            my $dir = "$dir/$vmid";
+            my $dh = IO::Dir->new($dir) or return;
+            while (defined(my $entry = $dh->read)) {
+                next if $entry eq '.' || $entry eq '..';
+                $code->("$dir/$entry");
+            }
+            close $dh;
+        },
+    );
+}
 
 sub list_images {
-    my ($class, $storeid, $scfg, $vmid, $vollist, $cache) = @_;
-
-    my $imagedir = $class->get_subdir($scfg, 'images');
+    my ($class, $storeid, $scfg, $vmid, $vollist, $cache, $content_type) = @_;
+
+    my @image_dirs;
+    if (defined($content_type)) {
+        if ($content_type eq 'images') {
+            @image_dirs = (
+                [undef, $class->get_subdir($scfg, 'images')],
+                ['vm-vol', $class->get_subdir($scfg, 'vm-vol')],
+            );
+        } elsif ($content_type eq 'rootdir') {
+            # In the legacy case, the 'rootdir' `content-dir` option did not take
+            # effect, so use the 'images' dir for it, as that is what it used to return!
+            @image_dirs = (
+                [undef, $class->get_subdir($scfg, 'images')],
+                ['ct-vol', $class->get_subdir($scfg, 'ct-vol')],
+            );
+        } else {
+            @image_dirs = [$content_type, $class->get_subdir($scfg, $content_type)];
+        }
+    } else {
+        @image_dirs = (
+            [undef, $class->get_subdir($scfg, 'images')],
+            ['vm-vol', $class->get_subdir($scfg, 'vm-vol')],
+            ['ct-vol', $class->get_subdir($scfg, 'ct-vol')],
+        );
+    }
 
     my $format_info = $class->get_formats($scfg, $storeid);
     my $fmts = join('|', sort keys $format_info->{valid}->%*);
 
     my $res = [];
 
-    foreach my $fn (<$imagedir/[0-9][0-9]*/*>) {
+    my $current_type;
+    my $code = sub {
+        my ($fn) = @_;
 
-        next if $fn !~ m!^(/.+/(\d+)/([^/]+\.($fmts)))$!;
+        return if $fn !~ m!^(/.+/(\d+)/([^/]+\.($fmts)))$!;
         $fn = $1; # untaint
 
         my $owner = $2;
         my $name = $3;
         my $format = $4;
 
-        next if !$vollist && defined($vmid) && ($owner ne $vmid);
+        return if !$vollist && defined($vmid) && ($owner ne $vmid);
 
-        my ($size, undef, $used, $parent, $ctime) = eval { file_size_info($fn, undef, $format); };
+        my ($size, undef, $used, $parent, $ctime) =
+            eval { file_size_info($fn, undef, $format); };
         if (my $err = $@) {
             die $err if $err !~ m/Image is not in \S+ format$/;
             warn "image '$fn' is not in expected format '$format', querying as raw\n";
             ($size, undef, $used, $parent, $ctime) = file_size_info($fn, undef, 'raw');
             $format = 'invalid';
         }
-        next if !defined($size);
+        return if !defined($size);
 
         my $volid;
         if ($parent && $parent =~ m!^../(\d+)/([^/]+\.($fmts))$!) {
@@ -1608,7 +1664,7 @@ sub list_images {
 
         if ($vollist) {
             my $found = grep { $_ eq $volid } @$vollist;
-            next if !$found;
+            return if !$found;
         }
 
         my $info = {
@@ -1620,9 +1676,19 @@ sub list_images {
             parent => $parent,
         };
 
+        # Only add vtype if it is not 'images'...
+        $info->{vtype} = $current_type if defined($current_type);
         $info->{ctime} = $ctime if $ctime;
 
         push @$res, $info;
+    };
+
+    my %dedup;
+    for my $dir_entry (@image_dirs) {
+        ($current_type, my $dir) = @$dir_entry;
+        next if $dedup{$dir};
+        $dedup{$dir} = 1;
+        foreach_guest_file($dir, $code);
     }
 
     return $res;
@@ -1709,12 +1775,29 @@ sub list_volumes {
 
     my $res = [];
     my $vmlist = PVE::Cluster::get_vmlist();
+
+    my $guest_dirs = 0;
+    my $DIRS_IMAGES = 1 << 0;
+    my $DIRS_VMS = 1 << 1;
+    my $DIRS_CTS = 1 << 2;
+    my $DIRS_ALL = $DIRS_IMAGES | $DIRS_VMS | $DIRS_CTS;
     foreach my $type (@$content_types) {
         my $data;
+        if ($type eq 'images') {
+            $guest_dirs |= $DIRS_IMAGES | $DIRS_VMS;
+            next;
+        } elsif ($type eq 'rootdir') {
+            $guest_dirs |= $DIRS_IMAGES | $DIRS_CTS;
+            next;
+        } elsif ($type eq 'vm-vol') {
+            $guest_dirs |= $DIRS_VMS;
+            next;
+        } elsif ($type eq 'ct-vol') {
+            $guest_dirs |= $DIRS_CTS;
+            next;
+        }
 
-        if ($type eq 'images' || $type eq 'rootdir') {
-            $data = $class->list_images($storeid, $scfg, $vmid);
-        } elsif ($scfg->{path}) {
+        if ($scfg->{path}) {
             my $path = $class->get_subdir($scfg, $type);
 
             if ($type eq 'iso' && !defined($vmid)) {
@@ -1733,7 +1816,32 @@ sub list_volumes {
         next if !$data;
 
         foreach my $item (@$data) {
-            if ($type eq 'images' || $type eq 'rootdir') {
+            $item->{content} = $type;
+            push @$res, $item;
+        }
+    }
+
+    if ($guest_dirs) {
+        my $data;
+
+        if ($guest_dirs == $DIRS_ALL) {
+            $data = $class->list_images($storeid, $scfg, $vmid, undef, undef, undef);
+        } elsif ($guest_dirs == ($DIRS_IMAGES | $DIRS_VMS)) {
+            $data = $class->list_images($storeid, $scfg, $vmid, undef, undef, 'images');
+        } elsif ($guest_dirs == ($DIRS_IMAGES | $DIRS_CTS)) {
+            $data = $class->list_images($storeid, $scfg, $vmid, undef, undef, 'rootdir');
+        } elsif ($guest_dirs == $DIRS_VMS) {
+            $data = $class->list_images($storeid, $scfg, $vmid, undef, undef, 'vm-vol');
+        } elsif ($guest_dirs == $DIRS_CTS) {
+            $data = $class->list_images($storeid, $scfg, $vmid, undef, undef, 'ct-vol');
+        } else {
+            die "unexpected request to list only untyped images\n";
+        }
+
+        for my $item (@$data) {
+            if (defined(my $vtype = $item->{vtype})) {
+                $item->{content} = $vtype;
+            } else {
                 my $vminfo = $vmlist->{ids}->{ $item->{vmid} };
                 my $vmtype;
                 if (defined($vminfo)) {
@@ -1744,9 +1852,6 @@ sub list_volumes {
                 } else {
                     $item->{content} = 'images';
                 }
-                next if $type ne $item->{content};
-            } else {
-                $item->{content} = $type;
             }
 
             push @$res, $item;
-- 
2.47.2



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