From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 25B1E1FF183 for ; Wed, 30 Jul 2025 10:35:46 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 03FBF364EA; Wed, 30 Jul 2025 10:37:00 +0200 (CEST) Date: Wed, 30 Jul 2025 10:36:52 +0200 From: Fabian =?iso-8859-1?q?Gr=FCnbichler?= To: Proxmox VE development discussion References: <20250729111557.136012-1-w.bumiller@proxmox.com> <20250729111557.136012-15-w.bumiller@proxmox.com> In-Reply-To: <20250729111557.136012-15-w.bumiller@proxmox.com> MIME-Version: 1.0 User-Agent: astroid/0.16.0 (https://github.com/astroidmail/astroid) Message-Id: <1753862619.nlcvtibp88.astroid@yuna.none> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1753864604991 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.046 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pve-devel] [PATCH storage 14/26] plugins: update image/volume listing to support new types X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox VE development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" On July 29, 2025 1:15 pm, Wolfgang Bumiller wrote: > Signed-off-by: Wolfgang Bumiller > --- > 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 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-(?vm|ct)-|vm)-(?\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-(?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