[pve-devel] applied: [PATCH v2 qemu-server] restore: implement rate limiting

Thomas Lamprecht t.lamprecht at proxmox.com
Wed Mar 21 11:17:41 CET 2018


applied

added two small fixups on top (see below)

On 2/22/18 5:15 PM, Wolfgang Bumiller wrote:
> Signed-off-by: Wolfgang Bumiller <w.bumiller at proxmox.com>
> ---
> Changes (addressed comments from dominik):
> * bwlimit is now an integer, now sure if this is really better, but
>   sub-kB/s accuracy seems rather useless here
> * handle undef when going through storage limits
> 
>  PVE/API2/Qemu.pm     | 11 +++++++-
>  PVE/CLI/qmrestore.pm |  6 ++++
>  PVE/QemuServer.pm    | 79 +++++++++++++++++++++++++++++++++++++++++-----------
>  3 files changed, 78 insertions(+), 18 deletions(-)
> 
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index b277a26..b569328 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.",

synced description with the one you use in the container patchset:
"Override i/o bandwidth limit (in KiB/s)."

> +		    optional => 1,
> +		    type => 'integer',
> +		    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.",

same here

> +		optional => 1,
> +		type => 'number',
> +		minimum => '0',
> +	    }
>  	},
>      },
>      returns => { 
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 20d6682..9c300e8 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;

given this         ^^^^ 

>  
> -    my $uncomp = '';
> -    if ($comp) {
> +    my $cfg = PVE::Storage::config();
> +    my $commands = [];
> +    my $bwlimit = $opts->{bwlimit};
> +
> +    my $dbg_cmdstring = '';
> +    my $add_pipe = sub {
> +	my ($cmd) = @_;
> +	push @$commands, $cmd;
> +	$dbg_cmdstring .= ' | ' if length($dbg_cmdstring);
> +	$dbg_cmdstring .= PVE::Tools::cmd2string($cmd);
>  	$readfrom = '-';
> -	my $qarchive = PVE::Tools::shellquote($archive);
> +    };
> +
> +    my $input = undef;
> +    if ($archive eq '-') {

this catches all STDIN cases, else will never be taken with '-'

> +	$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 '-') {

so this check here is unnecessary, I removed it.

> +		    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,24 @@ 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);
> +	    next if !$limit;
> +	    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 +5720,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 +5746,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 +5799,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 +5812,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