Before, 'undef' was equivalent to unlimited, but '0' is the
"explicitly unlimited" value, so if the user doesn't request
an override, apply limits as if the user was unprivileged
(otherwise there's no way for privileged users to explicitly
ask to not override the configured limits).
Signed-off-by: Wolfgang Bumiller <w.bumil...@proxmox.com>
---
PVE/Storage.pm | 13 ++++----
test/run_bwlimit_tests.pl | 75 ++++++++++++++++++++++++++++++-----------------
2 files changed, 56 insertions(+), 32 deletions(-)
diff --git a/PVE/Storage.pm b/PVE/Storage.pm
index 995ebd3..143ed2e 100755
--- a/PVE/Storage.pm
+++ b/PVE/Storage.pm
@@ -1562,8 +1562,11 @@ sub get_bandwidth_limit {
$use_global_limits = 1;
};
- my $rpcenv = PVE::RPCEnvironment->get();
- my $authuser = $rpcenv->get_user();
+ my ($rpcenv, $authuser);
+ if (defined($override)) {
+ $rpcenv = PVE::RPCEnvironment->get();
+ $authuser = $rpcenv->get_user();
+ }
# Apply per-storage limits - if there are storages involved.
if (@$storage_list) {
@@ -1573,7 +1576,7 @@ sub get_bandwidth_limit {
# limits, therefore it also allows us to override them.
# Since we have most likely multiple storages to check, do a quick
check on
# the general '/storage' path to see if we can skip the checks entirely:
- return $override if $rpcenv->check($authuser, '/storage',
['Datastore.Allocate'], 1);
+ return $override if $rpcenv && $rpcenv->check($authuser, '/storage',
['Datastore.Allocate'], 1);
my %done;
foreach my $storage (@$storage_list) {
@@ -1582,7 +1585,7 @@ sub get_bandwidth_limit {
$done{$storage} = 1;
# Otherwise we may still have individual /storage/$ID permissions:
- if (!$rpcenv->check($authuser, "/storage/$storage",
['Datastore.Allocate'], 1)) {
+ if (!$rpcenv || !$rpcenv->check($authuser, "/storage/$storage",
['Datastore.Allocate'], 1)) {
# And if not: apply the limits.
my $storecfg = storage_config($config, $storage);
$apply_limit->($storecfg->{bwlimit});
@@ -1596,7 +1599,7 @@ sub get_bandwidth_limit {
# Sys.Modify on '/' means we can change datacenter.cfg which contains the
# global default limits.
- if (!$rpcenv->check($authuser, '/', ['Sys.Modify'], 1)) {
+ if (!$rpcenv || !$rpcenv->check($authuser, '/', ['Sys.Modify'], 1)) {
# So if we cannot modify global limits, apply them to our currently
# requested override.
my $dc = cfs_read_file('datacenter.cfg');
diff --git a/test/run_bwlimit_tests.pl b/test/run_bwlimit_tests.pl
index e4f723f..a835dc9 100755
--- a/test/run_bwlimit_tests.pl
+++ b/test/run_bwlimit_tests.pl
@@ -95,12 +95,18 @@ my $rpcenv = PVE::RPCEnvironment->init('pub');
my @tests = (
[ user => 'root@pam' ],
- [ ['unknown', ['nolimit'], undef], undef, 'root / generic default limit'
],
- [ ['move', ['nolimit'], undef], undef, 'root / specific default limit
(move)' ],
- [ ['restore', ['nolimit'], undef], undef, 'root / specific default limit
(restore)' ],
- [ ['unknown', ['d50m40r30'], undef], undef, 'root / storage default limit'
],
- [ ['move', ['d50m40r30'], undef], undef, 'root / specific storage limit
(move)' ],
- [ ['restore', ['d50m40r30'], undef], undef, 'root / specific storage limit
(restore)' ],
+ [ ['unknown', ['nolimit'], undef], 100, 'root / generic default limit,
requesting default' ],
+ [ ['move', ['nolimit'], undef], 80, 'root / specific default limit,
requesting default (move)' ],
+ [ ['restore', ['nolimit'], undef], 60, 'root / specific default limit,
requesting default (restore)' ],
+ [ ['unknown', ['d50m40r30'], undef], 50, 'root / storage default limit' ],
+ [ ['move', ['d50m40r30'], undef], 40, 'root / specific storage limit
(move)' ],
+ [ ['restore', ['d50m40r30'], undef], 30, 'root / specific storage limit
(restore)' ],
+ [ ['unknown', ['nolimit'], 0], 0, 'root / generic default limit' ],
+ [ ['move', ['nolimit'], 0], 0, 'root / specific default limit
(move)' ],
+ [ ['restore', ['nolimit'], 0], 0, 'root / specific default limit
(restore)' ],
+ [ ['unknown', ['d50m40r30'], 0], 0, 'root / storage default limit' ],
+ [ ['move', ['d50m40r30'], 0], 0, 'root / specific storage limit
(move)' ],
+ [ ['restore', ['d50m40r30'], 0], 0, 'root / specific storage limit
(restore)' ],
[ user => 'user1@test' ],
[ ['unknown', ['nolimit'], undef], 100, 'generic default limit' ],
@@ -118,23 +124,30 @@ my @tests = (
[ user => 'user2@test' ],
[ ['unknown', ['nolimit'], 0], 0, 'generic default limit with
Sys.Modify, passing unlimited' ],
- [ ['unknown', ['nolimit'], undef], undef, 'generic default limit with
Sys.Modify' ],
- [ ['restore', ['nolimit'], undef], undef, 'specific default limit with
Sys.Modify (restore)' ],
- [ ['move', ['nolimit'], undef], undef, 'specific default limit with
Sys.Modify (move)' ],
+ [ ['unknown', ['nolimit'], undef], 100, 'generic default limit with
Sys.Modify' ],
+ [ ['move', ['nolimit'], undef], 80, 'specific default limit with
Sys.Modify (move)' ],
+ [ ['restore', ['nolimit'], undef], 60, 'specific default limit with
Sys.Modify (restore)' ],
+ [ ['restore', ['nolimit'], 0], 0, 'specific default limit with
Sys.Modify, passing unlimited (restore)' ],
+ [ ['move', ['nolimit'], 0], 0, 'specific default limit with
Sys.Modify, passing unlimited (move)' ],
[ ['unknown', ['d50m40r30'], undef], 50, 'storage default limit with
Sys.Modify' ],
- [ ['move', ['d50m40r30'], undef], 40, 'specific storage limit with
Sys.Modify (move)' ],
[ ['restore', ['d50m40r30'], undef], 30, 'specific storage limit with
Sys.Modify (restore)' ],
+ [ ['move', ['d50m40r30'], undef], 40, 'specific storage limit with
Sys.Modify (move)' ],
[ user => 'user3@test' ],
+ [ ['unknown', ['nolimit'], undef], 100, 'generic default limit with
privileges on /' ],
[ ['unknown', ['nolimit'], 80], 80, 'generic default limit with
privileges on /, passing an override value' ],
[ ['unknown', ['nolimit'], 0], 0, 'generic default limit with
privileges on /, passing unlimited' ],
- [ ['unknown', ['nolimit'], undef], undef, 'generic default limit with
privileges on /' ],
- [ ['move', ['nolimit'], undef], undef, 'specific default limit with
privileges on / (move)' ],
- [ ['restore', ['nolimit'], undef], undef, 'specific default limit with
privileges on / (restore)' ],
- [ ['unknown', ['d50m20r20'], 0], 0, 'storage default limit with
privileges on /, passing unlimited' ],
- [ ['unknown', ['d50m20r20'], undef], undef, 'storage default limit with
privileges on /' ],
- [ ['move', ['d50m20r20'], undef], undef, 'specific storage limit with
privileges on / (move)' ],
- [ ['restore', ['d50m20r20'], undef], undef, 'specific storage limit with
privileges on / (restore)' ],
+ [ ['move', ['nolimit'], undef], 80, 'specific default limit with
privileges on / (move)' ],
+ [ ['move', ['nolimit'], 0], 0, 'specific default limit with
privileges on /, passing unlimited (move)' ],
+ [ ['restore', ['nolimit'], undef], 60, 'specific default limit with
privileges on / (restore)' ],
+ [ ['restore', ['nolimit'], 0], 0, 'specific default limit with
privileges on /, passing unlimited (restore)' ],
+ [ ['unknown', ['d50m40r30'], 0], 0, 'storage default limit with
privileges on /, passing unlimited' ],
+ [ ['unknown', ['d50m40r30'], undef], 50, 'storage default limit with
privileges on /' ],
+ [ ['unknown', ['d50m40r30'], 0], 0, 'storage default limit with
privileges on, passing unlimited /' ],
+ [ ['move', ['d50m40r30'], undef], 40, 'specific storage limit with
privileges on / (move)' ],
+ [ ['move', ['d50m40r30'], 0], 0, 'specific storage limit with
privileges on, passing unlimited / (move)' ],
+ [ ['restore', ['d50m40r30'], undef], 30, 'specific storage limit with
privileges on / (restore)' ],
+ [ ['restore', ['d50m40r30'], 0], 0, 'specific storage limit with
privileges on /, passing unlimited (restore)' ],
[ user => 'user4@test' ],
[ ['unknown', ['nolimit'], 10], 10, 'generic default
limit with privileges on a different storage, passing lower override' ],
@@ -145,13 +158,18 @@ my @tests = (
[ ['unknown', ['d50m40r30'], undef], 50, 'storage default
limit with privileges on a different storage' ],
[ ['move', ['d50m40r30'], undef], 40, 'specific
storage limit with privileges on a different storage (move)' ],
[ ['restore', ['d50m40r30'], undef], 30, 'specific
storage limit with privileges on a different storage (restore)' ],
- [ ['unknown', ['d20m40r30'], undef], undef, 'storage default
limit with privileges on that storage' ],
+ [ ['unknown', ['d20m40r30'], undef], 20, 'storage default
limit with privileges on that storage' ],
[ ['unknown', ['d20m40r30'], 0], 0, 'storage default
limit with privileges on that storage, passing unlimited' ],
- [ ['move', ['d20m40r30'], undef], undef, 'specific
storage limit with privileges on that storage (move)' ],
- [ ['restore', ['d20m40r30'], undef], undef, 'specific
storage limit with privileges on that storage (restore)' ],
- [ ['unknown', ['d50m40r30', 'd20m40r30'], undef], 50, 'multiple
storages default limit with privileges on one of them' ],
- [ ['move', ['d50m40r30', 'd20m40r30'], undef], 40, 'multiple
storages specific limit with privileges on one of them (move)' ],
- [ ['restore', ['d50m40r30', 'd20m40r30'], undef], 30, 'multiple
storages specific limit with privileges on one of them (restore)' ],
+ [ ['move', ['d20m40r30'], undef], 40, 'specific
storage limit with privileges on that storage (move)' ],
+ [ ['move', ['d20m40r30'], 0], 0, 'specific
storage limit with privileges on that storage, passing unlimited (move)' ],
+ [ ['move', ['d20m40r30'], 10], 10, 'specific
storage limit with privileges on that storage, passing low override (move)' ],
+ [ ['move', ['d20m40r30'], 300], 300, 'specific
storage limit with privileges on that storage, passing high override (move)' ],
+ [ ['restore', ['d20m40r30'], undef], 30, 'specific
storage limit with privileges on that storage (restore)' ],
+ [ ['restore', ['d20m40r30'], 0], 0, 'specific
storage limit with privileges on that storage, passing unlimited (restore)' ],
+ [ ['unknown', ['d50m40r30', 'd20m40r30'], 0], 50, 'multiple
storages default limit with privileges on one of them, passing unlimited' ],
+ [ ['move', ['d50m40r30', 'd20m40r30'], 0], 40, 'multiple
storages specific limit with privileges on one of them, passing unlimited
(move)' ],
+ [ ['restore', ['d50m40r30', 'd20m40r30'], 0], 30, 'multiple
storages specific limit with privileges on one of them, passing unlimited
(restore)' ],
+ [ ['unknown', ['d50m40r30', 'd20m40r30'], undef], 20, 'multiple
storages default limit with privileges on one of them' ],
[ ['unknown', ['d10', 'd20m40r30'], undef], 10, 'multiple
storages default limit with privileges on one of them (storage limited)' ],
[ ['move', ['d10', 'd20m40r30'], undef], 10, 'multiple
storages specific limit with privileges on one of them (storage limited)
(move)' ],
[ ['restore', ['d10', 'd20m40r30'], undef], 10, 'multiple
storages specific limit with privileges on one of them (storage limited)
(restore)' ],
@@ -161,10 +179,13 @@ my @tests = (
[ ['restore', ['d200', 'd200m400r300'], 0], 200, 'multiple
storages specific limit (storage limited) (restore), passing unlimited' ],
[ ['restore', ['d200', 'd200m400r300'], 1], 1, 'multiple
storages specific limit (storage limited) (restore), passing 1' ],
[ ['restore', ['d10', 'd20m40r30'], 500], 10, 'multiple
storages specific limit with privileges on one of them (storage limited)
(restore), passing higher override' ],
- [ ['unknown', ['nolimit', 'd20m40r30'], undef], 100, 'multiple
storages default limit with privileges on one of them (default limited)' ],
- [ ['move', ['nolimit', 'd20m40r30'], undef], 80, 'multiple
storages specific limit with privileges on one of them (default limited)
(move)' ],
- [ ['restore', ['nolimit', 'd20m40r30'], undef], 60, '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 (default limited)
(restore)' ],
+ [ ['unknown', ['nolimit', 'd20m40r30'], 0], 100, 'multiple
storages default limit with privileges on one of them, passing unlimited
(default limited)' ],
+ [ ['move', ['nolimit', 'd20m40r30'], 0], 80, 'multiple
storages specific limit with privileges on one of them, passing unlimited
(default limited) (move)' ],
+ [ ['restore', ['nolimit', 'd20m40r30'], 0], 60, 'multiple
storages specific limit with privileges on one of them, passing unlimited
(default limited) (restore)' ],
+ [ ['unknown', ['nolimit', 'd20m40r30'], undef], 20, 'multiple
storages default limit with privileges on one of them (default limited)' ],
+ [ ['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)' ],
);
foreach my $t (@tests) {
--
2.11.0
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
