On July 29, 2025 1:15 pm, Wolfgang Bumiller wrote:
> `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.bumil...@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($$) {
should this go into PVE::Storage::Common?
> + 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;
this now means list_images lists less than before - somebody might have
used a very high ID as a placeholder not realizing its outside the "allowed"
range..
not sure it's worth it to have this restriction here..
> + 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
>
>
>
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
