public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH qemu-server 1/3] restore vma: fix applying storage-specific bandwidth limit
@ 2021-03-15 11:57 Fabian Ebner
  2021-03-15 11:57 ` [pve-devel] [PATCH manager 2/3] ui: bandwidth limit selector: make it possible to allow zero Fabian Ebner
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Fabian Ebner @ 2021-03-15 11:57 UTC (permalink / raw)
  To: pve-devel

At this stage, there are no keys in %storage_limits to iterate over. The
refactoring in commit 9f3d73bc353c79f84498122b779764184f504005 broke the logic
by accident.

Also explicitly set zero if there is no limit to avoid repeating the
get_bandwith_limit call for the same storage. When accessing the value later,
zero is already correctly handled as 'no limit'.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 PVE/QemuServer.pm | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 57cfe62..15100ed 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -6400,11 +6400,13 @@ sub restore_vma_archive {
 
 	my $virtdev_hash = $parse_backup_hints->($rpcenv, $user, $cfg, $fh, $devinfo, $opts);
 
-	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 $info (values %{$virtdev_hash}) {
+	    my $storeid = $info->{storeid};
+	    next if defined($storage_limits{$storeid});
+
+	    my $limit = PVE::Storage::get_bandwidth_limit('restore', [$storeid], $bwlimit) // 0;
+	    print STDERR "rate limit for storage $storeid: $limit KiB/s\n" if $limit;
+	    $storage_limits{$storeid} = $limit * 1024;
 	}
 
 	foreach my $devname (keys %$devinfo) {
-- 
2.20.1





^ permalink raw reply	[flat|nested] 4+ messages in thread

* [pve-devel] [PATCH manager 2/3] ui: bandwidth limit selector: make it possible to allow zero
  2021-03-15 11:57 [pve-devel] [PATCH qemu-server 1/3] restore vma: fix applying storage-specific bandwidth limit Fabian Ebner
@ 2021-03-15 11:57 ` Fabian Ebner
  2021-03-15 11:57 ` [pve-devel] [PATCH manager 3/3] ui: restore: fix bandwidth limit behavior Fabian Ebner
  2021-03-15 12:30 ` [pve-devel] applied-series: [PATCH qemu-server 1/3] restore vma: fix applying storage-specific bandwidth limit Thomas Lamprecht
  2 siblings, 0 replies; 4+ messages in thread
From: Fabian Ebner @ 2021-03-15 11:57 UTC (permalink / raw)
  To: pve-devel

The initial value is '' and in getSubmitValue(), previously the branch
   if (v == 0 || v == 0.0) return null;
was taken, because of the lax '==' comparision. Make sure we still return null
for '' by explicitly checking for it.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 www/manager6/form/BandwidthSelector.js | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/www/manager6/form/BandwidthSelector.js b/www/manager6/form/BandwidthSelector.js
index 4ae52f9d..6f564fb8 100644
--- a/www/manager6/form/BandwidthSelector.js
+++ b/www/manager6/form/BandwidthSelector.js
@@ -36,12 +36,16 @@ Ext.define('PVE.form.BandwidthField', {
     // for KiB set it to 'KiB'
     backendUnit: undefined,
 
+    // allow setting 0 and using it as a submit value
+    allowZero: false,
+
     items: [
 	{
 	    xtype: 'numberfield',
 	    cbind: {
 		name: '{name}',
 		emptyText: '{emptyText}',
+		allowZero: '{allowZero}',
 	    },
 	    minValue: 0,
 	    step: 1,
@@ -61,7 +65,9 @@ Ext.define('PVE.form.BandwidthField', {
 		    this._transformed = true;
 		}
 
-		if (v == 0) v = undefined;
+		if (Number(v) === 0 && !this.allowZero) {
+		    v = undefined;
+		}
 
 		return Ext.form.field.Text.prototype.setValue.call(this, v);
 	    },
@@ -69,9 +75,13 @@ Ext.define('PVE.form.BandwidthField', {
 		let v = this.processRawValue(this.getRawValue());
 		v = v.replace(this.decimalSeparator, '.');
 
-		if (v === undefined) return null;
-		// FIXME: make it configurable, as this only works if 0 === default
-		if (v == 0 || v == 0.0) return null;
+		if (v === undefined || v === '') {
+		    return null;
+		}
+
+		if (Number(v) === 0) {
+		    return this.allowZero ? 0 : null;
+		}
 
 		let fieldct = this.up('pveBandwidthField');
 		let vm = fieldct.getViewModel();
-- 
2.20.1





^ permalink raw reply	[flat|nested] 4+ messages in thread

* [pve-devel] [PATCH manager 3/3] ui: restore: fix bandwidth limit behavior
  2021-03-15 11:57 [pve-devel] [PATCH qemu-server 1/3] restore vma: fix applying storage-specific bandwidth limit Fabian Ebner
  2021-03-15 11:57 ` [pve-devel] [PATCH manager 2/3] ui: bandwidth limit selector: make it possible to allow zero Fabian Ebner
@ 2021-03-15 11:57 ` Fabian Ebner
  2021-03-15 12:30 ` [pve-devel] applied-series: [PATCH qemu-server 1/3] restore vma: fix applying storage-specific bandwidth limit Thomas Lamprecht
  2 siblings, 0 replies; 4+ messages in thread
From: Fabian Ebner @ 2021-03-15 11:57 UTC (permalink / raw)
  To: pve-devel

by allowing zero and updating the field name. Otherwise the hint mentioning zero
is wrong. Also, it's not only a read limit as the emptyText already indicates.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 www/manager6/window/Restore.js | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/www/manager6/window/Restore.js b/www/manager6/window/Restore.js
index d220c7bf..9e47ebd6 100644
--- a/www/manager6/window/Restore.js
+++ b/www/manager6/window/Restore.js
@@ -59,7 +59,8 @@ Ext.define('PVE.window.Restore', {
 		xtype: 'pveBandwidthField',
 		name: 'bwlimit',
 		backendUnit: 'KiB',
-		fieldLabel: gettext('Read Limit'),
+		allowZero: true,
+		fieldLabel: gettext('Bandwidth Limit'),
 		emptyText: gettext('Defaults to target storage restore limit'),
 		autoEl: {
 		    tag: 'div',
-- 
2.20.1





^ permalink raw reply	[flat|nested] 4+ messages in thread

* [pve-devel] applied-series: [PATCH qemu-server 1/3] restore vma: fix applying storage-specific bandwidth limit
  2021-03-15 11:57 [pve-devel] [PATCH qemu-server 1/3] restore vma: fix applying storage-specific bandwidth limit Fabian Ebner
  2021-03-15 11:57 ` [pve-devel] [PATCH manager 2/3] ui: bandwidth limit selector: make it possible to allow zero Fabian Ebner
  2021-03-15 11:57 ` [pve-devel] [PATCH manager 3/3] ui: restore: fix bandwidth limit behavior Fabian Ebner
@ 2021-03-15 12:30 ` Thomas Lamprecht
  2 siblings, 0 replies; 4+ messages in thread
From: Thomas Lamprecht @ 2021-03-15 12:30 UTC (permalink / raw)
  To: Proxmox VE development discussion, Fabian Ebner

On 15.03.21 12:57, Fabian Ebner wrote:
> At this stage, there are no keys in %storage_limits to iterate over. The
> refactoring in commit 9f3d73bc353c79f84498122b779764184f504005 broke the logic
> by accident.
> 
> Also explicitly set zero if there is no limit to avoid repeating the
> get_bandwith_limit call for the same storage. When accessing the value later,
> zero is already correctly handled as 'no limit'.
> 
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
>  PVE/QemuServer.pm | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
>

applied all three patches, thanks!




^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-03-15 12:30 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-15 11:57 [pve-devel] [PATCH qemu-server 1/3] restore vma: fix applying storage-specific bandwidth limit Fabian Ebner
2021-03-15 11:57 ` [pve-devel] [PATCH manager 2/3] ui: bandwidth limit selector: make it possible to allow zero Fabian Ebner
2021-03-15 11:57 ` [pve-devel] [PATCH manager 3/3] ui: restore: fix bandwidth limit behavior Fabian Ebner
2021-03-15 12:30 ` [pve-devel] applied-series: [PATCH qemu-server 1/3] restore vma: fix applying storage-specific bandwidth limit Thomas Lamprecht

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal