From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
Leo Nunner <l.nunner@proxmox.com>
Subject: Re: [pve-devel] [PATCH qemu-server] fix #4249: make qemu_img_convert respect bwlimit
Date: Wed, 16 Nov 2022 13:39:30 +0100 [thread overview]
Message-ID: <16fbd4f9-dd57-ea2b-53df-d8d82eb7045a@proxmox.com> (raw)
In-Reply-To: <20220916114741.107577-1-l.nunner@proxmox.com>
Am 16/09/2022 um 13:47 schrieb Leo Nunner:
> Previously, cloning a stopped VM didn't respect bwlimit. Passing the -r
> (ratelimit) parameter to qemu-img convert fixes this issue.
looks mostly OK
> Signed-off-by: Leo Nunner <l.nunner@proxmox.com>
> ---
> With qemu_img_convert getting a parameter for bwlimit, would it maybe
> also make sense to apply the migration limit inside ImportDisk? I'm not
> quite sure if this falls under that category though.
>
> PVE/QemuServer.pm | 7 ++++---
> PVE/QemuServer/ImportDisk.pm | 2 +-
> test/run_qemu_img_convert_tests.pl | 32 +++++++++++++++---------------
> 3 files changed, 21 insertions(+), 20 deletions(-)
>
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index c706653..0be906e 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -7357,7 +7357,7 @@ sub convert_iscsi_path {
> }
>
> sub qemu_img_convert {
> - my ($src_volid, $dst_volid, $size, $snapname, $is_zero_initialized) = @_;
> + my ($src_volid, $dst_volid, $size, $snapname, $is_zero_initialized, $bwlimit) = @_;
>
> my $storecfg = PVE::Storage::config();
> my ($src_storeid, $src_volname) = PVE::Storage::parse_volume_id($src_volid, 1);
> @@ -7397,6 +7397,7 @@ sub qemu_img_convert {
> if $snapname && $src_format && $src_format eq "qcow2";
> push @$cmd, '-t', 'none' if $dst_scfg->{type} eq 'zfspool';
> push @$cmd, '-T', $cachemode if defined($cachemode);
> + push @$cmd, '-r', "${bwlimit}K" if defined($bwlimit);
>
> if ($src_is_iscsi) {
> push @$cmd, '--image-opts';
> @@ -7777,7 +7778,7 @@ sub clone_disk {
> push $cmd->@*, "bs=$bs", "osize=$size", "if=$src_path", "of=$dst_path";
> run_command($cmd);
> } else {
> - qemu_img_convert($drive->{file}, $newvolid, $size, $snapname, $sparseinit);
> + qemu_img_convert($drive->{file}, $newvolid, $size, $snapname, $sparseinit, $bwlimit);
> }
> }
> }
> @@ -7863,7 +7864,7 @@ sub create_efidisk($$$$$$$) {
> my $volid = PVE::Storage::vdisk_alloc($storecfg, $storeid, $vmid, $fmt, undef, $vars_size);
> PVE::Storage::activate_volumes($storecfg, [$volid]);
>
> - qemu_img_convert($ovmf_vars, $volid, $vars_size_b, undef, 0);
> + qemu_img_convert($ovmf_vars, $volid, $vars_size_b, undef, 0, undef);
fyi, you don't need to add undef explicitly, every parameter not passed along will be
undef anyway.
> my ($size) = PVE::Storage::volume_size_info($storecfg, $volid, 3);
>
> return ($volid, $size/1024);
> diff --git a/PVE/QemuServer/ImportDisk.pm b/PVE/QemuServer/ImportDisk.pm
> index 3e0474b..1c0851d 100755
> --- a/PVE/QemuServer/ImportDisk.pm
> +++ b/PVE/QemuServer/ImportDisk.pm
> @@ -71,7 +71,7 @@ sub do_import {
> local $SIG{PIPE} = sub { die "interrupted by signal $!\n"; };
>
> PVE::Storage::activate_volumes($storecfg, [$dst_volid]);
> - PVE::QemuServer::qemu_img_convert($src_path, $dst_volid, $src_size, undef, $zeroinit);
> + PVE::QemuServer::qemu_img_convert($src_path, $dst_volid, $src_size, undef, $zeroinit, undef);
same here
> PVE::Storage::deactivate_volumes($storecfg, [$dst_volid]);
> PVE::QemuConfig->lock_config($vmid, $create_drive) if !$params->{'skip-config-update'};
> };
> diff --git a/test/run_qemu_img_convert_tests.pl b/test/run_qemu_img_convert_tests.pl
> index 5e78d1b..f4ce369 100755
> --- a/test/run_qemu_img_convert_tests.pl
> +++ b/test/run_qemu_img_convert_tests.pl
> @@ -55,7 +55,7 @@ my $storage_config = {
> my $tests = [
> {
> name => 'qcow2raw',
> - parameters => [ "local:$vmid/vm-$vmid-disk-0.qcow2", "local:$vmid/vm-$vmid-disk-0.raw", 1024*10, undef, 0 ],
> + parameters => [ "local:$vmid/vm-$vmid-disk-0.qcow2", "local:$vmid/vm-$vmid-disk-0.raw", 1024*10, undef, 0, undef ],
> expected => [
> "/usr/bin/qemu-img", "convert", "-p", "-n", "-f", "qcow2", "-O", "raw",
> "/var/lib/vz/images/$vmid/vm-$vmid-disk-0.qcow2", "/var/lib/vz/images/$vmid/vm-$vmid-disk-0.raw"
why isn't there a new test with $bwlimit set added here?
prev parent reply other threads:[~2022-11-16 12:40 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-16 11:47 Leo Nunner
2022-11-16 12:39 ` Thomas Lamprecht [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=16fbd4f9-dd57-ea2b-53df-d8d82eb7045a@proxmox.com \
--to=t.lamprecht@proxmox.com \
--cc=l.nunner@proxmox.com \
--cc=pve-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal