[pve-devel] [PATCH pve-storage 1/2] rbd plugin: do not use cache in list_images()

[pve-devel] [PATCH pve-storage 1/2] rbd plugin: do not use cache in list_images()

[Dmitry Petrov via pve-devel]

[-- Attachment #1: Type: message/rfc822, Size: 6848 bytes --]

From: Dmitry Petrov <dpetrov67@gmail.com>
To: pve-devel@lists.proxmox.com
Cc: Dmitry Petrov <dpetrov67@gmail.com>
Subject: [PATCH pve-storage 1/2] rbd plugin: do not use cache in list_images()
Date: Thu,  6 Feb 2025 10:02:36 -0500
Message-ID: <20250206150237.2046776-2-dpetrov67@gmail.com>

A static cache key 'rbd' was used to cache pool/storeid-specific
information. This was causing issues in case of multiple RBD pools.

Drop $cache usage instead of fixing the issue as there is no caller
using it and the same pool/storeid multiple times.

Remove $pool from rbd_ls() and get_rbd_path($scfg) from list_images().

Signed-off-by: Dmitry Petrov <dpetrov67@gmail.com>
---
 src/PVE/Storage/RBDPlugin.pm | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/src/PVE/Storage/RBDPlugin.pm b/src/PVE/Storage/RBDPlugin.pm
index ef4faa6..42eefc6 100644
--- a/src/PVE/Storage/RBDPlugin.pm
+++ b/src/PVE/Storage/RBDPlugin.pm
@@ -209,9 +209,6 @@ sub run_rbd_command {
 sub rbd_ls {
     my ($scfg, $storeid) = @_;
 
-    my $pool =  $scfg->{pool} ? $scfg->{pool} : 'rbd';
-    $pool .= "/$scfg->{namespace}" if defined($scfg->{namespace});
-
     my $raw = '';
     my $parser = sub { $raw .= shift };
 
@@ -237,7 +234,7 @@ sub rbd_ls {
 	my ($owner) = $image =~ m/^(?:vm|base)-(\d+)-/;
 	next if !defined($owner);
 
-	$list->{$pool}->{$image} = {
+	$list->{$image} = {
 	    name => $image,
 	    size => $el->{size},
 	    parent => $get_parent_image_name->($el->{parent}),
@@ -672,9 +669,7 @@ sub free_image {
 sub list_images {
     my ($class, $storeid, $scfg, $vmid, $vollist, $cache) = @_;
 
-    $cache->{rbd} = rbd_ls($scfg, $storeid) if !$cache->{rbd};
-
-    my $dat = $cache->{rbd}->{get_rbd_path($scfg)};
+    my $dat = rbd_ls($scfg, $storeid);
     return [] if !$dat; # nothing found
 
     my $res = [];
-- 
2.39.5



[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

[pve-devel] [PATCH pve-storage 2/2] iscsidirect plugin: do not use cache in list_images()

[Dmitry Petrov via pve-devel]

[-- Attachment #1: Type: message/rfc822, Size: 9487 bytes --]

From: Dmitry Petrov <dpetrov67@gmail.com>
To: pve-devel@lists.proxmox.com
Cc: Dmitry Petrov <dpetrov67@gmail.com>
Subject: [PATCH pve-storage 2/2] iscsidirect plugin: do not use cache in list_images()
Date: Thu,  6 Feb 2025 10:02:37 -0500
Message-ID: <20250206150237.2046776-3-dpetrov67@gmail.com>

A static cache key 'directiscsi' was used to cache storeid-specific
information. This was causing issues in case of multiple instances.

Drop $cache usage instead of fixing the issue as there is no caller
using it and the same portal/storeid multiple times.

Remove $storeid from iscsi_ls() and its callers.

Signed-off-by: Dmitry Petrov <dpetrov67@gmail.com>
---
 src/PVE/Storage/ISCSIDirectPlugin.pm | 71 ++++++++++++----------------
 1 file changed, 30 insertions(+), 41 deletions(-)

diff --git a/src/PVE/Storage/ISCSIDirectPlugin.pm b/src/PVE/Storage/ISCSIDirectPlugin.pm
index 0aca4b6..60bc94e 100644
--- a/src/PVE/Storage/ISCSIDirectPlugin.pm
+++ b/src/PVE/Storage/ISCSIDirectPlugin.pm
@@ -15,7 +15,7 @@ use PVE::JSONSchema qw(get_standard_option);
 use base qw(PVE::Storage::Plugin);
 
 sub iscsi_ls {
-    my ($scfg, $storeid) = @_;
+    my ($scfg) = @_;
 
     my $portal = $scfg->{portal};
     my $cmd = ['/usr/bin/iscsi-ls', '-s', 'iscsi://'.$portal ];
@@ -27,29 +27,27 @@ sub iscsi_ls {
        "T"   => 1024*1024*1024*1024
     );
     eval {
-
 	    run_command($cmd, errmsg => "iscsi error", errfunc => sub {}, outfunc => sub {
-	        my $line = shift;
-	        $line = trim($line);
-	        if( $line =~ /Lun:(\d+)\s+([A-Za-z0-9\-\_\.\:]*)\s+\(Size:([0-9\.]*)(k|M|G|T)\)/ ) {
-	            my $image = "lun".$1;
-	            my $size = $3;
-	            my $unit = $4;
-				
-	            $list->{$storeid}->{$image} = {
-	                name => $image,
-	                size => $size * $unittobytes{$unit},
+		my $line = shift;
+		$line = trim($line);
+		if( $line =~ /Lun:(\d+)\s+([A-Za-z0-9\-\_\.\:]*)\s+\(Size:([0-9\.]*)(k|M|G|T)\)/ ) {
+		    my $image = "lun".$1;
+		    my $size = $3;
+		    my $unit = $4;
+
+		    $list->{$image} = {
+			name => $image,
+			size => $size * $unittobytes{$unit},
 			format => 'raw',
-	            };
-	        }
+		    };
+		}
 	    });
     };
 
     my $err = $@;
-    die $err if $err && $err !~ m/TESTUNITREADY failed with SENSE KEY/ ;
+    die $err if $err && $err !~ m/TESTUNITREADY failed with SENSE KEY/;
 
     return $list;
-
 }
 
 # Configuration
@@ -130,43 +128,34 @@ sub free_image {
     die "can't free space in iscsi storage\n";
 }
 
-
 sub list_images {
     my ($class, $storeid, $scfg, $vmid, $vollist, $cache) = @_;
 
     my $res = [];
 
-    $cache->{directiscsi} = iscsi_ls($scfg,$storeid) if !$cache->{directiscsi};
-
     # we have no owner for iscsi devices
 
-    my $target = $scfg->{target};
-
-    if (my $dat = $cache->{directiscsi}->{$storeid}) {
-
-        foreach my $volname (keys %$dat) {
-
-            my $volid = "$storeid:$volname";
+    my $dat = iscsi_ls($scfg);
+    foreach my $volname (keys %$dat) {
+	my $volid = "$storeid:$volname";
 
-            if ($vollist) {
-                my $found = grep { $_ eq $volid } @$vollist;
-                next if !$found;
-            } else {
-                # we have no owner for iscsi devices
-                next if defined($vmid);
-            }
+	if ($vollist) {
+	    my $found = grep { $_ eq $volid } @$vollist;
+	    next if !$found;
+	} else {
+	    # we have no owner for iscsi devices
+	    next if defined($vmid);
+	}
 
-            my $info = $dat->{$volname};
-            $info->{volid} = $volid;
+	my $info = $dat->{$volname};
+	$info->{volid} = $volid;
 
-            push @$res, $info;
-        }
+	push @$res, $info;
     }
 
     return $res;
 }
 
-
 sub status {
     my ($class, $storeid, $scfg, $cache) = @_;
 
@@ -208,8 +197,8 @@ sub deactivate_volume {
 sub volume_size_info {
     my ($class, $scfg, $storeid, $volname, $timeout) = @_;
 
-    my $vollist = iscsi_ls($scfg,$storeid);
-    my $info = $vollist->{$storeid}->{$volname};
+    my $vollist = iscsi_ls($scfg);
+    my $info = $vollist->{$volname};
 
     return wantarray ? ($info->{size}, 'raw', 0, undef) : $info->{size};
 }
@@ -236,7 +225,7 @@ sub volume_snapshot_delete {
 
 sub volume_has_feature {
     my ($class, $scfg, $feature, $storeid, $volname, $snapname, $running) = @_;
-    
+
     my $features = {
 	copy => { current => 1},
     };
-- 
2.39.5



[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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