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

[pve-devel] [PATCH manager 00/10] 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.

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 (10):
  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: add qemu/MultiHDEdit and use it in the wizard

 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        | 291 ++++++++++++++++++++++++
 www/manager6/qemu/OSTypeEdit.js         |   2 +-
 www/manager6/window/Wizard.js           |   5 +-
 8 files changed, 477 insertions(+), 82 deletions(-)
 create mode 100644 www/manager6/qemu/MultiHDEdit.js

-- 
2.30.2

[pve-devel] [PATCH manager 01/10] 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 02/10] 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 03/10] 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 04/10] 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 05/10] 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 06/10] 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 07/10] 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 08/10] 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 09/10] 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 10/10] 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  | 291 ++++++++++++++++++++++++++++++
 4 files changed, 301 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..079a6fc6
--- /dev/null
+++ b/www/manager6/qemu/MultiHDEdit.js
@@ -0,0 +1,291 @@
+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 vmconfig = {
+		ide2: 'media=cdrom',
+		scsihw: me.getViewModel().get('current.scsihw'),
+	    };
+
+	    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: 150,
+	    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: 4,
+			},
+			{
+			    flex: 1,
+			    xtype: 'actioncolumn',
+			    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',
+		    value: true,
+		    validator: (val) => !!val,
+		},
+	    ],
+	},
+    ],
+});
-- 
2.30.2

Re: [pve-devel] [PATCH manager 00/10] multi tab disk panel & multi disk wizard

[Lorenz Stechauner]

Hi,

tested this series a bit. everything seems to work as advertised, except 
the VM creation wizard. After submitting I get an error:

Parameter verification failed. (400)
textfield-4006-inputEl: property is not defined in schema and the schema 
does not allow additional properties

other small detail: width of the Bus/Device field for the bus nr. is too 
short to display two digit numbers.

On 20.09.21 14:23, 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.
>
> 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 (10):
>    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: add qemu/MultiHDEdit and use it in the wizard
>
>   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        | 291 ++++++++++++++++++++++++
>   www/manager6/qemu/OSTypeEdit.js         |   2 +-
>   www/manager6/window/Wizard.js           |   5 +-
>   8 files changed, 477 insertions(+), 82 deletions(-)
>   create mode 100644 www/manager6/qemu/MultiHDEdit.js
>