add Storage::get_bandwidth_limit helper

Takes an operation, an optional requested bandwidth
limit override, and a list of storages involved in the
operation and lowers the requested bandwidth against global
and storage-specific limits unless the user has permissions
to change those.
This means:
 * Global limits apply to all users without Sys.Modify on /
   (as they can change datacenter.cfg options via the API).
 * Storage specific limits apply to users without
   Datastore.Allocate access on /storage/X for any involved
   storage X.

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
This commit is contained in:
Wolfgang Bumiller
2018-01-30 11:46:19 +01:00
parent d2bf0b4945
commit 9edb99a5a7
15 changed files with 271 additions and 1 deletions

View File

@ -1534,4 +1534,76 @@ sub complete_volume {
return $res;
}
# Various io-heavy operations require io/bandwidth limits which can be
# configured on multiple levels: The global defaults in datacenter.cfg, and
# per-storage overrides. When we want to do a restore from storage A to storage
# B, we should take the smaller limit defined for storages A and B, and if no
# such limit was specified, use the one from datacenter.cfg.
sub get_bandwidth_limit {
my ($operation, $storage_list, $override) = @_;
# called for each limit (global, per-storage) with the 'default' and the
# $operation limit and should udpate $override for every limit affecting
# us.
my $use_global_limits = 0;
my $apply_limit = sub {
my ($bwlimit) = @_;
if (defined($bwlimit)) {
my $limits = PVE::JSONSchema::parse_property_string('bwlimit', $bwlimit);
my $limit = $limits->{$operation} // $limits->{default};
if (defined($limit)) {
if (!$override || $limit < $override) {
$override = $limit;
}
return;
}
}
# If there was no applicable limit, try to apply the global ones.
$use_global_limits = 1;
};
my $rpcenv = PVE::RPCEnvironment->get();
my $authuser = $rpcenv->get_user();
# Apply per-storage limits - if there are storages involved.
if (@$storage_list) {
my $config = config();
# The Datastore.Allocate permission allows us to modify the per-storage
# 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);
my %done;
foreach my $storage (@$storage_list) {
# Avoid duplicate checks:
next if $done{$storage};
$done{$storage} = 1;
# Otherwise we may still have individual /storage/$ID permissions:
if (!$rpcenv->check($authuser, "/storage/$storage", ['Datastore.Allocate'], 1)) {
# And if not: apply the limits.
my $storecfg = storage_config($config, $storage);
$apply_limit->($storecfg->{bwlimit});
}
}
# Storage limits take precedence over the datacenter defaults, so if
# a limit was applied:
return $override if !$use_global_limits;
}
# Sys.Modify on '/' means we can change datacenter.cfg which contains the
# global default limits.
if (!$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');
$apply_limit->($dc->{bwlimit});
}
return $override;
}
1;

View File

@ -45,6 +45,7 @@ sub options {
content => { optional => 1 },
nodes => { optional => 1 },
disable => { optional => 1 },
bwlimit => { optional => 1 },
};
}

View File

@ -42,6 +42,7 @@ sub properties {
type => 'string',
default => 'no',
},
bwlimit => get_standard_option('bwlimit'),
};
}
@ -56,6 +57,7 @@ sub options {
format => { optional => 1 },
mkdir => { optional => 1 },
is_mountpoint => { optional => 1 },
bwlimit => { optional => 1 },
};
}

View File

@ -136,6 +136,7 @@ sub options {
content => { optional => 1 },
format => { optional => 1 },
mkdir => { optional => 1 },
bwlimit => { optional => 1 },
};
}

View File

@ -68,6 +68,7 @@ sub options {
nodes => { optional => 1},
disable => { optional => 1},
content => { optional => 1},
bwlimit => { optional => 1 },
};
}

View File

@ -261,6 +261,7 @@ sub options {
nodes => { optional => 1},
disable => { optional => 1},
content => { optional => 1},
bwlimit => { optional => 1 },
};
}

View File

@ -202,6 +202,7 @@ sub options {
content => { optional => 1 },
base => { fixed => 1, optional => 1 },
tagged_only => { optional => 1 },
bwlimit => { optional => 1 },
};
}

View File

@ -49,6 +49,7 @@ sub options {
nodes => { optional => 1 },
disable => { optional => 1 },
content => { optional => 1 },
bwlimit => { optional => 1 },
};
}

View File

@ -86,6 +86,7 @@ sub options {
content => { optional => 1 },
format => { optional => 1 },
mkdir => { optional => 1 },
bwlimit => { optional => 1 },
};
}

View File

@ -278,6 +278,7 @@ sub options {
username => { optional => 1 },
content => { optional => 1 },
krbd => { optional => 1 },
bwlimit => { optional => 1 },
};
}

View File

@ -115,6 +115,7 @@ sub options {
disable => { optional => 1 },
portal => { fixed => 1 },
content => { optional => 1 },
bwlimit => { optional => 1 },
};
}

View File

@ -205,6 +205,7 @@ sub options {
comstar_hg => { optional => 1 },
comstar_tg => { optional => 1 },
content => { optional => 1 },
bwlimit => { optional => 1 },
};
}

View File

@ -43,6 +43,7 @@ sub options {
nodes => { optional => 1 },
disable => { optional => 1 },
content => { optional => 1 },
bwlimit => { optional => 1 },
};
}

View File

@ -1,9 +1,12 @@
all: test
test: test_zfspoolplugin test_disklist
test: test_zfspoolplugin test_disklist test_bwlimit
test_zfspoolplugin: run_test_zfspoolplugin.pl
./run_test_zfspoolplugin.pl
test_disklist: run_disk_tests.pl
./run_disk_tests.pl
test_bwlimit: run_bwlimit_tests.pl
./run_bwlimit_tests.pl

182
test/run_bwlimit_tests.pl Executable file
View File

@ -0,0 +1,182 @@
#!/usr/bin/perl
use strict;
use warnings;
use Test::MockModule;
use Test::More;
use lib ('.', '..');
use PVE::RPCEnvironment;
use PVE::Cluster;
use PVE::Storage;
my $datacenter_cfg = <<'EOF';
bwlimit: default=100,move=80,restore=60
EOF
my $storage_cfg = <<'EOF';
dir: nolimit
path /dir/a
dir: d50
path /dir/b
bwlimit default=50
dir: d50m40r30
path /dir/c
bwlimit default=50,move=40,restore=30
dir: d20m40r30
path /dir/c
bwlimit default=20,move=40,restore=30
dir: d200m400r300
path /dir/c
bwlimit default=200,move=400,restore=300
dir: d10
path /dir/d
bwlimit default=10
dir: m50
path /dir/e
bwlimit move=50
dir: d200
path /dir/f
bwlimit default=200
EOF
my $permissions = {
'user1@test' => {},
'user2@test' => { '/' => ['Sys.Modify'], },
'user3@test' => { '/storage' => ['Datastore.Allocate'], },
'user4@test' => { '/storage/d20m40r30' => ['Datastore.Allocate'], },
};
my $pve_cluster_module;
$pve_cluster_module = Test::MockModule->new('PVE::Cluster');
$pve_cluster_module->mock(
cfs_update => sub {},
get_config => sub {
my ($file) = @_;
if ($file eq 'datacenter.cfg') {
return $datacenter_cfg;
} elsif ($file eq 'storage.cfg') {
return $storage_cfg;
}
die "TODO: mock get_config($file)\n";
},
);
my $rpcenv_module;
$rpcenv_module = Test::MockModule->new('PVE::RPCEnvironment');
$rpcenv_module->mock(
check => sub {
my ($env, $user, $path, $perms, $noerr) = @_;
return 1 if $user eq 'root@pam';
my $userperms = $permissions->{$user}
or die "no permissions defined for user $user\n";
if (defined(my $pathperms = $userperms->{$path})) {
foreach my $pp (@$pathperms) {
foreach my $reqp (@$perms) {
return 1 if $pp eq $reqp;
}
}
}
die "permission denied\n" if !$noerr;
return 0;
},
);
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)' ],
[ user => 'user1@test' ],
[ ['unknown', ['nolimit'], undef], 100, 'generic default limit' ],
[ ['move', ['nolimit'], undef], 80, 'specific default limit (move)' ],
[ ['restore', ['nolimit'], undef], 60, 'specific default limit (restore)' ],
[ ['unknown', ['d50m40r30'], undef], 50, 'storage default limit' ],
[ ['move', ['d50m40r30'], undef], 40, 'specific storage limit (move)' ],
[ ['restore', ['d50m40r30'], undef], 30, 'specific storage limit (restore)' ],
[ ['unknown', ['d200m400r300'], undef], 200, 'storage default limit above datacenter limits' ],
[ ['move', ['d200m400r300'], undef], 400, 'specific storage limit above datacenter limits (move)' ],
[ ['restore', ['d200m400r300'], undef], 300, 'specific storage limit above datacenter limits (restore)' ],
[ ['unknown', ['d50'], undef], 50, 'storage default limit' ],
[ ['move', ['d50'], undef], 50, 'storage default limit (move)' ],
[ ['restore', ['d50'], undef], 50, 'storage default limit (restore)' ],
[ 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', ['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)' ],
[ user => 'user3@test' ],
[ ['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)' ],
[ user => 'user4@test' ],
[ ['unknown', ['nolimit'], 10], 10, 'generic default limit with privileges on a different storage, passing lower override' ],
[ ['unknown', ['nolimit'], undef], 100, 'generic default limit with privileges on a different storage' ],
[ ['unknown', ['nolimit'], 0], 100, 'generic default limit with privileges on a different storage, passing unlimited' ],
[ ['move', ['nolimit'], undef], 80, 'specific default limit with privileges on a different storage (move)' ],
[ ['restore', ['nolimit'], undef], 60, 'specific default limit with privileges on a different storage (restore)' ],
[ ['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'], 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)' ],
[ ['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)' ],
[ ['restore', ['d10', 'd20m40r30'], 5], 5, 'multiple storages specific limit (storage limited) (restore), passing lower override' ],
[ ['restore', ['d200', 'd200m400r300'], 65], 65, 'multiple storages specific limit (storage limited) (restore), passing lower override' ],
[ ['restore', ['d200', 'd200m400r300'], 400], 200, 'multiple storages specific limit (storage limited) (restore), passing higher override' ],
[ ['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)' ],
);
foreach my $t (@tests) {
my ($args, $expected, $description) = @$t;
if (!ref($args)) {
if ($args eq 'user') {
$rpcenv->set_user($expected);
} else {
die "not a test specification\n";
}
next;
}
is(PVE::Storage::get_bandwidth_limit(@$args), $expected, $description);
}
done_testing();