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.bumil...@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;
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
