[pve-devel] [PATCH storage 1/2] test: bwlimit: fix test description

[pve-devel] [PATCH storage 1/2] test: bwlimit: fix test description

[Fiona Ebner]

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 test/run_bwlimit_tests.pl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/run_bwlimit_tests.pl b/test/run_bwlimit_tests.pl
index 8488637..5e59bf0 100755
--- a/test/run_bwlimit_tests.pl
+++ b/test/run_bwlimit_tests.pl
@@ -188,7 +188,7 @@ my @tests = (
     [ ['move',    ['nolimit', 'd20m40r30'],   undef],     40, 'multiple storages specific limit with privileges on one of them (default limited) (move)' ],
     [ ['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 100 (move)' ],
+    [ ['move',    ['nolimit', undef ],          40] ,     40, 'multiple storages one undefined, passing 40 (move)' ],
 );
 
 foreach my $t (@tests) {
-- 
2.30.2

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

[Fiona Ebner]

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

[pve-devel] applied-series: [PATCH storage 1/2] test: bwlimit: fix test description

[Thomas Lamprecht]

Am 23/11/2022 um 12:40 schrieb Fiona Ebner:
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
>  test/run_bwlimit_tests.pl | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
>

applied both patches, thanks!