[pve-devel] [PATCH pve-manager 0/2] ui: fix #3760: change units of ram/swap

[pve-devel] [PATCH pve-manager 0/2] ui: fix #3760: change units of ram/swap

[Noel Ullreich]

Up until now, ram, balloonsize and swap were set in MiB, meaning if you wanted to
create or change a VMs/CTs memory, you would first have to do the math to
convert from GiB to MiB.
This patch adds a dropdown-menu of units, MiB, GiB, and TiB, (although
PiB could easily be added in the future) for ram and swap.
A future patch in line with this could add units in the API, allowing for
improved usability from the commandline (e.g. `qm create <id> --memory 2GiB`).

having `scalingFactor` list in MemoryEdit and Resource Edits might not
be the most elegant solution, the other option was sticking them in
`proxmox-widget-toolkit/src/Utils.js` or using the units from there.
However, if we want to use the units from there, we need to scale them
and trim the unneeded units. So I thought the least invasive solution
was to put the `scalingFactor` right in the Files.

Noel Ullreich (2):
  ui: fix #3760: change unit of memory of a VM
  ui: fix: change units of memory/swap for containers

 www/manager6/lxc/ResourceEdit.js | 136 ++++++++++++++++---
 www/manager6/qemu/MemoryEdit.js  | 221 +++++++++++++++++++++++--------
 2 files changed, 285 insertions(+), 72 deletions(-)

-- 
2.30.2

[pve-devel] [PATCH pve-manager 1/2] ui: fix #3760: change unit of memory of a VM

[Noel Ullreich]

This patch adds a dropdown-menu (in the web interface) of units, MiB, GiB,
and TiB, (although PiB could easily be added in the future) for ram
and balloonsize of VMs.

Signed-off-by: Noel Ullreich <n.ullreich@proxmox.com>
---
 www/manager6/qemu/MemoryEdit.js | 221 ++++++++++++++++++++++++--------
 1 file changed, 168 insertions(+), 53 deletions(-)

diff --git a/www/manager6/qemu/MemoryEdit.js b/www/manager6/qemu/MemoryEdit.js
index 5e91dc9be..341f4611d 100644
--- a/www/manager6/qemu/MemoryEdit.js
+++ b/www/manager6/qemu/MemoryEdit.js
@@ -1,3 +1,9 @@
+let scalingFactor = [
+    { 'name': gettext('MiB'), 'factor': 1 },
+    { 'name': gettext('GiB'), 'factor': 1024 },
+    { 'name': gettext('TiB'), 'factor': 1024**2 },
+];
+
 Ext.define('PVE.qemu.MemoryInputPanel', {
     extend: 'Proxmox.panel.InputPanel',
     alias: 'widget.pveQemuMemoryPanel',
@@ -23,26 +29,28 @@ Ext.define('PVE.qemu.MemoryInputPanel', {
 		let memory = view.down('pveMemoryField[name=memory]');
 		// NOTE: we only set memory but that then sets balloon in its change handler
 		if (viewModel.get('current.ostype') === 'win11') {
-		    memory.setValue('4096');
+		    memory.setValue(4);
 		} else {
-		    memory.setValue('2048');
+		    memory.setValue(2);
 		}
+		memory.setMinValue(1);
+		memory.step=1;
 	    }
 	},
     },
 
     onGetValues: function(values) {
-	var me = this;
+	let me = this;
 
-	var res = {};
+	let res = {};
 
-	res.memory = values.memory;
-	res.balloon = values.balloon;
+	res.memory = values.memory*values.memorySize;
+	res.balloon = values.balloon*values.balloonSize;
 
 	if (!values.ballooning) {
 	    res.balloon = 0;
 	    res.delete = 'shares';
-	} else if (values.memory === values.balloon) {
+	} else if (res.memory===res.balloon) {
 	    delete res.balloon;
 	    res.delete = 'balloon,shares';
 	} else if (Ext.isDefined(values.shares) && values.shares !== "") {
@@ -54,53 +62,155 @@ Ext.define('PVE.qemu.MemoryInputPanel', {
 	return res;
     },
 
+    onSetValues: function(values) {
+	let me = this;
+	let res = structuredClone(values);
+
+	for (let i in scalingFactor) {
+	    if (Object.prototype.hasOwnProperty.call(scalingFactor, i)) {
+		if (values.memory%scalingFactor[i].factor===0) {
+		    res.memory = values.memory/scalingFactor[i].factor;
+		    res.memorySize = scalingFactor[i];
+		}
+		if (values.balloon%scalingFactor[i].factor===0 && values.balloon) {
+		    res.balloon = values.balloon/scalingFactor[i].factor;
+		    res.balloonSize = scalingFactor[i];
+		}
+	    }
+	}
+
+	return res;
+    },
+
     initComponent: function() {
-	var me = this;
-	var labelWidth = 160;
+	let me = this;
+	let labelWidth = 160;
 
-	me.items= [
+	me.items = [
 	    {
-		xtype: 'pveMemoryField',
+		xtype: 'fieldcontainer',
+		layout: 'hbox',
 		labelWidth: labelWidth,
-		fieldLabel: gettext('Memory') + ' (MiB)',
-		name: 'memory',
-		value: '512', // better defaults get set via the view controllers afterrender
-		minValue: 1,
-		step: 32,
-		hotplug: me.hotplug,
-		listeners: {
-		    change: function(f, value, old) {
-			var bf = me.down('field[name=balloon]');
-			var balloon = bf.getValue();
-			bf.setMaxValue(value);
-			if (balloon === old) {
-			    bf.setValue(value);
-			}
-			bf.validate();
+		fieldLabel: gettext('Memory'),
+		items: [
+		    {
+			xtype: 'pveMemoryField',
+			flex: 2,
+			name: 'memory',
+			value: 512, // better defaults get set via the view controllers afterrender
+			minValue: 16,
+			step: 32,
+			hotplug: me.hotplug,
+			listeners: {
+			    change: function(f, value, old) {
+				let bf = me.down('field[name=balloon]');
+				let bsf = me.down('field[name=balloonSize]');
+				let msf = me.down('field[name=memorySize]');
+
+				let balloon = bf.getValue();
+				let balloonSize = bsf.getValue();
+				let memorySize = msf.getValue();
+
+				//max value is set based on which units are currently in use
+				bf.setMaxValue(value*memorySize/balloonSize);
+
+				if (balloon===old && memorySize===balloonSize) {
+				    bf.setValue(value);
+				}
+				bf.validate();
+			    },
+			},
 		    },
-		},
+		    {
+			xtype: 'combobox',
+			name: 'memorySize',
+			itemId: 'memorySize',
+			value: me.insideWizard ? scalingFactor[1] : scalingFactor[0],
+			flex: 1,
+			editable: false,
+			allowBlank: false,
+			store: scalingFactor,
+			displayField: 'name',
+			valueField: 'factor',
+			listeners: {
+			    change: function(f, value, old) {
+				let mf = me.down('field[name=memory]');
+				let bsf = me.down('field[name=balloonSize]');
+				let bf = me.down('field[name=balloon]');
+
+				let memory = mf.getValue();
+				let balloonSize = bsf.getValue();
+
+				if (value===1) {
+				    mf.setMinValue(16);
+				    mf.step=32;
+				} else {
+				    mf.setMinValue(1);
+				    mf.step=1;
+				}
+				bf.setMaxValue(memory*value/balloonSize);
+
+				mf.validate();
+				bf.validate();
+			    },
+			},
+		    },
+		],
 	    },
 	];
 
 	me.advancedItems= [
 	    {
-		xtype: 'pveMemoryField',
-		name: 'balloon',
-		minValue: 1,
-		maxValue: me.insideWizard ? 2048 : 512,
-		value: '512', // better defaults get set (indirectly) via the view controllers afterrender
-		step: 32,
-		fieldLabel: gettext('Minimum memory') + ' (MiB)',
-		hotplug: me.hotplug,
+		xtype: 'fieldcontainer',
+		layout: 'hbox',
 		labelWidth: labelWidth,
-		allowBlank: false,
-		listeners: {
-		    change: function(f, value) {
-			var memory = me.down('field[name=memory]').getValue();
-			var shares = me.down('field[name=shares]');
-			shares.setDisabled(value === memory);
+		fieldLabel: gettext('Minimum memory'),
+		items: [
+		    {
+			xtype: 'pveMemoryField',
+			name: 'balloon',
+			flex: 2,
+			minValue: 1,
+			maxValue: me.insideWizard ? 2 : 512,
+			value: 512, // better defaults get set (indirectly) via the view controllers afterrender
+			step: 32,
+			hotplug: me.hotplug,
+			labelWidth: labelWidth,
+			allowBlank: false,
+			listeners: {
+			    change: function(f, value) {
+				let memory = me.down('field[name=memory]').getValue();
+				let shares = me.down('field[name=shares]');
+				shares.setDisabled(value === memory);
+			    },
+			},
 		    },
-		},
+		    {
+			xtype: 'combobox',
+			name: 'balloonSize',
+			itemId: 'balloonSize',
+			value: me.insideWizard ? scalingFactor[1] : scalingFactor[0],
+			flex: 1,
+			editable: false,
+			allowBlank: false,
+			store: scalingFactor,
+			displayField: 'name',
+			valueField: 'factor',
+			listeners: {
+			    change: function(f, value) {
+				let bf = me.down('field[name=balloon]');
+				let memorySize = me.down('field[name=memorySize]').getValue();
+				let memory = me.down('field[name=memory]').getValue();
+
+				bf.step = value===1 ? 32 : 1;
+
+				bf.setMaxValue(memory*memorySize/value);
+
+				bf.validate();
+			    },
+			},
+		    },
+		],
 	    },
 	    {
 		xtype: 'proxmoxintegerfield',
@@ -124,10 +234,13 @@ Ext.define('PVE.qemu.MemoryInputPanel', {
 		fieldLabel: gettext('Ballooning Device'),
 		listeners: {
 		    change: function(f, value) {
-			var bf = me.down('field[name=balloon]');
-			var shares = me.down('field[name=shares]');
-			var memory = me.down('field[name=memory]');
+			let bf = me.down('field[name=balloon]');
+			let shares = me.down('field[name=shares]');
+			let memory = me.down('field[name=memory]');
+			let bsf = me.down('field[name=balloonSize]');
+
 			bf.setDisabled(!value);
+			bsf.setDisabled(!value);
 			shares.setDisabled(!value || bf.getValue() === memory.getValue());
 		    },
 		},
@@ -149,9 +262,9 @@ Ext.define('PVE.qemu.MemoryEdit', {
     extend: 'Proxmox.window.Edit',
 
     initComponent: function() {
-	var me = this;
+	let me = this;
 
-	var memoryhotplug;
+	let memoryhotplug;
 	if (me.hotplug) {
 	    Ext.each(me.hotplug.split(','), function(el) {
 		if (el === 'memory') {
@@ -160,7 +273,7 @@ Ext.define('PVE.qemu.MemoryEdit', {
 	    });
 	}
 
-	var ipanel = Ext.create('PVE.qemu.MemoryInputPanel', {
+	let ipanel = Ext.create('PVE.qemu.MemoryInputPanel', {
 	    hotplug: memoryhotplug,
 	});
 
@@ -176,13 +289,15 @@ Ext.define('PVE.qemu.MemoryEdit', {
 
 	me.load({
 	    success: function(response, options) {
-		var data = response.result.data;
+		let data = response.result.data;
 
-		var values = {
-		    ballooning: data.balloon === 0 ? '0' : '1',
+		let values = {
+		    ballooning: data.balloon === 0 ? 0 : 1,
 		    shares: data.shares,
-		    memory: data.memory || '512',
-		    balloon: data.balloon > 0 ? data.balloon : data.memory || '512',
+		    memory: data.memory || 512,
+		    memorySize: data.memorySize || scalingFactor[0],
+		    balloon: data.balloon > 0 ? data.balloon : data.memory || 512,
+		    balloonSize: data.balloonSize || scalingFactor[0],
 		};
 
 		ipanel.setValues(values);
-- 
2.30.2

[pve-devel] [PATCH pve-manager 2/2] ui: fix: change units of memory/swap for containers

[Noel Ullreich]

This patch adds a dropdown-menu (in the web interface) of units, MiB, GiB,
and TiB, (although PiB could easily be added in the future) for ram
and swapsize of containers.

Signed-off-by: Noel Ullreich <n.ullreich@proxmox.com>
---
 www/manager6/lxc/ResourceEdit.js | 136 ++++++++++++++++++++++++++-----
 1 file changed, 117 insertions(+), 19 deletions(-)

diff --git a/www/manager6/lxc/ResourceEdit.js b/www/manager6/lxc/ResourceEdit.js
index 9f4f7e08d..a5437e292 100644
--- a/www/manager6/lxc/ResourceEdit.js
+++ b/www/manager6/lxc/ResourceEdit.js
@@ -1,10 +1,10 @@
-var labelWidth = 120;
+let labelWidth = 100;
 
 Ext.define('PVE.lxc.MemoryEdit', {
     extend: 'Proxmox.window.Edit',
 
     initComponent: function() {
-	var me = this;
+	let me = this;
 
 	Ext.apply(me, {
 	    subject: gettext('Memory'),
@@ -102,7 +102,7 @@ Ext.define('PVE.lxc.CPUInputPanel', {
     ],
 
     initComponent: function() {
-	var me = this;
+	let me = this;
 
 	me.column1 = [
 	    {
@@ -122,6 +122,12 @@ Ext.define('PVE.lxc.CPUInputPanel', {
     },
 });
 
+let unitScalingFactor = [
+    { 'name': gettext('MiB'), 'factor': 1 },
+    { 'name': gettext('GiB'), 'factor': 1024 },
+    { 'name': gettext('TiB'), 'factor': 1024**2 },
+];
+
 Ext.define('PVE.lxc.MemoryInputPanel', {
     extend: 'Proxmox.panel.InputPanel',
     alias: 'widget.pveLxcMemoryInputPanel',
@@ -130,29 +136,121 @@ Ext.define('PVE.lxc.MemoryInputPanel', {
 
     insideWizard: false,
 
+    onGetValues: function(values) {
+	let me = this;
+	let ret = {};
+
+	ret.memory = values.memory*values.memorySize;
+	ret.swap = values.swap*values.swapSize;
+
+	return ret;
+    },
+
+    onSetValues: function(values) {
+	let me = this;
+	let ret = structuredClone(values);
+
+	for (let i in unitScalingFactor) {
+	    if (Object.prototype.hasOwnProperty.call(unitScalingFactor, i)) {
+		if (values.memory%unitScalingFactor[i].factor===0) {
+		    ret.memory = values.memory/unitScalingFactor[i].factor;
+		    ret.memorySize = unitScalingFactor[i];
+		}
+		if (values.swap%unitScalingFactor[i].factor===0 && values.swap) {
+		    ret.swap = values.swap/unitScalingFactor[i].factor;
+		    ret.swapSize = unitScalingFactor[i];
+		}
+	    }
+	}
+
+	return ret;
+    },
+
     initComponent: function() {
-	var me = this;
+	let me = this;
 
-	var items = [
+	let items = [
 	    {
-		xtype: 'proxmoxintegerfield',
-		name: 'memory',
-		minValue: 16,
-		value: '512',
-		step: 32,
-		fieldLabel: gettext('Memory') + ' (MiB)',
+		xtype: 'fieldcontainer',
+		layout: 'hbox',
 		labelWidth: labelWidth,
-		allowBlank: false,
+		fieldLabel: gettext('Memory'),
+		items: [
+		    {
+			xtype: 'proxmoxintegerfield',
+			name: 'memory',
+			flex: 2,
+			minValue: 16,
+			value: '512',
+			step: 32,
+			labelWidth: labelWidth,
+			allowBlank: false,
+		    },
+		    {
+			xtype: 'combobox',
+			name: 'memorySize',
+			itemId: 'memorySize',
+			value: unitScalingFactor[0], //set to MiB by default
+			flex: 1,
+			editable: false,
+			allowBlank: false,
+			store: unitScalingFactor,
+			displayField: 'name',
+			valueField: 'factor',
+			listeners: {
+			    change: function(f, value) {
+				let mf = me.down('field[name=memory]');
+				if (value===1) {
+				    mf.setMinValue(16);
+				    mf.step=32;
+				} else {
+				    mf.setMinValue(1);
+				    mf.step=1;
+				}
+				mf.validate();
+			    },
+			},
+		    },
+		],
 	    },
 	    {
-		xtype: 'proxmoxintegerfield',
-		name: 'swap',
-		minValue: 0,
-		value: '512',
-		step: 32,
-		fieldLabel: gettext('Swap') + ' (MiB)',
+		xtype: 'fieldcontainer',
+		layout: 'hbox',
 		labelWidth: labelWidth,
-		allowBlank: false,
+		fieldLabel: gettext('Swap'),
+		items: [
+		    {
+			xtype: 'proxmoxintegerfield',
+			name: 'swap',
+			flex: 2,
+			minValue: 0,
+			value: '512',
+			step: 32,
+			labelWidth: labelWidth,
+			allowBlank: false,
+		    },
+		    {
+			xtype: 'combobox',
+			name: 'swapSize',
+			itemId: 'swapSize',
+			value: unitScalingFactor[0], //set to MiB by default
+			flex: 1,
+			editable: false,
+			allowBlank: false,
+			store: unitScalingFactor,
+			displayField: 'name',
+			valueField: 'factor',
+			listeners: {
+			    change: function(f, value) {
+				let sf = me.down('field[name=swap]');
+
+				sf.step = value===1 ? 32 : 1;
+
+				sf.validate();
+			    },
+			},
+		    },
+		],
 	    },
 	];
 
-- 
2.30.2

Re: [pve-devel] [PATCH pve-manager 0/2] ui: fix #3760: change units of ram/swap

[Noel Ullreich]

Ping. Still applies

With me leaving at the end of the month, I want to make sure there is 
plenty of time for my outstanding patches to be reviewed, updated, and 
applied.

On 16-06-2023 11:54, Noel Ullreich wrote:
> Up until now, ram, balloonsize and swap were set in MiB, meaning if you wanted to
> create or change a VMs/CTs memory, you would first have to do the math to
> convert from GiB to MiB.
> This patch adds a dropdown-menu of units, MiB, GiB, and TiB, (although
> PiB could easily be added in the future) for ram and swap.
> A future patch in line with this could add units in the API, allowing for
> improved usability from the commandline (e.g. `qm create <id> --memory 2GiB`).
>
> having `scalingFactor` list in MemoryEdit and Resource Edits might not
> be the most elegant solution, the other option was sticking them in
> `proxmox-widget-toolkit/src/Utils.js` or using the units from there.
> However, if we want to use the units from there, we need to scale them
> and trim the unneeded units. So I thought the least invasive solution
> was to put the `scalingFactor` right in the Files.
>
> Noel Ullreich (2):
>    ui: fix #3760: change unit of memory of a VM
>    ui: fix: change units of memory/swap for containers
>
>   www/manager6/lxc/ResourceEdit.js | 136 ++++++++++++++++---
>   www/manager6/qemu/MemoryEdit.js  | 221 +++++++++++++++++++++++--------
>   2 files changed, 285 insertions(+), 72 deletions(-)
>

Re: [pve-devel] [PATCH pve-manager 0/2] ui: fix #3760: change units of ram/swap

[Fiona Ebner]

Am 16.06.23 um 11:54 schrieb Noel Ullreich:
> Up until now, ram, balloonsize and swap were set in MiB, meaning if you wanted to
> create or change a VMs/CTs memory, you would first have to do the math to
> convert from GiB to MiB.
> This patch adds a dropdown-menu of units, MiB, GiB, and TiB, (although
> PiB could easily be added in the future) for ram and swap.
> A future patch in line with this could add units in the API, allowing for
> improved usability from the commandline (e.g. `qm create <id> --memory 2GiB`).
> 
> having `scalingFactor` list in MemoryEdit and Resource Edits might not
> be the most elegant solution, the other option was sticking them in
> `proxmox-widget-toolkit/src/Utils.js` or using the units from there.
> However, if we want to use the units from there, we need to scale them
> and trim the unneeded units. So I thought the least invasive solution
> was to put the `scalingFactor` right in the Files.
> 

Haven't looked at the details, but we already have
Proxmox.form.SizeField (in widget-toolkit's
src/form/BandwidthSelector.js) with a comment

// display unit (TODO: make (optionally) selectable)
unit: 'MiB',

Maybe you can implement the change there and then simply re-use that
component here to avoid some duplication?