all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Leo Nunner <l.nunner@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [PATCH qemu-server] fix #4249: make qemu_img_convert respect bwlimit
Date: Fri, 16 Sep 2022 13:47:41 +0200	[thread overview]
Message-ID: <20220916114741.107577-1-l.nunner@proxmox.com> (raw)

Previously, cloning a stopped VM didn't respect bwlimit. Passing the -r
(ratelimit) parameter to qemu-img convert fixes this issue.

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);
     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);
 	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"
@@ -63,7 +63,7 @@ my $tests = [
     },
     {
 	name => "raw2qcow2",
-	parameters => [ "local:$vmid/vm-$vmid-disk-0.raw", "local:$vmid/vm-$vmid-disk-0.qcow2", 1024*10, undef, 0 ],
+	parameters => [ "local:$vmid/vm-$vmid-disk-0.raw", "local:$vmid/vm-$vmid-disk-0.qcow2", 1024*10, undef, 0, undef ],
 	expected => [
 	    "/usr/bin/qemu-img", "convert", "-p", "-n", "-f", "raw", "-O", "qcow2",
 	    "/var/lib/vz/images/$vmid/vm-$vmid-disk-0.raw", "/var/lib/vz/images/$vmid/vm-$vmid-disk-0.qcow2"
@@ -71,7 +71,7 @@ my $tests = [
     },
     {
 	name => "local2rbd",
-	parameters => [ "local:$vmid/vm-$vmid-disk-0.raw", "rbd-store:vm-$vmid-disk-0", 1024*10, undef, 0 ],
+	parameters => [ "local:$vmid/vm-$vmid-disk-0.raw", "rbd-store:vm-$vmid-disk-0", 1024*10, undef, 0, undef ],
 	expected => [
 	    "/usr/bin/qemu-img", "convert", "-p", "-n", "-f", "raw", "-O", "raw",
 	    "/var/lib/vz/images/$vmid/vm-$vmid-disk-0.raw", "rbd:cpool/vm-$vmid-disk-0:mon_host=127.0.0.42;127.0.0.21;[\\:\\:1]:auth_supported=none"
@@ -79,7 +79,7 @@ my $tests = [
     },
     {
 	name => "rbd2local",
-	parameters => [ "rbd-store:vm-$vmid-disk-0", "local:$vmid/vm-$vmid-disk-0.raw", 1024*10, undef, 0 ],
+	parameters => [ "rbd-store:vm-$vmid-disk-0", "local:$vmid/vm-$vmid-disk-0.raw", 1024*10, undef, 0, undef ],
 	expected => [
 	    "/usr/bin/qemu-img", "convert", "-p", "-n", "-f", "raw", "-O", "raw",
 	    "rbd:cpool/vm-$vmid-disk-0:mon_host=127.0.0.42;127.0.0.21;[\\:\\:1]:auth_supported=none", "/var/lib/vz/images/$vmid/vm-$vmid-disk-0.raw"
@@ -87,7 +87,7 @@ my $tests = [
     },
     {
 	name => "local2zos",
-	parameters => [ "local:$vmid/vm-$vmid-disk-0.raw", "zfs-over-iscsi:vm-$vmid-disk-0", 1024*10, undef, 0 ],
+	parameters => [ "local:$vmid/vm-$vmid-disk-0.raw", "zfs-over-iscsi:vm-$vmid-disk-0", 1024*10, undef, 0, undef ],
 	expected => [
 	    "/usr/bin/qemu-img", "convert", "-p", "-n", "-f", "raw", "--target-image-opts",
 	    "/var/lib/vz/images/$vmid/vm-$vmid-disk-0.raw",
@@ -96,7 +96,7 @@ my $tests = [
     },
     {
 	name => "zos2local",
-	parameters => [ "zfs-over-iscsi:vm-$vmid-disk-0", "local:$vmid/vm-$vmid-disk-0.raw", 1024*10, undef, 0 ],
+	parameters => [ "zfs-over-iscsi:vm-$vmid-disk-0", "local:$vmid/vm-$vmid-disk-0.raw", 1024*10, undef, 0, undef ],
 	expected => [
 	    "/usr/bin/qemu-img", "convert", "-p", "-n", "--image-opts", "-O", "raw",
 	    "file.driver=iscsi,file.transport=tcp,file.initiator-name=foobar,file.portal=127.0.0.1,file.target=iqn.2019-10.org.test:foobar,file.lun=1,driver=raw",
@@ -105,7 +105,7 @@ my $tests = [
     },
     {
 	name => "zos2rbd",
-	parameters => [ "zfs-over-iscsi:vm-$vmid-disk-0", "rbd-store:vm-$vmid-disk-0", 1024*10, undef, 0 ],
+	parameters => [ "zfs-over-iscsi:vm-$vmid-disk-0", "rbd-store:vm-$vmid-disk-0", 1024*10, undef, 0, undef ],
 	expected => [
 	    "/usr/bin/qemu-img", "convert", "-p", "-n", "--image-opts", "-O", "raw",
 	    "file.driver=iscsi,file.transport=tcp,file.initiator-name=foobar,file.portal=127.0.0.1,file.target=iqn.2019-10.org.test:foobar,file.lun=1,driver=raw",
@@ -114,7 +114,7 @@ my $tests = [
     },
     {
 	name => "rbd2zos",
-	parameters => [ "rbd-store:vm-$vmid-disk-0", "zfs-over-iscsi:vm-$vmid-disk-0", 1024*10, undef, 0  ],
+	parameters => [ "rbd-store:vm-$vmid-disk-0", "zfs-over-iscsi:vm-$vmid-disk-0", 1024*10, undef, 0, undef ],
 	expected => [
 	    "/usr/bin/qemu-img", "convert", "-p", "-n", "-f", "raw", "--target-image-opts",
 	    "rbd:cpool/vm-$vmid-disk-0:mon_host=127.0.0.42;127.0.0.21;[\\:\\:1]:auth_supported=none",
@@ -123,7 +123,7 @@ my $tests = [
     },
     {
 	name => "local2lvmthin",
-	parameters => [ "local:$vmid/vm-$vmid-disk-0.raw", "local-lvm:vm-$vmid-disk-0", 1024*10, undef, 0 ],
+	parameters => [ "local:$vmid/vm-$vmid-disk-0.raw", "local-lvm:vm-$vmid-disk-0", 1024*10, undef, 0, undef ],
 	expected => [
 	    "/usr/bin/qemu-img", "convert", "-p", "-n", "-f", "raw", "-O", "raw",
 	    "/var/lib/vz/images/$vmid/vm-$vmid-disk-0.raw",
@@ -132,7 +132,7 @@ my $tests = [
     },
     {
 	name => "lvmthin2local",
-	parameters => [ "local-lvm:vm-$vmid-disk-0", "local:$vmid/vm-$vmid-disk-0.raw", 1024*10, undef, 0 ],
+	parameters => [ "local-lvm:vm-$vmid-disk-0", "local:$vmid/vm-$vmid-disk-0.raw", 1024*10, undef, 0, undef ],
 	expected => [
 	    "/usr/bin/qemu-img", "convert", "-p", "-n", "-f", "raw", "-O", "raw",
 	    "/dev/pve/vm-$vmid-disk-0",
@@ -141,7 +141,7 @@ my $tests = [
     },
     {
 	name => "zeroinit",
-	parameters => [ "local-lvm:vm-$vmid-disk-0", "local:$vmid/vm-$vmid-disk-0.raw", 1024*10, undef, 1 ],
+	parameters => [ "local-lvm:vm-$vmid-disk-0", "local:$vmid/vm-$vmid-disk-0.raw", 1024*10, undef, 1, undef ],
 	expected => [
 	    "/usr/bin/qemu-img", "convert", "-p", "-n", "-f", "raw", "-O", "raw",
 	    "/dev/pve/vm-$vmid-disk-0",
@@ -150,12 +150,12 @@ my $tests = [
     },
     {
 	name => "notexistingstorage",
-	parameters => [ "local-lvm:vm-$vmid-disk-0", "not-existing:$vmid/vm-$vmid-disk-0.raw", 1024*10, undef, 1 ],
+	parameters => [ "local-lvm:vm-$vmid-disk-0", "not-existing:$vmid/vm-$vmid-disk-0.raw", 1024*10, undef, 1, undef ],
 	expected => "storage 'not-existing' does not exist\n",
     },
     {
 	name => "vmdkfile",
-	parameters => [ "./test.vmdk", "local:$vmid/vm-$vmid-disk-0.raw", 1024*10, undef, 0 ],
+	parameters => [ "./test.vmdk", "local:$vmid/vm-$vmid-disk-0.raw", 1024*10, undef, 0, undef ],
 	expected => [
 	    "/usr/bin/qemu-img", "convert", "-p", "-n", "-f", "vmdk", "-O", "raw",
 	    "./test.vmdk",
@@ -164,12 +164,12 @@ my $tests = [
     },
     {
 	name => "notexistingfile",
-	parameters => [ "/foo/bar", "local:$vmid/vm-$vmid-disk-0.raw", 1024*10, undef, 0 ],
+	parameters => [ "/foo/bar", "local:$vmid/vm-$vmid-disk-0.raw", 1024*10, undef, 0, undef ],
 	expected => "source '/foo/bar' is not a valid volid nor path for qemu-img convert\n",
     },
     {
 	name => "efidisk",
-	parameters => [ "/usr/share/kvm/OVMF_VARS-pure-efi.fd", "local:$vmid/vm-$vmid-disk-0.raw", 1024*10, undef, 0 ],
+	parameters => [ "/usr/share/kvm/OVMF_VARS-pure-efi.fd", "local:$vmid/vm-$vmid-disk-0.raw", 1024*10, undef, 0, undef ],
 	expected => [
 	    "/usr/bin/qemu-img", "convert", "-p", "-n", "-O", "raw",
 	    "/usr/share/kvm/OVMF_VARS-pure-efi.fd",
@@ -178,7 +178,7 @@ my $tests = [
     },
     {
 	name => "efi2zos",
-	parameters => [ "/usr/share/kvm/OVMF_VARS-pure-efi.fd", "zfs-over-iscsi:vm-$vmid-disk-0", 1024*10, undef, 0 ],
+	parameters => [ "/usr/share/kvm/OVMF_VARS-pure-efi.fd", "zfs-over-iscsi:vm-$vmid-disk-0", 1024*10, undef, 0, undef ],
 	expected => [
 	    "/usr/bin/qemu-img", "convert", "-p", "-n", "--target-image-opts",
 	    "/usr/share/kvm/OVMF_VARS-pure-efi.fd",
-- 
2.30.2





             reply	other threads:[~2022-09-16 11:48 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-16 11:47 Leo Nunner [this message]
2022-11-16 12:39 ` Thomas Lamprecht

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=20220916114741.107577-1-l.nunner@proxmox.com \
    --to=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