[pve-devel] [PATCH manager v2 00/12] multi tab disk panel & multi disk wizard

[pve-devel] [PATCH manager v2 00/12] multi tab disk panel & multi disk wizard

[Dominik Csapak]

this series is intended to replace dominics and my previous attempts
at this [0][1][2]

splits the bandwidth options into their on tab on the disk panel and
introduces a 'MultiHDEdit' panel which creates/deletes the
HDEdit panels on demand.

The UX is modeled after Dominics first attempt, but a very different
approach code-wise. instead of having a seperate 'data' panel that
contains the vm config, let the multi disk panel handle that
and pass it through to the panels below. this way the HDEdit does
not need a big code-change to get/set the config.

changes from v1:
* fixed a bug which prevented the wizard from finishing
* made the wizard a little wider so that the form field labes are
  readable
* added logic to use the ostype to determine the first disk if one
  deleted all before

patch 10/12 and 12/12 are new, other changes are in 11/12

0: https://lists.proxmox.com/pipermail/pve-devel/2021-June/048690.html
1: https://lists.proxmox.com/pipermail/pve-devel/2021-July/049295.html
2: https://lists.proxmox.com/pipermail/pve-devel/2021-September/050013.html

Dominik Csapak (12):
  ui: qemu/HDEdit: move Bandwidth options to a different tab
  ui: form/ControllerSelector: set correct max value for the device-id
  ui: refactor sortByPreviousUsage and nextFreeDisk
  ui: form/ControllerSelector: add updateVMConfig and getConfId
  ui: qemu/HDEdit: use me instead of this
  ui: qemu/HDEdit: fire an event when the disk id changes
  ui: qemu/OSTypeEdit: drop throwing an error on multiple widgets
  ui: Utils: add capture group for the id in bus_match
  ui: form/ControllerSelector: add option for selecting free on inital
    config
  ui: qemu/OSTypeEdit: set ostype in viewmodel
  ui: add qemu/MultiHDEdit and use it in the wizard
  ui: window/Wizard: make it a little wider

 www/manager6/Makefile                   |   1 +
 www/manager6/Utils.js                   |  48 +++-
 www/manager6/form/ControllerSelector.js |  65 +++---
 www/manager6/qemu/CreateWizard.js       |   4 +-
 www/manager6/qemu/HDEdit.js             | 143 ++++++++----
 www/manager6/qemu/MultiHDEdit.js        | 294 ++++++++++++++++++++++++
 www/manager6/qemu/OSTypeEdit.js         |   3 +-
 www/manager6/window/Wizard.js           |   7 +-
 8 files changed, 482 insertions(+), 83 deletions(-)
 create mode 100644 www/manager6/qemu/MultiHDEdit.js

-- 
2.30.2

[pve-devel] [PATCH manager v2 01/12] ui: qemu/HDEdit: move Bandwidth options to a different tab

[Dominik Csapak]

for that we have to nest the now two tabs in a tabpanel into an inputpanel.
to prevent the options to be collected twice, we override the
'getValues' function of the 'sub-inputpanels' to return an empty object.
(we could make that an option for the inputpanel, but not necessary for
now)

also we have to move the 'bodyPadding' of the wizard to the 'defaults'
so we can override it for specific panels

and we have to manually set the width of the edit window since it
believes we only have a single column ('twoColumns' could also be an
option of the edit window should we need that again)

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/manager6/qemu/CreateWizard.js |   1 +
 www/manager6/qemu/HDEdit.js       | 113 +++++++++++++++++++++---------
 www/manager6/window/Wizard.js     |   5 +-
 3 files changed, 85 insertions(+), 34 deletions(-)

diff --git a/www/manager6/qemu/CreateWizard.js b/www/manager6/qemu/CreateWizard.js
index d4535c9d..015a099d 100644
--- a/www/manager6/qemu/CreateWizard.js
+++ b/www/manager6/qemu/CreateWizard.js
@@ -155,6 +155,7 @@ Ext.define('PVE.qemu.CreateWizard', {
 	},
 	{
 	    xtype: 'pveQemuHDInputPanel',
+	    padding: 0,
 	    bind: {
 		nodename: '{nodename}',
 	    },
diff --git a/www/manager6/qemu/HDEdit.js b/www/manager6/qemu/HDEdit.js
index 95a98b0b..ae7e3fde 100644
--- a/www/manager6/qemu/HDEdit.js
+++ b/www/manager6/qemu/HDEdit.js
@@ -155,6 +155,8 @@ Ext.define('PVE.qemu.HDInputPanel', {
 	me.down('#hdimage').setStorage(undefined, nodename);
     },
 
+    hasAdvanced: true,
+
     initComponent: function() {
 	var me = this;
 
@@ -162,17 +164,17 @@ Ext.define('PVE.qemu.HDInputPanel', {
 
 	me.drive = {};
 
-	me.column1 = [];
-	me.column2 = [];
+	let column1 = [];
+	let column2 = [];
 
-	me.advancedColumn1 = [];
-	me.advancedColumn2 = [];
+	let advancedColumn1 = [];
+	let advancedColumn2 = [];
 
 	if (!me.confid || me.unused) {
 	    me.bussel = Ext.create('PVE.form.ControllerSelector', {
 		vmconfig: me.insideWizard ? { ide2: 'cdrom' } : {},
 	    });
-	    me.column1.push(me.bussel);
+	    column1.push(me.bussel);
 
 	    me.scsiController = Ext.create('Ext.form.field.Display', {
 		fieldLabel: gettext('SCSI Controller'),
@@ -184,7 +186,7 @@ Ext.define('PVE.qemu.HDInputPanel', {
 		submitValue: false,
 		hidden: true,
 	    });
-	    me.column1.push(me.scsiController);
+	    column1.push(me.scsiController);
 	}
 
 	if (me.unused) {
@@ -198,9 +200,9 @@ Ext.define('PVE.qemu.HDInputPanel', {
 		data: [],
 		allowBlank: false,
 	    });
-	    me.column1.push(me.unusedDisks);
+	    column1.push(me.unusedDisks);
 	} else if (me.isCreate) {
-	    me.column1.push({
+	    column1.push({
 		xtype: 'pveDiskStorageSelector',
 		storageContent: 'images',
 		name: 'disk',
@@ -208,7 +210,7 @@ Ext.define('PVE.qemu.HDInputPanel', {
 		autoSelect: me.insideWizard,
 	    });
 	} else {
-	    me.column1.push({
+	    column1.push({
 		xtype: 'textfield',
 		disabled: true,
 		submitValue: false,
@@ -217,7 +219,7 @@ Ext.define('PVE.qemu.HDInputPanel', {
 	    });
 	}
 
-	me.column2.push(
+	column2.push(
 	    {
 		xtype: 'CacheTypeSelector',
 		name: 'cache',
@@ -232,7 +234,7 @@ Ext.define('PVE.qemu.HDInputPanel', {
 	    },
 	);
 
-	me.advancedColumn1.push(
+	advancedColumn1.push(
 	    {
 		xtype: 'proxmoxcheckbox',
 		disabled: me.confid && me.confid.match(/^virtio/),
@@ -249,6 +251,31 @@ Ext.define('PVE.qemu.HDInputPanel', {
 		reference: 'iothread',
 		name: 'iothread',
 	    },
+	);
+
+	advancedColumn2.push(
+	    {
+		xtype: 'proxmoxcheckbox',
+		fieldLabel: gettext('Backup'),
+		autoEl: {
+		    tag: 'div',
+		    'data-qtip': gettext('Include volume in backup job'),
+		},
+		labelWidth: labelWidth,
+		name: 'backup',
+		bind: {
+		    value: '{isIncludedInBackup}',
+		},
+	    },
+	    {
+		xtype: 'proxmoxcheckbox',
+		fieldLabel: gettext('Skip replication'),
+		labelWidth: labelWidth,
+		name: 'noreplicate',
+	    },
+	);
+
+	let bwColumn1 = [
 	    {
 		xtype: 'numberfield',
 		name: 'mbps_rd',
@@ -285,28 +312,9 @@ Ext.define('PVE.qemu.HDInputPanel', {
 		labelWidth: labelWidth,
 		emptyText: gettext('unlimited'),
 	    },
-	);
+	];
 
-	me.advancedColumn2.push(
-	    {
-		xtype: 'proxmoxcheckbox',
-		fieldLabel: gettext('Backup'),
-		autoEl: {
-		    tag: 'div',
-		    'data-qtip': gettext('Include volume in backup job'),
-		},
-		labelWidth: labelWidth,
-		name: 'backup',
-		bind: {
-		    value: '{isIncludedInBackup}',
-		},
-	    },
-	    {
-		xtype: 'proxmoxcheckbox',
-		fieldLabel: gettext('Skip replication'),
-		labelWidth: labelWidth,
-		name: 'noreplicate',
-	    },
+	let bwColumn2 = [
 	    {
 		xtype: 'numberfield',
 		name: 'mbps_rd_max',
@@ -343,10 +351,46 @@ Ext.define('PVE.qemu.HDInputPanel', {
 		labelWidth: labelWidth,
 		emptyText: gettext('default'),
 	    },
-	);
+	];
+
+	me.items = [
+	    {
+		xtype: 'tabpanel',
+		plain: true,
+		bodyPadding: 10,
+		border: 0,
+		items: [
+		    {
+			title: gettext('Disk'),
+			xtype: 'inputpanel',
+			reference: 'diskpanel',
+			column1,
+			column2,
+			advancedColumn1,
+			advancedColumn2,
+			showAdvanced: me.showAdvanced,
+			getValues: () => ({}),
+		    },
+		    {
+			title: gettext('Bandwidth'),
+			xtype: 'inputpanel',
+			reference: 'bwpanel',
+			column1: bwColumn1,
+			column2: bwColumn2,
+			showAdvanced: me.showAdvanced,
+			getValues: () => ({}),
+		    },
+		],
+	    },
+	];
 
 	me.callParent();
     },
+
+    setAdvancedVisible: function(visible) {
+	this.lookup('diskpanel').setAdvancedVisible(visible);
+	this.lookup('bwpanel').setAdvancedVisible(visible);
+    },
 });
 
 Ext.define('PVE.qemu.HDEdit', {
@@ -356,6 +400,9 @@ Ext.define('PVE.qemu.HDEdit', {
 
     backgroundDelay: 5,
 
+    width: 600,
+    bodyPadding: 0,
+
     initComponent: function() {
 	var me = this;
 
diff --git a/www/manager6/window/Wizard.js b/www/manager6/window/Wizard.js
index 47d60b8e..d12f4d90 100644
--- a/www/manager6/window/Wizard.js
+++ b/www/manager6/window/Wizard.js
@@ -131,7 +131,7 @@ Ext.define('PVE.window.Wizard', {
 			itemId: 'wizcontent',
 			xtype: 'tabpanel',
 			activeItem: 0,
-			bodyPadding: 10,
+			bodyPadding: 0,
 			listeners: {
 			    afterrender: function(tp) {
 				tabchange(tp, this.getActiveTab());
@@ -140,6 +140,9 @@ Ext.define('PVE.window.Wizard', {
 				tabchange(tp, newcard, oldcard);
 			    },
 			},
+			defaults: {
+			    padding: 10,
+			},
 			items: tabs,
 		    }],
 		},
-- 
2.30.2

[pve-devel] [PATCH manager v2 02/12] ui: form/ControllerSelector: set correct max value for the device-id

[Dominik Csapak]

the 'diskControllerMaxIDs' object in Utils does not describe the
'maximum ids', but the maximum *number* of ids, so the max is one less

correctly set that instead

the api rejected those values (e.g. ide4) already, so its only a ui change

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/manager6/form/ControllerSelector.js | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/www/manager6/form/ControllerSelector.js b/www/manager6/form/ControllerSelector.js
index daca2432..27c06169 100644
--- a/www/manager6/form/ControllerSelector.js
+++ b/www/manager6/form/ControllerSelector.js
@@ -95,7 +95,7 @@ Ext.define('PVE.form.ControllerSelector', {
 				return;
 			    }
 			    let field = me.down('field[name=deviceid]');
-			    field.setMaxValue(PVE.Utils.diskControllerMaxIDs[value]);
+			    field.setMaxValue(PVE.Utils.diskControllerMaxIDs[value] - 1);
 			    field.validate();
 			},
 		    },
@@ -104,7 +104,7 @@ Ext.define('PVE.form.ControllerSelector', {
 		    xtype: 'proxmoxintegerfield',
 		    name: 'deviceid',
 		    minValue: 0,
-		    maxValue: PVE.Utils.diskControllerMaxIDs.ide,
+		    maxValue: PVE.Utils.diskControllerMaxIDs.ide - 1,
 		    value: '0',
 		    flex: 1,
 		    allowBlank: false,
-- 
2.30.2

[pve-devel] [PATCH manager v2 03/12] ui: refactor sortByPreviousUsage and nextFreeDisk

[Dominik Csapak]

we'll use them outside of the controllerSelector soon

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/manager6/Utils.js                   | 46 +++++++++++++++++++++++++
 www/manager6/form/ControllerSelector.js | 45 +++++-------------------
 2 files changed, 54 insertions(+), 37 deletions(-)

diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index 4041c010..8631a67c 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -1757,6 +1757,52 @@ Ext.define('PVE.Utils', {
 
 	return true;
     },
+
+    sortByPreviousUsage: function(vmconfig, controllerList) {
+	if (!controllerList) {
+	    controllerList = ['ide', 'virtio', 'scsi', 'sata'];
+	}
+	let usedControllers = {};
+	for (const type of Object.keys(PVE.Utils.diskControllerMaxIDs)) {
+	    usedControllers[type] = 0;
+	}
+
+	for (const property of Object.keys(vmconfig)) {
+	    if (property.match(PVE.Utils.bus_match) && !vmconfig[property].match(/media=cdrom/)) {
+		const foundController = property.match(PVE.Utils.bus_match)[1];
+		usedControllers[foundController]++;
+	    }
+	}
+
+	let sortPriority = PVE.qemu.OSDefaults.getDefaults(vmconfig.ostype).busPriority;
+
+	let sortedList = Ext.clone(controllerList);
+	sortedList.sort(function(a, b) {
+	    if (usedControllers[b] === usedControllers[a]) {
+		return sortPriority[b] - sortPriority[a];
+	    }
+	    return usedControllers[b] - usedControllers[a];
+	});
+
+	return sortedList;
+    },
+
+    nextFreeDisk: function(controllers, config) {
+	for (const controller of controllers) {
+	    for (let i = 0; i < PVE.Utils.diskControllerMaxIDs[controller]; i++) {
+		let confid = controller + i.toString();
+		if (!Ext.isDefined(config[confid])) {
+		    return {
+			controller,
+			id: i,
+			confid,
+		    };
+		}
+	    }
+	}
+
+	return undefined;
+    },
 },
 
     singleton: true,
diff --git a/www/manager6/form/ControllerSelector.js b/www/manager6/form/ControllerSelector.js
index 27c06169..fcf625a1 100644
--- a/www/manager6/form/ControllerSelector.js
+++ b/www/manager6/form/ControllerSelector.js
@@ -6,44 +6,15 @@ Ext.define('PVE.form.ControllerSelector', {
 
     vmconfig: {}, // used to check for existing devices
 
-    sortByPreviousUsage: function(vmconfig, controllerList) {
-	let usedControllers = {};
-	for (const type of Object.keys(PVE.Utils.diskControllerMaxIDs)) {
-	    usedControllers[type] = 0;
-	}
-
-	for (const property of Object.keys(vmconfig)) {
-	    if (property.match(PVE.Utils.bus_match) && !vmconfig[property].match(/media=cdrom/)) {
-		const foundController = property.match(PVE.Utils.bus_match)[1];
-		usedControllers[foundController]++;
-	    }
-	}
-
-	let sortPriority = PVE.qemu.OSDefaults.getDefaults(vmconfig.ostype).busPriority;
-
-	let sortedList = Ext.clone(controllerList);
-	sortedList.sort(function(a, b) {
-	    if (usedControllers[b] === usedControllers[a]) {
-		return sortPriority[b] - sortPriority[a];
-	    }
-	    return usedControllers[b] - usedControllers[a];
-	});
-
-	return sortedList;
-    },
-
     setToFree: function(controllers, busField, deviceIDField) {
 	let me = this;
-	for (const controller of controllers) {
-	    busField.setValue(controller);
-	    for (let i = 0; i < PVE.Utils.diskControllerMaxIDs[controller]; i++) {
-		let confid = controller + i.toString();
-		if (!Ext.isDefined(me.vmconfig[confid])) {
-		    deviceIDField.setValue(i);
-		    return;
-		}
-	    }
+	let freeId = PVE.Utils.nextFreeDisk(controllers, me.vmconfig);
+
+	if (freeId !== undefined) {
+	    busField.setValue(freeId.controller);
+	    deviceIDField.setValue(freeId.id);
 	}
+
     },
 
     setVMConfig: function(vmconfig, autoSelect) {
@@ -54,7 +25,7 @@ Ext.define('PVE.form.ControllerSelector', {
 	let bussel = me.down('field[name=controller]');
 	let deviceid = me.down('field[name=deviceid]');
 
-	let clist = ['ide', 'virtio', 'scsi', 'sata'];
+	let clist;
 	if (autoSelect === 'cdrom') {
 	    if (!Ext.isDefined(me.vmconfig.ide2)) {
 		bussel.setValue('ide');
@@ -64,7 +35,7 @@ Ext.define('PVE.form.ControllerSelector', {
 	    clist = ['ide', 'scsi', 'sata'];
 	} else {
 	    // in most cases we want to add a disk to the same controller we previously used
-	    clist = me.sortByPreviousUsage(me.vmconfig, clist);
+	    clist = PVE.Utils.sortByPreviousUsage(me.vmconfig);
 	}
 
 	me.setToFree(clist, bussel, deviceid);
-- 
2.30.2

[pve-devel] [PATCH manager v2 04/12] ui: form/ControllerSelector: add updateVMConfig and getConfId

[Dominik Csapak]

those are helpers that will be useful when we want to change the
vmconfig for the ControllerSelector on the fly

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/manager6/form/ControllerSelector.js | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/www/manager6/form/ControllerSelector.js b/www/manager6/form/ControllerSelector.js
index fcf625a1..ba83b4dd 100644
--- a/www/manager6/form/ControllerSelector.js
+++ b/www/manager6/form/ControllerSelector.js
@@ -14,7 +14,13 @@ Ext.define('PVE.form.ControllerSelector', {
 	    busField.setValue(freeId.controller);
 	    deviceIDField.setValue(freeId.id);
 	}
+    },
+
+    updateVMConfig: function(vmconfig) {
+	let me = this;
+	me.vmconfig = Ext.apply({}, vmconfig);
 
+	me.down('field[name=deviceid]').validate();
     },
 
     setVMConfig: function(vmconfig, autoSelect) {
@@ -43,6 +49,14 @@ Ext.define('PVE.form.ControllerSelector', {
 	deviceid.validate();
     },
 
+    getConfId: function() {
+	let me = this;
+	let controller = me.getComponent('controller').getValue() || 'ide';
+	let id = me.getComponent('deviceid').getValue() || 0;
+
+	return `${controller}${id}`;
+    },
+
     initComponent: function() {
 	var me = this;
 
@@ -56,6 +70,7 @@ Ext.define('PVE.form.ControllerSelector', {
 		{
 		    xtype: 'pveBusSelector',
 		    name: 'controller',
+		    itemId: 'controller',
 		    value: PVE.qemu.OSDefaults.generic.busType,
 		    noVirtIO: me.noVirtIO,
 		    allowBlank: false,
@@ -74,6 +89,7 @@ Ext.define('PVE.form.ControllerSelector', {
 		{
 		    xtype: 'proxmoxintegerfield',
 		    name: 'deviceid',
+		    itemId: 'deviceid',
 		    minValue: 0,
 		    maxValue: PVE.Utils.diskControllerMaxIDs.ide - 1,
 		    value: '0',
-- 
2.30.2

[pve-devel] [PATCH manager v2 05/12] ui: qemu/HDEdit: use me instead of this

[Dominik Csapak]

more in line with our remaining code style

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/manager6/qemu/HDEdit.js | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/www/manager6/qemu/HDEdit.js b/www/manager6/qemu/HDEdit.js
index ae7e3fde..bbd4a2c6 100644
--- a/www/manager6/qemu/HDEdit.js
+++ b/www/manager6/qemu/HDEdit.js
@@ -17,21 +17,22 @@ Ext.define('PVE.qemu.HDInputPanel', {
 	xclass: 'Ext.app.ViewController',
 
 	onControllerChange: function(field) {
+	    let me = this;
 	    var value = field.getValue();
 
 	    var allowIOthread = value.match(/^(virtio|scsi)/);
-	    this.lookup('iothread').setDisabled(!allowIOthread);
+	    me.lookup('iothread').setDisabled(!allowIOthread);
 	    if (!allowIOthread) {
-		this.lookup('iothread').setValue(false);
+		me.lookup('iothread').setValue(false);
 	    }
 
 	    var virtio = value.match(/^virtio/);
-	    this.lookup('ssd').setDisabled(virtio);
+	    me.lookup('ssd').setDisabled(virtio);
 	    if (virtio) {
-		this.lookup('ssd').setValue(false);
+		me.lookup('ssd').setValue(false);
 	    }
 
-	    this.lookup('scsiController').setVisible(value.match(/^scsi/));
+	    me.lookup('scsiController').setVisible(value.match(/^scsi/));
 	},
 
 	control: {
-- 
2.30.2

[pve-devel] [PATCH manager v2 06/12] ui: qemu/HDEdit: fire an event when the disk id changes

[Dominik Csapak]

e.g. from scsi0 to scsi1 or from scsi0 to virtio0

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/manager6/qemu/HDEdit.js | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/www/manager6/qemu/HDEdit.js b/www/manager6/qemu/HDEdit.js
index bbd4a2c6..2142c746 100644
--- a/www/manager6/qemu/HDEdit.js
+++ b/www/manager6/qemu/HDEdit.js
@@ -33,6 +33,13 @@ Ext.define('PVE.qemu.HDInputPanel', {
 	    }
 
 	    me.lookup('scsiController').setVisible(value.match(/^scsi/));
+
+	    me.fireIdChange();
+	},
+
+	fireIdChange: function() {
+	    let view = this.getView();
+	    view.fireEvent('diskidchange', view, view.bussel.getConfId());
 	},
 
 	control: {
@@ -40,6 +47,9 @@ Ext.define('PVE.qemu.HDInputPanel', {
 		change: 'onControllerChange',
 		afterrender: 'onControllerChange',
 	    },
+	    'field[name=deviceid]': {
+		change: 'fireIdChange',
+	    },
 	    'field[name=iothread]': {
 		change: function(f, value) {
 		    if (!this.getView().insideWizard) {
-- 
2.30.2

[pve-devel] [PATCH manager v2 07/12] ui: qemu/OSTypeEdit: drop throwing an error on multiple widgets

[Dominik Csapak]

we will have multiple panels with the same widget. Instead
of raising an error in that case, simply ignore it, since
we normally only want to set the default initially, not when
users configured something else

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/manager6/qemu/OSTypeEdit.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/www/manager6/qemu/OSTypeEdit.js b/www/manager6/qemu/OSTypeEdit.js
index 438d7c6b..fad56e8a 100644
--- a/www/manager6/qemu/OSTypeEdit.js
+++ b/www/manager6/qemu/OSTypeEdit.js
@@ -37,7 +37,7 @@ Ext.define('PVE.qemu.OSTypeInputPanel', {
 	    if (widgets.length === 1) {
 		widgets[0].setValue(newValue);
 	    } else {
-		throw 'non unique widget :' + widget + ' in Wizard';
+		// ignore multiple disks, we only want to set the type if there is a single disk
 	    }
 	},
     },
-- 
2.30.2

[pve-devel] [PATCH manager v2 08/12] ui: Utils: add capture group for the id in bus_match

[Dominik Csapak]

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/manager6/Utils.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index 8631a67c..71e5fc9a 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -13,7 +13,7 @@ Ext.define('PVE.Utils', {
 
     toolkit: undefined, // (extjs|touch), set inside Toolkit.js
 
-    bus_match: /^(ide|sata|virtio|scsi)\d+$/,
+    bus_match: /^(ide|sata|virtio|scsi)(\d+)$/,
 
     log_severity_hash: {
 	0: "panic",
-- 
2.30.2

[pve-devel] [PATCH manager v2 09/12] ui: form/ControllerSelector: add option for selecting free on inital config

[Dominik Csapak]

we will sometimes directly give it a config, so the id can be selected
at the start

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/manager6/form/ControllerSelector.js | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/www/manager6/form/ControllerSelector.js b/www/manager6/form/ControllerSelector.js
index ba83b4dd..71332db5 100644
--- a/www/manager6/form/ControllerSelector.js
+++ b/www/manager6/form/ControllerSelector.js
@@ -111,5 +111,9 @@ Ext.define('PVE.form.ControllerSelector', {
 	});
 
 	me.callParent();
+
+	if (me.selectFree) {
+	    me.setVMConfig(me.vmconfig);
+	}
     },
 });
-- 
2.30.2

[pve-devel] [PATCH manager v2 10/12] ui: qemu/OSTypeEdit: set ostype in viewmodel

[Dominik Csapak]

so that we can retrieve it in the MultiDiskPanel

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/manager6/qemu/OSTypeEdit.js | 1 +
 1 file changed, 1 insertion(+)

diff --git a/www/manager6/qemu/OSTypeEdit.js b/www/manager6/qemu/OSTypeEdit.js
index fad56e8a..d9a0988e 100644
--- a/www/manager6/qemu/OSTypeEdit.js
+++ b/www/manager6/qemu/OSTypeEdit.js
@@ -29,6 +29,7 @@ Ext.define('PVE.qemu.OSTypeInputPanel', {
 	    me.setWidget('pveNetworkCardSelector', targetValues.networkCard);
 	    var scsihw = targetValues.scsihw || '__default__';
 	    this.getViewModel().set('current.scsihw', scsihw);
+	    this.getViewModel().set('current.ostype', ostype);
 	},
 	setWidget: function(widget, newValue) {
 	    // changing a widget is safe only if ComponentQuery.query returns us
-- 
2.30.2

[pve-devel] [PATCH manager v2 11/12] ui: add qemu/MultiHDEdit and use it in the wizard

[Dominik Csapak]

this adds a new panel where a user can add multiple disks.

Has a simple grid for displaying the already added disks and displays
a warning triangle if the disk is not valid.

This allows also to create a vm without any disk by removing all of them.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/manager6/Makefile             |   1 +
 www/manager6/qemu/CreateWizard.js |   5 +-
 www/manager6/qemu/HDEdit.js       |   9 +-
 www/manager6/qemu/MultiHDEdit.js  | 294 ++++++++++++++++++++++++++++++
 4 files changed, 304 insertions(+), 5 deletions(-)
 create mode 100644 www/manager6/qemu/MultiHDEdit.js

diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index 7d491f57..d76acf14 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -213,6 +213,7 @@ JSSRC= 							\
 	qemu/MachineEdit.js				\
 	qemu/MemoryEdit.js				\
 	qemu/Monitor.js					\
+	qemu/MultiHDEdit.js				\
 	qemu/NetworkEdit.js				\
 	qemu/OSDefaults.js				\
 	qemu/OSTypeEdit.js				\
diff --git a/www/manager6/qemu/CreateWizard.js b/www/manager6/qemu/CreateWizard.js
index 015a099d..75836aab 100644
--- a/www/manager6/qemu/CreateWizard.js
+++ b/www/manager6/qemu/CreateWizard.js
@@ -154,14 +154,11 @@ Ext.define('PVE.qemu.CreateWizard', {
 	    insideWizard: true,
 	},
 	{
-	    xtype: 'pveQemuHDInputPanel',
-	    padding: 0,
+	    xtype: 'pveMultiHDPanel',
 	    bind: {
 		nodename: '{nodename}',
 	    },
 	    title: gettext('Hard Disk'),
-	    isCreate: true,
-	    insideWizard: true,
 	},
 	{
 	    xtype: 'pveQemuProcessorPanel',
diff --git a/www/manager6/qemu/HDEdit.js b/www/manager6/qemu/HDEdit.js
index 2142c746..9c453b2a 100644
--- a/www/manager6/qemu/HDEdit.js
+++ b/www/manager6/qemu/HDEdit.js
@@ -107,6 +107,12 @@ Ext.define('PVE.qemu.HDInputPanel', {
 	return params;
     },
 
+    updateVMConfig: function(vmconfig) {
+	var me = this;
+	me.vmconfig = vmconfig;
+	me.bussel?.updateVMConfig(vmconfig);
+    },
+
     setVMConfig: function(vmconfig) {
 	var me = this;
 
@@ -183,7 +189,8 @@ Ext.define('PVE.qemu.HDInputPanel', {
 
 	if (!me.confid || me.unused) {
 	    me.bussel = Ext.create('PVE.form.ControllerSelector', {
-		vmconfig: me.insideWizard ? { ide2: 'cdrom' } : {},
+		vmconfig: me.vmconfig,
+		selectFree: true,
 	    });
 	    column1.push(me.bussel);
 
diff --git a/www/manager6/qemu/MultiHDEdit.js b/www/manager6/qemu/MultiHDEdit.js
new file mode 100644
index 00000000..2323be0d
--- /dev/null
+++ b/www/manager6/qemu/MultiHDEdit.js
@@ -0,0 +1,294 @@
+Ext.define('PVE.qemu.MultiHDPanel', {
+    extend: 'Ext.panel.Panel',
+    alias: 'widget.pveMultiHDPanel',
+
+    onlineHelp: 'qm_hard_disk',
+
+    setNodename: function(nodename) {
+	this.items.each((panel) => panel.setNodename(nodename));
+    },
+
+    border: false,
+    bodyBorder: false,
+
+    layout: 'card',
+
+    controller: {
+	xclass: 'Ext.app.ViewController',
+
+	vmconfig: {},
+
+	onAdd: function() {
+	    let me = this;
+	    me.lookup('addButton').setDisabled(true);
+	    me.addDisk();
+	    let count = me.lookup('grid').getStore().getCount() + 1; // +1 is from ide2
+	    me.lookup('addButton').setDisabled(count >= me.maxCount);
+	},
+
+	addDisk: function() {
+	    let me = this;
+	    let view = me.getView();
+	    let grid = me.lookup('grid');
+	    let store = grid.getStore();
+
+	    // get free disk id
+	    let vmconfig = me.getVMConfig(true);
+	    let clist = PVE.Utils.sortByPreviousUsage(vmconfig);
+	    let nextFreeDisk = PVE.Utils.nextFreeDisk(clist, vmconfig);
+	    if (!nextFreeDisk) {
+		return;
+	    }
+
+	    // add store entry + panel
+	    let itemId = 'disk-card-' + ++Ext.idSeed;
+	    let rec = store.add({
+		name: nextFreeDisk.confid,
+		itemId,
+	    })[0];
+
+	    let panel = view.add({
+		vmconfig,
+		border: false,
+		showAdvanced: Ext.state.Manager.getProvider().get('proxmox-advanced-cb'),
+		xtype: 'pveQemuHDInputPanel',
+		bind: {
+		    nodename: '{nodename}',
+		},
+		padding: '0 0 0 5',
+		itemId,
+		isCreate: true,
+		insideWizard: true,
+	    });
+
+	    panel.updateVMConfig(vmconfig);
+
+	    // we need to setup a validitychange handler, so that we can show
+	    // that a disk has invalid fields
+	    let fields = panel.query('field');
+	    fields.forEach((el) => el.on('validitychange', () => {
+		let valid = fields.every((field) => field.isValid());
+		rec.set('valid', valid);
+		me.checkValidity();
+	    }));
+
+	    store.sort();
+
+	    // select if the panel added is the only one
+	    if (store.getCount() === 1) {
+		grid.getSelectionModel().select(0, false);
+	    }
+	},
+
+	getVMConfig: function(all) {
+	    let me = this;
+	    let vm = me.getViewModel();
+
+	    let vmconfig = {
+		ide2: 'media=cdrom',
+		scsihw: vm.get('current.scsihw'),
+		ostype: vm.get('current.ostype'),
+	    };
+
+	    me.lookup('grid').getStore().each((rec) => {
+		if (all || rec.get('valid')) {
+		    vmconfig[rec.get('name')] = rec.get('itemId');
+		}
+	    });
+
+	    return vmconfig;
+	},
+
+	checkValidity: function() {
+	    let me = this;
+	    let valid = me.lookup('grid').getStore().findExact('valid', false) !== -1;
+	    me.lookup('validationfield').setValue(valid);
+	},
+
+	updateVMConfig: function() {
+	    let me = this;
+	    let view = me.getView();
+	    let grid = me.lookup('grid');
+	    let store = grid.getStore();
+
+	    let vmconfig = me.getVMConfig();
+
+	    me.getViewModel().set('current.scsihw', vmconfig.scsihw);
+
+	    let valid = true;
+
+	    store.each((rec) => {
+		let itemId = rec.get('itemId');
+		let name = rec.get('name');
+		let panel = view.getComponent(itemId);
+		if (!panel) {
+		    throw "unexpected missing panel";
+		}
+
+		// copy config for each panel and remote its own id
+		let panel_vmconfig = Ext.apply({}, vmconfig);
+		if (panel_vmconfig[name] === itemId) {
+		    delete panel_vmconfig[name];
+		}
+
+		if (!rec.get('valid')) {
+		    valid = false;
+		}
+
+		panel.updateVMConfig(panel_vmconfig);
+	    });
+
+	    me.lookup('validationfield').setValue(valid);
+	},
+
+	onChange: function(panel, newVal) {
+	    let me = this;
+	    let store = me.lookup('grid').getStore();
+
+	    let el = store.findRecord('itemId', panel.itemId, 0, false, true, true);
+	    if (el.get('name') === newVal) {
+		// do not update if there was no change
+		return;
+	    }
+
+	    el.set('name', newVal);
+	    el.commit();
+
+	    store.sort();
+
+	    // so that it happens after the layouting
+	    setTimeout(function() {
+		me.updateVMConfig();
+	    }, 10);
+	},
+
+	onRemove: function(tableview, rowIndex, colIndex, item, event, record) {
+	    let me = this;
+	    let grid = me.lookup('grid');
+	    let store = grid.getStore();
+	    let removed_idx = store.indexOf(record);
+
+	    let selection = grid.getSelection()[0];
+	    let selected_idx = store.indexOf(selection);
+
+	    if (selected_idx === removed_idx) {
+		let newidx = store.getCount() > removed_idx + 1 ? removed_idx + 1: removed_idx - 1;
+		grid.getSelectionModel().select(newidx, false);
+	    }
+
+	    store.remove(record);
+	    me.getView().remove(record.get('itemId'));
+	    me.lookup('addButton').setDisabled(false);
+	    me.checkValidity();
+	},
+
+	onSelectionChange: function(grid, selection) {
+	    let me = this;
+	    if (!selection || selection.length < 1) {
+		return;
+	    }
+
+	    me.getView().setActiveItem(selection[0].data.itemId);
+	},
+
+	control: {
+	    'pveQemuHDInputPanel': {
+		diskidchange: 'onChange',
+	    },
+	    'grid[reference=grid]': {
+		selectionchange: 'onSelectionChange',
+	    },
+	},
+
+	init: function(view) {
+	    let me = this;
+	    me.onAdd();
+	    me.lookup('grid').getSelectionModel().select(0, false);
+
+	    // only calculate once
+	    me.maxCount = Object.values(PVE.Utils.diskControllerMaxIDs)
+		.reduce((previous, current) => previous+current, 0);
+	},
+    },
+
+    dockedItems: [
+	{
+	    xtype: 'container',
+	    layout: {
+		type: 'vbox',
+		align: 'stretch',
+	    },
+	    dock: 'left',
+	    border: false,
+	    width: 130,
+	    items: [
+		{
+		    xtype: 'grid',
+		    hideHeaders: true,
+		    reference: 'grid',
+		    flex: 1,
+		    emptyText: gettext('No Disks'),
+		    margin: '0 0 5 0',
+		    store: {
+			sorters: [{
+			    sorterFn: function(rec1, rec2) {
+				let [, name1, id1] = PVE.Utils.bus_match.exec(rec1.data.name);
+				let [, name2, id2] = PVE.Utils.bus_match.exec(rec2.data.name);
+
+				if (name1 === name2) {
+				    return parseInt(id1, 10) - parseInt(id2, 10);
+				}
+
+				return name1 < name2 ? -1 : 1;
+			    },
+			}],
+			fields: ['name', 'itemId', 'valid'],
+			data: [],
+		    },
+		    columns: [
+			{
+			    dataIndex: 'name',
+			    renderer: function(val, md, rec) {
+				let warn = '';
+				if (!rec.get('valid')) {
+				    warn = ' <i class="fa warning fa-warning"></i>';
+				}
+				return val + warn;
+			    },
+			    flex: 1,
+			},
+			{
+			    xtype: 'actioncolumn',
+			    width: 30,
+			    align: 'center',
+			    menuDisabled: true,
+			    items: [
+				{
+				    iconCls: 'x-fa fa-trash critical',
+				    tooltip: 'Delete',
+				    handler: 'onRemove',
+				},
+			    ],
+			},
+		    ],
+		},
+		{
+		    xtype: 'button',
+		    reference: 'addButton',
+		    text: gettext('Add'),
+		    iconCls: 'fa fa-plus-circle',
+		    handler: 'onAdd',
+		},
+		{
+		    // dummy field to control wizard validation
+		    xtype: 'textfield',
+		    hidden: true,
+		    reference: 'validationfield',
+		    submitValue: false,
+		    value: true,
+		    validator: (val) => !!val,
+		},
+	    ],
+	},
+    ],
+});
-- 
2.30.2

[pve-devel] [PATCH manager v2 12/12] ui: window/Wizard: make it a little wider

[Dominik Csapak]

for the multi disk panel, we want it to be just a little wider, so
that all form fields are still readable

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/manager6/window/Wizard.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/www/manager6/window/Wizard.js b/www/manager6/window/Wizard.js
index d12f4d90..98e46d44 100644
--- a/www/manager6/window/Wizard.js
+++ b/www/manager6/window/Wizard.js
@@ -3,7 +3,7 @@ Ext.define('PVE.window.Wizard', {
 
     activeTitle: '', // used for automated testing
 
-    width: 700,
+    width: 720,
     height: 510,
 
     modal: true,
-- 
2.30.2

[pve-devel] partially-applied: [PATCH manager v2 00/12] multi tab disk panel & multi disk wizard

[Thomas Lamprecht]

On 22.09.21 11:27, Dominik Csapak wrote:
> this series is intended to replace dominics and my previous attempts
> at this [0][1][2]
> 
> splits the bandwidth options into their on tab on the disk panel and
> introduces a 'MultiHDEdit' panel which creates/deletes the
> HDEdit panels on demand.
> 
> The UX is modeled after Dominics first attempt, but a very different
> approach code-wise. instead of having a seperate 'data' panel that
> contains the vm config, let the multi disk panel handle that
> and pass it through to the panels below. this way the HDEdit does
> not need a big code-change to get/set the config.
> 
> changes from v1:
> * fixed a bug which prevented the wizard from finishing
> * made the wizard a little wider so that the form field labes are
>   readable
> * added logic to use the ostype to determine the first disk if one
>   deleted all before
> 
> patch 10/12 and 12/12 are new, other changes are in 11/12
> 
> 0: https://lists.proxmox.com/pipermail/pve-devel/2021-June/048690.html
> 1: https://lists.proxmox.com/pipermail/pve-devel/2021-July/049295.html
> 2: https://lists.proxmox.com/pipermail/pve-devel/2021-September/050013.html
> 
> Dominik Csapak (12):
>   ui: qemu/HDEdit: move Bandwidth options to a different tab
>   ui: form/ControllerSelector: set correct max value for the device-id
>   ui: refactor sortByPreviousUsage and nextFreeDisk
>   ui: form/ControllerSelector: add updateVMConfig and getConfId
>   ui: qemu/HDEdit: use me instead of this
>   ui: qemu/HDEdit: fire an event when the disk id changes
>   ui: qemu/OSTypeEdit: drop throwing an error on multiple widgets
>   ui: Utils: add capture group for the id in bus_match
>   ui: form/ControllerSelector: add option for selecting free on inital
>     config
>   ui: qemu/OSTypeEdit: set ostype in viewmodel

applied above, delayed below; mostly because I'd like to have some feedback from
others here and I'd also like to have the same for CT, to avoid a feature drift in
the wizards.

thanks!

>   ui: add qemu/MultiHDEdit and use it in the wizard
>   ui: window/Wizard: make it a little wider
> 
>  www/manager6/Makefile                   |   1 +
>  www/manager6/Utils.js                   |  48 +++-
>  www/manager6/form/ControllerSelector.js |  65 +++---
>  www/manager6/qemu/CreateWizard.js       |   4 +-
>  www/manager6/qemu/HDEdit.js             | 143 ++++++++----
>  www/manager6/qemu/MultiHDEdit.js        | 294 ++++++++++++++++++++++++
>  www/manager6/qemu/OSTypeEdit.js         |   3 +-
>  www/manager6/window/Wizard.js           |   7 +-
>  8 files changed, 482 insertions(+), 83 deletions(-)
>  create mode 100644 www/manager6/qemu/MultiHDEdit.js
> 

Re: [pve-devel] partially-applied: [PATCH manager v2 00/12] multi tab disk panel & multi disk wizard

[Aaron Lauterer]

On 10/4/21 09:36, Thomas Lamprecht wrote:
> On 22.09.21 11:27, Dominik Csapak wrote:
>> this series is intended to replace dominics and my previous attempts
>> at this [0][1][2]
>>
>> splits the bandwidth options into their on tab on the disk panel and
>> introduces a 'MultiHDEdit' panel which creates/deletes the
>> HDEdit panels on demand.
>>
>> The UX is modeled after Dominics first attempt, but a very different
>> approach code-wise. instead of having a seperate 'data' panel that
>> contains the vm config, let the multi disk panel handle that
>> and pass it through to the panels below. this way the HDEdit does
>> not need a big code-change to get/set the config.
>>
>> changes from v1:
>> * fixed a bug which prevented the wizard from finishing
>> * made the wizard a little wider so that the form field labes are
>>    readable
>> * added logic to use the ostype to determine the first disk if one
>>    deleted all before
>>
>> patch 10/12 and 12/12 are new, other changes are in 11/12
>>
>> 0: https://lists.proxmox.com/pipermail/pve-devel/2021-June/048690.html
>> 1: https://lists.proxmox.com/pipermail/pve-devel/2021-July/049295.html
>> 2: https://lists.proxmox.com/pipermail/pve-devel/2021-September/050013.html
>>
>> Dominik Csapak (12):
>>    ui: qemu/HDEdit: move Bandwidth options to a different tab
>>    ui: form/ControllerSelector: set correct max value for the device-id
>>    ui: refactor sortByPreviousUsage and nextFreeDisk
>>    ui: form/ControllerSelector: add updateVMConfig and getConfId
>>    ui: qemu/HDEdit: use me instead of this
>>    ui: qemu/HDEdit: fire an event when the disk id changes
>>    ui: qemu/OSTypeEdit: drop throwing an error on multiple widgets
>>    ui: Utils: add capture group for the id in bus_match
>>    ui: form/ControllerSelector: add option for selecting free on inital
>>      config
>>    ui: qemu/OSTypeEdit: set ostype in viewmodel
> 
> applied above, delayed below; mostly because I'd like to have some feedback from
> others here and I'd also like to have the same for CT, to avoid a feature drift in
> the wizards.

I took the last two patches for a quick spin, and I have to say that I do like the functionality. Yes it does change the way one interacts with the wizard quite a bit, but it really makes it so much easier to create VMs with no disks at all (useful when preparing for qm importdisk) or more than one disk.

> 
> thanks!
> 
>>    ui: add qemu/MultiHDEdit and use it in the wizard
>>    ui: window/Wizard: make it a little wider
>>
>>   www/manager6/Makefile                   |   1 +
>>   www/manager6/Utils.js                   |  48 +++-
>>   www/manager6/form/ControllerSelector.js |  65 +++---
>>   www/manager6/qemu/CreateWizard.js       |   4 +-
>>   www/manager6/qemu/HDEdit.js             | 143 ++++++++----
>>   www/manager6/qemu/MultiHDEdit.js        | 294 ++++++++++++++++++++++++
>>   www/manager6/qemu/OSTypeEdit.js         |   3 +-
>>   www/manager6/window/Wizard.js           |   7 +-
>>   8 files changed, 482 insertions(+), 83 deletions(-)
>>   create mode 100644 www/manager6/qemu/MultiHDEdit.js
>>
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 

Re: [pve-devel] partially-applied: [PATCH manager v2 00/12] multi tab disk panel & multi disk wizard

[Thomas Lamprecht]

On 19.10.21 15:04, Aaron Lauterer wrote:
>>
>> applied above, delayed below; mostly because I'd like to have some feedback from
>> others here and I'd also like to have the same for CT, to avoid a feature drift in
>> the wizards.
> 
> I took the last two patches for a quick spin, and I have to say that I do
> like the functionality. Yes it does change the way one interacts with the
> wizard quite a bit, but it really makes it so much easier to create VMs with
> no disks at all (useful when preparing for qm importdisk) or more than one
> disk.

Thanks for the feedback, appreciated, for the record a v3 from that series was already
available (as you've now seen yourself, just wanted to ensure other devs see that this
is the reason the remaining v2 patches will not be used)