[pve-devel] [PATCH storage 2/2] get bandwidth limit: improve detecting if storages are involved

[Fiona Ebner <f.ebner@proxmox.com>]

Previously, calling with e.g. $storage_list = [undef] would lead to an
early return of $override and not consider the limit from
datacenter.cfg.

Refactoring the bandwidth limit handling for migration introduced
calls such as described above, which broke applying the limit from
datacenter.cfg for VM RAM/state migration.

Reported in the community forum:
https://forum.proxmox.com/threads/37920/post-513005

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

We could of course also fix the call sites in AbstractMigrate.pm and
StorageTunnel.pm (or not pass [undef] as a list of storages to the
tunnel in the first place), but supporting [undef] in Storage.pm
directly seemed nicer IMHO, as we already support having some of the
entries in the list undef (just not all yet).

 PVE/Storage.pm            | 2 +-
 test/run_bwlimit_tests.pl | 5 +++++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/PVE/Storage.pm b/PVE/Storage.pm
index c21b85e..89c7116 100755
--- a/PVE/Storage.pm
+++ b/PVE/Storage.pm
@@ -2082,7 +2082,7 @@ sub get_bandwidth_limit {
     }
 
     # Apply per-storage limits - if there are storages involved.
-    if (defined($storage_list) && @$storage_list) {
+    if (defined($storage_list) && grep { defined($_) } $storage_list->@*) {
 	my $config = config();
 
 	# The Datastore.Allocate permission allows us to modify the per-storage
diff --git a/test/run_bwlimit_tests.pl b/test/run_bwlimit_tests.pl
index 5e59bf0..6ae379c 100755
--- a/test/run_bwlimit_tests.pl
+++ b/test/run_bwlimit_tests.pl
@@ -109,6 +109,8 @@ my @tests = (
     [ ['restore', ['d50m40r30'],     0],   0, 'root / specific storage limit (restore)' ],
     [ ['migrate', undef,           100], 100, 'root / undef storage (migrate)' ],
     [ ['migrate', [],              100], 100, 'root / no storage (migrate)' ],
+    [ ['migrate', [undef],       undef], 100, 'root / [undef] storage no override (migrate)' ],
+    [ ['migrate', [undef, undef],  200], 200, 'root / list of undef storages with override (migrate)' ],
 
     [ user => 'user1@test' ],
     [ ['unknown', ['nolimit'],      undef], 100, 'generic default limit' ],
@@ -189,6 +191,9 @@ my @tests = (
     [ ['restore', ['nolimit', 'd20m40r30'],   undef],     30, 'multiple storages specific limit with privileges on one of them (default limited) (restore)' ],
     [ ['restore', ['d20m40r30', 'm50'],         200],     60, 'multiple storages specific limit with privileges on one of them (global default limited) (restore)' ],
     [ ['move',    ['nolimit', undef ],          40] ,     40, 'multiple storages one undefined, passing 40 (move)' ],
+    [ ['move',    undef,                       100] ,     80, 'undef storage, passing 100 (move)' ],
+    [ ['move',    [undef],                     100] ,     80, '[undef] storage, passing 100 (move)' ],
+    [ ['move',    [undef],                   undef] ,     80, '[undef] storage, no override (move)' ],
 );
 
 foreach my $t (@tests) {
-- 
2.30.2