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.eb...@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



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

Reply via email to