[pve-devel] [PATCH v2 qemu-server] restore: implement rate limiting
Dominik Csapak
d.csapak at proxmox.com
Wed Feb 21 13:01:15 CET 2018
one comment inline,
rest looks ok (only tested with qmrestore --bwlimit, not with storage
limits atm)
On 02/15/2018 01:41 PM, Wolfgang Bumiller wrote:
> Signed-off-by: Wolfgang Bumiller <w.bumiller at proxmox.com>
> ---
> This version avoids the refactoring and double-reading of the VMA file,
> thereby supporting reading from a pipe again.
> Instead this now uses an extension to vma extract (see the other patch),
> and instead of picking the lowest limit altogether, applies limits to
> disks separately per-storage by creating a throttling group per storage
> id.
>
> PVE/API2/Qemu.pm | 11 +++++++-
> PVE/CLI/qmrestore.pm | 6 ++++
> PVE/QemuServer.pm | 78 ++++++++++++++++++++++++++++++++++++++++------------
> 3 files changed, 77 insertions(+), 18 deletions(-)
>
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index b277a26..a63f2c9 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -405,6 +405,12 @@ __PACKAGE__->register_method({
> type => 'string', format => 'pve-poolid',
> description => "Add the VM to the specified pool.",
> },
> + bwlimit => {
> + description => "Override io bandwidth.",
> + optional => 1,
> + type => 'number',
integer would be better here i think, since having a sub kb resolution
for this makes not really sense and
if you specify things such as 0.0001
you get errors for cstream:
** (process:30221): ERROR **: read map failed - not a number: 0.1024
sidenote: choosing a very low value (<= 30 in my case) results in a timeout
> + minimum => '0',
> + }
> }),
> },
> returns => {
> @@ -431,6 +437,8 @@ __PACKAGE__->register_method({
>
> my $pool = extract_param($param, 'pool');
>
> + my $bwlimit = extract_param($param, 'bwlimit');
> +
> my $filename = PVE::QemuConfig->config_file($vmid);
>
> my $storecfg = PVE::Storage::config();
> @@ -513,7 +521,8 @@ __PACKAGE__->register_method({
> PVE::QemuServer::restore_archive($archive, $vmid, $authuser, {
> storage => $storage,
> pool => $pool,
> - unique => $unique });
> + unique => $unique,
> + bwlimit => $bwlimit, });
>
> PVE::AccessControl::add_vm_to_pool($vmid, $pool) if $pool;
> };
> diff --git a/PVE/CLI/qmrestore.pm b/PVE/CLI/qmrestore.pm
> index 17018d2..7e12b10 100755
> --- a/PVE/CLI/qmrestore.pm
> +++ b/PVE/CLI/qmrestore.pm
> @@ -53,6 +53,12 @@ __PACKAGE__->register_method({
> type => 'string', format => 'pve-poolid',
> description => "Add the VM to the specified pool.",
> },
> + bwlimit => {
> + description => "Override io bandwidth.",
> + optional => 1,
> + type => 'number',
> + minimum => '0',
> + }
> },
> },
> returns => {
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 20d6682..1607932 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -5539,21 +5539,52 @@ sub rescan {
> sub restore_vma_archive {
> my ($archive, $vmid, $user, $opts, $comp) = @_;
>
> - my $input = $archive eq '-' ? "<&STDIN" : undef;
> my $readfrom = $archive;
>
> - my $uncomp = '';
> - if ($comp) {
> + my $cfg = PVE::Storage::config();
> + my $commands = [];
> + my $dbg_cmdstring = '';
> + my $bwlimit = $opts->{bwlimit};
> +
> + my $add_pipe = sub {
> + my ($cmd, $final) = @_;
> + push @$commands, $cmd;
> + $dbg_cmdstring .= PVE::Tools::cmd2string($cmd);
> + $dbg_cmdstring .= ' | ' if !$final;
> $readfrom = '-';
> - my $qarchive = PVE::Tools::shellquote($archive);
> + };
> +
> + my $input = undef;
> + if ($archive eq '-') {
> + $input = '<&STDIN';
> + } else {
> + # If we use a backup from a PVE defined storage we also consider that
> + # storage's rate limit:
> + my (undef, $volid) = PVE::Storage::path_to_volume_id($cfg, $archive);
> + if (defined($volid)) {
> + my ($sid, undef) = PVE::Storage::parse_volume_id($volid);
> + my $readlimit = PVE::Storage::get_bandwidth_limit('restore', [$sid], $bwlimit);
> + if ($readlimit) {
> + print STDERR "applying read rate limit: $readlimit\n";
> + my $cstream = ['cstream', '-t', $readlimit*1024];
> + if ($readfrom ne '-') {
> + push @$cstream, '--', $readfrom;
> + }
> + $add_pipe->($cstream);
> + }
> + }
> + }
> +
> + if ($comp) {
> + my $cmd;
> if ($comp eq 'gzip') {
> - $uncomp = "zcat $qarchive|";
> + $cmd = ['zcat', $readfrom];
> } elsif ($comp eq 'lzop') {
> - $uncomp = "lzop -d -c $qarchive|";
> + $cmd = ['lzop', '-d', '-c', $readfrom];
> } else {
> die "unknown compression method '$comp'\n";
> }
> -
> + $add_pipe->($cmd);
> }
>
> my $tmpdir = "/var/tmp/vzdumptmp$$";
> @@ -5573,7 +5604,7 @@ sub restore_vma_archive {
> open($fifofh, '>', $mapfifo) || die $!;
> };
>
> - my $cmd = "${uncomp}vma extract -v -r $mapfifo $readfrom $tmpdir";
> + $add_pipe->(['vma', 'extract', '-v', '-r', $mapfifo, $readfrom, $tmpdir]);
>
> my $oldtimeout;
> my $timeout = 5;
> @@ -5589,6 +5620,8 @@ sub restore_vma_archive {
> my $cfs_path = PVE::QemuConfig->cfs_config_path($vmid);
> my $oldconf = PVE::Cluster::cfs_read_file($cfs_path);
>
> + my %storage_limits;
> +
> my $print_devmap = sub {
> my $virtdev_hash = {};
>
> @@ -5627,17 +5660,23 @@ sub restore_vma_archive {
> $rpcenv->check($user, "/storage/$storeid", ['Datastore.AllocateSpace']);
> }
>
> + $storage_limits{$storeid} = $bwlimit;
> +
> $virtdev_hash->{$virtdev} = $devinfo->{$devname};
> }
> }
>
> + foreach my $key (keys %storage_limits) {
> + my $limit = PVE::Storage::get_bandwidth_limit('restore', [$key], $bwlimit);
$limit here can be undef (if no limits are set anywhere) so we have
to check before printing/multiplying
> + print STDERR "rate limit for storage $key: $limit KiB/s\n";
> + $storage_limits{$key} = $limit * 1024;
> + }
> +
> foreach my $devname (keys %$devinfo) {
> die "found no device mapping information for device '$devname'\n"
> if !$devinfo->{$devname}->{virtdev};
> }
>
> - my $cfg = PVE::Storage::config();
> -
> # create empty/temp config
> if ($oldconf) {
> PVE::Tools::file_set_contents($conffile, "memory: 128\n");
> @@ -5680,14 +5719,20 @@ sub restore_vma_archive {
> foreach my $virtdev (sort keys %$virtdev_hash) {
> my $d = $virtdev_hash->{$virtdev};
> my $alloc_size = int(($d->{size} + 1024 - 1)/1024);
> - my $scfg = PVE::Storage::storage_config($cfg, $d->{storeid});
> + my $storeid = $d->{storeid};
> + my $scfg = PVE::Storage::storage_config($cfg, $storeid);
> +
> + my $map_opts = '';
> + if (my $limit = $storage_limits{$storeid}) {
> + $map_opts .= "throttling.bps=$limit:throttling.group=$storeid:";
> + }
>
> # test if requested format is supported
> - my ($defFormat, $validFormats) = PVE::Storage::storage_default_format($cfg, $d->{storeid});
> + my ($defFormat, $validFormats) = PVE::Storage::storage_default_format($cfg, $storeid);
> my $supported = grep { $_ eq $d->{format} } @$validFormats;
> $d->{format} = $defFormat if !$supported;
>
> - my $volid = PVE::Storage::vdisk_alloc($cfg, $d->{storeid}, $vmid,
> + my $volid = PVE::Storage::vdisk_alloc($cfg, $storeid, $vmid,
> $d->{format}, undef, $alloc_size);
> print STDERR "new volume ID is '$volid'\n";
> $d->{volid} = $volid;
> @@ -5700,7 +5745,7 @@ sub restore_vma_archive {
> $write_zeros = 0;
> }
>
> - print $fifofh "format=$d->{format}:${write_zeros}:$d->{devname}=$path\n";
> + print $fifofh "${map_opts}format=$d->{format}:${write_zeros}:$d->{devname}=$path\n";
>
> print "map '$d->{devname}' to '$path' (write zeros = ${write_zeros})\n";
> $map->{$virtdev} = $volid;
> @@ -5753,8 +5798,8 @@ sub restore_vma_archive {
> }
> };
>
> - print "restore vma archive: $cmd\n";
> - run_command($cmd, input => $input, outfunc => $parser, afterfork => $openfifo);
> + print "restore vma archive: $dbg_cmdstring\n";
> + run_command($commands, input => $input, outfunc => $parser, afterfork => $openfifo);
> };
> my $err = $@;
>
> @@ -5766,7 +5811,6 @@ sub restore_vma_archive {
> push @$vollist, $volid if $volid;
> }
>
> - my $cfg = PVE::Storage::config();
> PVE::Storage::deactivate_volumes($cfg, $vollist);
>
> unlink $mapfifo;
>
More information about the pve-devel
mailing list