On July 29, 2025 1:15 pm, Wolfgang Bumiller wrote:
> Signed-off-by: Wolfgang Bumiller <w.bumil...@proxmox.com>
> ---
> src/PVE/Storage/Common.pm | 31 ++++++++++++++++++++++++++
> src/PVE/Storage/ISCSIDirectPlugin.pm | 5 ++++-
> src/PVE/Storage/ISCSIPlugin.pm | 7 ++++--
> src/PVE/Storage/LVMPlugin.pm | 28 +++++++++++++----------
> src/PVE/Storage/LvmThinPlugin.pm | 33 ++++++++++++++++++----------
> src/PVE/Storage/PBSPlugin.pm | 2 +-
> src/PVE/Storage/RBDPlugin.pm | 9 ++++++--
> src/PVE/Storage/ZFSPoolPlugin.pm | 16 +++++++++++++-
> 8 files changed, 102 insertions(+), 29 deletions(-)
>
> diff --git a/src/PVE/Storage/Common.pm b/src/PVE/Storage/Common.pm
> index b2ebd50..bab1260 100644
> --- a/src/PVE/Storage/Common.pm
> +++ b/src/PVE/Storage/Common.pm
> @@ -282,4 +282,35 @@ sub qemu_img_resize {
> run_command($cmd, timeout => $timeout);
> }
>
> +=head3 should_list_images($expected_content_type, $volume_type)
> +
> +Returns whether a volume of type C<$volume_type> should be listed in
> C<list_images> if the expected
> +content type is C<$expcted_content_type>.
> +
> +This effectively checks if C<$expected_content_type> is a "supertype" of
> C<$volume_type>.
> +
> +=cut
> +
> +sub should_list_images : prototype($$) ($expected_content_type,
> $volume_type) {
> + # If we have no expected type, everything should be listed.
> + return 1 if !defined($expected_content_type);
> +
> + if (!$volume_type) {
> + # For 'images' and 'rootdir', an unknown volume type should also be
> listed.
> + return 1
> + if $expected_content_type eq 'images' || $expected_content_type
> eq 'rootdir';
> + # Otherwise unknown volume types are unexpected.
> + return 0;
> + }
> +
> + # Images should also include vm-vol.
> + return 1 if $expected_content_type eq 'images' && $volume_type eq
> 'vm-vol';
> +
> + # Rootdir should also include ct-vol.
> + return 1 if $expected_content_type eq 'rootdir' && $volume_type eq
> 'ct-vol';
could we drop this if we made pve-container and qemu-server list twice,
once with the legacy and once with the new type? AFAICT the only user of
this is `PVE::Storage::vdisk_list` and list_volumes?
it seems like a potential footgun to have the legacy types refer to just
the legacy types sometimes, but both legacy and new types in other
places..
> +
> + # Otherwise they must be equal.
> + return $expected_content_type eq $volume_type;
> +}
> +
> 1;
> diff --git a/src/PVE/Storage/ISCSIDirectPlugin.pm
> b/src/PVE/Storage/ISCSIDirectPlugin.pm
> index f5b466e..98a3391 100644
> --- a/src/PVE/Storage/ISCSIDirectPlugin.pm
> +++ b/src/PVE/Storage/ISCSIDirectPlugin.pm
> @@ -152,10 +152,12 @@ sub free_image {
> }
>
> sub list_images {
> - my ($class, $storeid, $scfg, $vmid, $vollist, $cache) = @_;
> + my ($class, $storeid, $scfg, $vmid, $vollist, $cache, $content_type) =
> @_;
>
> my $res = [];
>
> + return $res if $content_type && $content_type ne 'images' &&
> $content_type ne 'vm-vol';
> +
> # we have no owner for iscsi devices
>
> my $dat = iscsi_ls($scfg);
> @@ -172,6 +174,7 @@ sub list_images {
>
> my $info = $dat->{$volname};
> $info->{volid} = $volid;
> + $info->{vtype} = 'vm-vol';
>
> push @$res, $info;
> }
> diff --git a/src/PVE/Storage/ISCSIPlugin.pm b/src/PVE/Storage/ISCSIPlugin.pm
> index 4875a1f..77a9173 100644
> --- a/src/PVE/Storage/ISCSIPlugin.pm
> +++ b/src/PVE/Storage/ISCSIPlugin.pm
> @@ -421,17 +421,19 @@ sub list_volumes {
> my $res = $class->list_images($storeid, $scfg, $vmid);
>
> for my $item (@$res) {
> - $item->{content} = 'images'; # we only have images
> + $item->{content} = 'vm-vol'; # we only have VM images
> }
>
> return $res;
> }
>
> sub list_images {
> - my ($class, $storeid, $scfg, $vmid, $vollist, $cache) = @_;
> + my ($class, $storeid, $scfg, $vmid, $vollist, $cache, $content_type) =
> @_;
>
> my $res = [];
>
> + return $res if $content_type && $content_type ne 'images' &&
> $content_type ne 'vm-vol';
> +
> $cache->{iscsi_devices} = iscsi_device_list() if
> !$cache->{iscsi_devices};
>
> # we have no owner for iscsi devices
> @@ -454,6 +456,7 @@ sub list_images {
>
> my $info = $dat->{$volname};
> $info->{volid} = $volid;
> + $info->{vtype} = 'vm-vol';
>
> push @$res, $info;
> }
> diff --git a/src/PVE/Storage/LVMPlugin.pm b/src/PVE/Storage/LVMPlugin.pm
> index 9b88c6a..55c4578 100644
> --- a/src/PVE/Storage/LVMPlugin.pm
> +++ b/src/PVE/Storage/LVMPlugin.pm
> @@ -750,7 +750,7 @@ my $check_tags = sub {
> };
>
> sub list_images {
> - my ($class, $storeid, $scfg, $vmid, $vollist, $cache) = @_;
> + my ($class, $storeid, $scfg, $vmid, $vollist, $cache, $content_type) =
> @_;
>
> my $vgname = $scfg->{vgname};
>
> @@ -762,8 +762,11 @@ sub list_images {
>
> foreach my $volname (keys %$dat) {
>
> - next if $volname !~ m/^vm-(\d+)-/;
> - my $owner = $1;
> + next if $volname !~
> m/^(?:vol-(?<vtype>vm|ct)-|vm)-(?<owner>\d+)-/xn;
> + my $owner = $+{owner};
> + my $vtype = $+{vtype} ? $+{vtype} . '-vol' : undef;
> +
> + next if !PVE::Storage::Common::should_list_images($content_type,
> $vtype);
>
> my $info = $dat->{$volname};
>
> @@ -787,14 +790,17 @@ sub list_images {
> ? $class->volume_size_info($scfg, $storeid, $volname)
> : $info->{lv_size};
>
> - push @$res,
> - {
> - volid => $volid,
> - format => $format,
> - size => $size,
> - vmid => $owner,
> - ctime => $info->{ctime},
> - };
> + my $entry = {
> + volid => $volid,
> + format => $format,
> + size => $size,
> + vmid => $owner,
> + ctime => $info->{ctime},
> + };
> +
> + $entry->{vtype} = $vtype if defined($vtype);
> +
> + push @$res, $entry;
> }
> }
>
> diff --git a/src/PVE/Storage/LvmThinPlugin.pm
> b/src/PVE/Storage/LvmThinPlugin.pm
> index 751bd7b..f6615d9 100644
> --- a/src/PVE/Storage/LvmThinPlugin.pm
> +++ b/src/PVE/Storage/LvmThinPlugin.pm
> @@ -182,7 +182,7 @@ sub free_image {
> }
>
> sub list_images {
> - my ($class, $storeid, $scfg, $vmid, $vollist, $cache) = @_;
> + my ($class, $storeid, $scfg, $vmid, $vollist, $cache, $content_type) =
> @_;
>
> my $vgname = $scfg->{vgname};
>
> @@ -193,9 +193,17 @@ sub list_images {
> if (my $dat = $cache->{lvs}->{$vgname}) {
>
> foreach my $volname (keys %$dat) {
> + my ($owner, $vtype);
> + if ($volname =~ m/^(?:base-)?vol-(ct|vm)-(\d+)-/) {
> + $vtype = "$1-vol";
> + $owner = $2;
> + } elsif ($volname =~ m/^(?:vm|base)-(\d+)-/) {
> + $owner = $1;
> + } else {
> + next;
> + }
>
> - next if $volname !~ m/^(vm|base)-(\d+)-/;
> - my $owner = $2;
> + next if !PVE::Storage::Common::should_list_images($content_type,
> $vtype);
>
> my $info = $dat->{$volname};
>
> @@ -212,14 +220,17 @@ sub list_images {
> next if defined($vmid) && ($owner ne $vmid);
> }
>
> - push @$res,
> - {
> - volid => $volid,
> - format => 'raw',
> - size => $info->{lv_size},
> - vmid => $owner,
> - ctime => $info->{ctime},
> - };
> + my $entry = {
> + volid => $volid,
> + format => 'raw',
> + size => $info->{lv_size},
> + vmid => $owner,
> + ctime => $info->{ctime},
> + };
> +
> + $entry->{vtype} = $vtype if defined($vtype);
> +
> + push @$res, $entry;
> }
> }
>
> diff --git a/src/PVE/Storage/PBSPlugin.pm b/src/PVE/Storage/PBSPlugin.pm
> index 00170f5..bab026b 100644
> --- a/src/PVE/Storage/PBSPlugin.pm
> +++ b/src/PVE/Storage/PBSPlugin.pm
> @@ -674,7 +674,7 @@ sub free_image {
> }
>
> sub list_images {
> - my ($class, $storeid, $scfg, $vmid, $vollist, $cache) = @_;
> + my ($class, $storeid, $scfg, $vmid, $vollist, $cache, $content_type) =
> @_;
>
> my $res = [];
>
> diff --git a/src/PVE/Storage/RBDPlugin.pm b/src/PVE/Storage/RBDPlugin.pm
> index 4400aeb..2ef1280 100644
> --- a/src/PVE/Storage/RBDPlugin.pm
> +++ b/src/PVE/Storage/RBDPlugin.pm
> @@ -773,7 +773,7 @@ sub free_image {
> }
>
> sub list_images {
> - my ($class, $storeid, $scfg, $vmid, $vollist, $cache) = @_;
> + my ($class, $storeid, $scfg, $vmid, $vollist, $cache, $content_type) =
> @_;
>
> my $dat = rbd_ls($scfg, $storeid);
> return [] if !$dat; # nothing found
> @@ -783,12 +783,17 @@ sub list_images {
> my $info = $dat->{$image};
> my ($volname, $parent, $owner) = $info->@{ 'name', 'parent', 'vmid'
> };
>
> - if ($parent && $parent =~ m/^(base-\d+-\S+)\@__base__$/) {
> + my $vtype;
> +
> + if ($parent && $parent =~
> m/^(base(?:-vol-(?<vtype>vm|ct))?-\d+-\S+)\@__base__$/xn) {
> + $vtype = $+{vtype} ? $+{vtype} . '-vol' : undef;
> $info->{volid} = "$storeid:$1/$volname";
> } else {
> $info->{volid} = "$storeid:$volname";
> }
>
> + next if !PVE::Storage::Common::should_list_images($content_type,
> $vtype);
> +
> if ($vollist) {
> my $found = grep { $_ eq $info->{volid} } @$vollist;
> next if !$found;
> diff --git a/src/PVE/Storage/ZFSPoolPlugin.pm
> b/src/PVE/Storage/ZFSPoolPlugin.pm
> index 8e917b4..d65af69 100644
> --- a/src/PVE/Storage/ZFSPoolPlugin.pm
> +++ b/src/PVE/Storage/ZFSPoolPlugin.pm
> @@ -122,6 +122,16 @@ sub zfs_parse_zvol_list {
> return $list;
> }
>
> +my sub image_vtype_from_name : prototype($) {
> + my ($name) = @_;
> +
> + return 'ct-vol' if $name =~ /^(base-)?subvol(-ct)?-/;
> + return 'ct-vol' if $name =~ /^basevol-/;
> + return 'vm-vol' if $name =~ /^(base-)?vol(-vm)?-/;
> + return 'vm-vol' if $name =~ /^base-/;
> + return 'images';
> +}
> +
> sub parse_volname {
> my ($class, $volname) = @_;
>
> @@ -305,7 +315,7 @@ sub free_image {
> }
>
> sub list_images {
> - my ($class, $storeid, $scfg, $vmid, $vollist, $cache) = @_;
> + my ($class, $storeid, $scfg, $vmid, $vollist, $cache, $content_type) =
> @_;
>
> my $zfs_list = $class->zfs_list_zvol($scfg);
>
> @@ -330,6 +340,10 @@ sub list_images {
> next if defined($vmid) && ($owner ne $vmid);
> }
>
> + my $vtype = volume_type_from_name($volname);
> + next if !PVE::Storage::Common::should_list_images($content_type,
> $vtype);
> + $info->{vtype} = $vtype if $vtype ne 'images' && $vtype ne 'rootdir';
> +
> push @$res, $info;
> }
> return $res;
> --
> 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
