public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH V5 pve-manager 0/2] fix #2822: add iscsi, lvm, lvmthin & zfs
@ 2022-09-23 12:46 Stefan Hrdlicka
  2022-09-23 12:46 ` [pve-devel] [PATCH V5 pve-manager 1/2] fix #2822: add iscsi, lvm, lvmthin & zfs storage for all cluster nodes Stefan Hrdlicka
  2022-09-23 12:46 ` [pve-devel] [PATCH V5 pve-manager 2/2] cleanup: "var" to "let", fix some indentation in related files Stefan Hrdlicka
  0 siblings, 2 replies; 4+ messages in thread
From: Stefan Hrdlicka @ 2022-09-23 12:46 UTC (permalink / raw)
  To: pve-devel

V1 -> V2:
# pve-storage
* removed because patch is not needed

# pve-manager (1/3)
* remove storage controller from V1
* added custom ComboBox with API URL & setNodeName function
* added scan node selection for iSCSI 
* scan node selection field no longer send to server

## (optional) pve-manager (2/3): cleanup related files
* var to let statement change
* some indentation

## ((very) optional) pve-manager (3/3): cleanup all var statements
* replaces all var with let statements


V2 -> V3:
# pve-manager (1/2)
* fix broken interface (broken in V2  )
* improve tooltip
* replace jNodeSelector function with class object
  (PVE.panel.StorageBaseWithNodeSelector)

# other things:
* removed very optional cleanup
* nothing changed for "Base storage" selector. It is still possible to
  select for example an iSCSI device only availabe on one node that
  isn't availabe on the other ones. I wasn't sure if this should be
  changed in this context as well.


V3 -> V4:
# pve-manager (1/2)
* "localhost" is set for "Scan Node"
* if another node is selcted and the "X" (clear) is used, localhost is
  set as the value again
* use lookupReference
* moved used template literals for building path strings


V4 -> V5:
# pve-manager (1/2)
* s/lookupReference/lookup/
* move ComboBoxSetStoreNode & StrageScanNodeSelector 
  to www/manager6/form
* move array pushes to initialization of array


Stefan Hrdlicka (2):
  fix #2822: add iscsi, lvm, lvmthin & zfs storage for all cluster nodes
  cleanup: "var" to "let", fix some indentation in related files

 www/manager6/Makefile                        |  2 +
 www/manager6/form/ComboBoxSetStoreNode.js    | 16 +++++
 www/manager6/form/StorageScanNodeSelector.js | 30 +++++++++
 www/manager6/storage/Base.js                 | 11 +--
 www/manager6/storage/IScsiEdit.js            | 38 ++++++++---
 www/manager6/storage/LVMEdit.js              | 43 +++++++++---
 www/manager6/storage/LvmThinEdit.js          | 70 ++++++++++++++------
 www/manager6/storage/ZFSPoolEdit.js          | 51 +++++++++-----
 8 files changed, 201 insertions(+), 60 deletions(-)
 create mode 100644 www/manager6/form/ComboBoxSetStoreNode.js
 create mode 100644 www/manager6/form/StorageScanNodeSelector.js

-- 
2.30.2





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

* [pve-devel] [PATCH V5 pve-manager 1/2] fix #2822: add iscsi, lvm, lvmthin & zfs storage for all cluster nodes
  2022-09-23 12:46 [pve-devel] [PATCH V5 pve-manager 0/2] fix #2822: add iscsi, lvm, lvmthin & zfs Stefan Hrdlicka
@ 2022-09-23 12:46 ` Stefan Hrdlicka
  2022-09-29 12:02   ` Dominik Csapak
  2022-09-23 12:46 ` [pve-devel] [PATCH V5 pve-manager 2/2] cleanup: "var" to "let", fix some indentation in related files Stefan Hrdlicka
  1 sibling, 1 reply; 4+ messages in thread
From: Stefan Hrdlicka @ 2022-09-23 12:46 UTC (permalink / raw)
  To: pve-devel

This adds a dropdown box for iSCSI, LVM, LVMThin & ZFS storage options where a
cluster node needs to be chosen. As default the current node is
selected. It restricts the the storage to be only availabe on the
selected node.

Signed-off-by: Stefan Hrdlicka <s.hrdlicka@proxmox.com>
---
 www/manager6/Makefile                        |  2 +
 www/manager6/form/ComboBoxSetStoreNode.js    | 16 ++++++
 www/manager6/form/StorageScanNodeSelector.js | 30 +++++++++++
 www/manager6/storage/Base.js                 |  1 +
 www/manager6/storage/IScsiEdit.js            | 32 +++++++++---
 www/manager6/storage/LVMEdit.js              | 29 +++++++++--
 www/manager6/storage/LvmThinEdit.js          | 52 +++++++++++++++-----
 www/manager6/storage/ZFSPoolEdit.js          | 28 +++++++++--
 8 files changed, 166 insertions(+), 24 deletions(-)
 create mode 100644 www/manager6/form/ComboBoxSetStoreNode.js
 create mode 100644 www/manager6/form/StorageScanNodeSelector.js

diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index d16770b1..81f5e5d8 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -27,6 +27,7 @@ JSSRC= 							\
 	form/CalendarEvent.js				\
 	form/CephPoolSelector.js			\
 	form/CephFSSelector.js				\
+	form/ComboBoxSetStoreNode.js			\
 	form/CompressionSelector.js			\
 	form/ContentTypeSelector.js			\
 	form/ControllerSelector.js			\
@@ -62,6 +63,7 @@ JSSRC= 							\
 	form/SecurityGroupSelector.js			\
 	form/SnapshotSelector.js			\
 	form/SpiceEnhancementSelector.js		\
+	form/StorageScanNodeSelector.js	    		\
 	form/StorageSelector.js				\
 	form/TFASelector.js				\
 	form/TokenSelector.js 				\
diff --git a/www/manager6/form/ComboBoxSetStoreNode.js b/www/manager6/form/ComboBoxSetStoreNode.js
new file mode 100644
index 00000000..3490ddd7
--- /dev/null
+++ b/www/manager6/form/ComboBoxSetStoreNode.js
@@ -0,0 +1,16 @@
+Ext.define('PVE.form.ComboBoxSetStoreNode', {
+    extend: 'Ext.form.field.ComboBox',
+    config: {
+	apiBaseUrl: '/api2/json/nodes/',
+	apiSuffix: '',
+    },
+
+    setNodeName: function(value) {
+	let me = this;
+	value ||= Proxmox.NodeName;
+
+	me.getStore().getProxy().setUrl(`${me.apiBaseUrl}${value}${me.apiSuffix}`);
+	this.clearValue();
+    },
+
+});
diff --git a/www/manager6/form/StorageScanNodeSelector.js b/www/manager6/form/StorageScanNodeSelector.js
new file mode 100644
index 00000000..80188707
--- /dev/null
+++ b/www/manager6/form/StorageScanNodeSelector.js
@@ -0,0 +1,30 @@
+Ext.define('PVE.form.StorageScanNodeSelector', {
+    extend: 'PVE.form.NodeSelector',
+    xtype: 'pveStorageScanNodeSelector',
+
+    name: 'storageScanNode',
+    itemId: 'pveStorageScanNodeSelector',
+    fieldLabel: gettext('Scan node'),
+    allowBlank: true,
+    disallowedNodes: undefined,
+    autoSelect: false,
+    submitValue: false,
+    value: 'localhost',
+    autoEl: {
+	tag: 'div',
+	'data-qtip': gettext('Scan for available storages on the selected node'),
+    },
+    triggers: {
+	clear: {
+	    handler: function() {
+		let me = this;
+		me.setValue('localhost');
+	    },
+	},
+    },
+    setValue: function(value) {
+	let me = this;
+	me.callParent([value]);
+	me.triggers.clear.setVisible(me.triggers.clear.isVisible() && value !== 'localhost');
+    },
+});
diff --git a/www/manager6/storage/Base.js b/www/manager6/storage/Base.js
index 7f6d7a09..1df7a8dd 100644
--- a/www/manager6/storage/Base.js
+++ b/www/manager6/storage/Base.js
@@ -36,6 +36,7 @@ Ext.define('PVE.panel.StorageBase', {
 	    {
 		xtype: 'pveNodeSelector',
 		name: 'nodes',
+		reference: 'storageNodeRestriction',
 		disabled: me.storageId === 'local',
 		fieldLabel: gettext('Nodes'),
 		emptyText: gettext('All') + ' (' + gettext('No restrictions') +')',
diff --git a/www/manager6/storage/IScsiEdit.js b/www/manager6/storage/IScsiEdit.js
index 2f35f882..393de3c3 100644
--- a/www/manager6/storage/IScsiEdit.js
+++ b/www/manager6/storage/IScsiEdit.js
@@ -1,5 +1,5 @@
 Ext.define('PVE.storage.IScsiScan', {
-    extend: 'Ext.form.field.ComboBox',
+    extend: 'PVE.form.ComboBoxSetStoreNode',
     alias: 'widget.pveIScsiScan',
 
     queryParam: 'portal',
@@ -10,6 +10,9 @@ Ext.define('PVE.storage.IScsiScan', {
 	loadingText: gettext('Scanning...'),
 	width: 350,
     },
+    config: {
+	apiSuffix: '/scan/iscsi',
+    },
     doRawQuery: function() {
 	// do nothing
     },
@@ -42,7 +45,7 @@ Ext.define('PVE.storage.IScsiScan', {
 	    fields: ['target', 'portal'],
 	    proxy: {
 		type: 'proxmox',
-		url: `/api2/json/nodes/${me.nodename}/scan/iscsi`,
+		url: `${me.apiBaseUrl}${me.nodename}${me.apiSuffix}`,
 	    },
 	});
 	store.sort('target', 'ASC');
@@ -78,6 +81,23 @@ Ext.define('PVE.storage.IScsiInputPanel', {
 	var me = this;
 
 	me.column1 = [
+	    {
+		xtype: 'pveStorageScanNodeSelector',
+		disabled: !me.isCreate,
+		hidden: !me.isCreate,
+		preferredValue: '',
+		allowBlank: true,
+		autoSelect: false,
+		listeners: {
+		    change: {
+			fn: function(field, value) {
+			    me.lookup('iScsiTargetScan').setNodeName(value);
+			    me.lookup('storageNodeRestriction')
+				.setValue(value === 'localhost' ? '' : value);
+			},
+		    },
+		},
+	    },
 	    {
 		xtype: me.isCreate ? 'textfield' : 'displayfield',
 		name: 'portal',
@@ -94,14 +114,14 @@ Ext.define('PVE.storage.IScsiInputPanel', {
 		    },
 		},
 	    },
-	    {
+	    Ext.createWidget(me.isCreate ? 'pveIScsiScan' : 'displayfield', {
 		readOnly: !me.isCreate,
-		xtype: me.isCreate ? 'pveIScsiScan' : 'displayfield',
 		name: 'target',
 		value: '',
-		fieldLabel: 'Target',
+		fieldLabel: gettext('Target'),
 		allowBlank: false,
-	    },
+		reference: 'iScsiTargetScan',
+	    }),
 	];
 
 	me.column2 = [
diff --git a/www/manager6/storage/LVMEdit.js b/www/manager6/storage/LVMEdit.js
index 2a9cd283..04d96786 100644
--- a/www/manager6/storage/LVMEdit.js
+++ b/www/manager6/storage/LVMEdit.js
@@ -1,10 +1,20 @@
 Ext.define('PVE.storage.VgSelector', {
-    extend: 'Ext.form.field.ComboBox',
+    extend: 'PVE.form.ComboBoxSetStoreNode',
     alias: 'widget.pveVgSelector',
     valueField: 'vg',
     displayField: 'vg',
     queryMode: 'local',
     editable: false,
+    config: {
+	apiSuffix: '/scan/lvm',
+    },
+
+    setNodeName: function(value) {
+	let me = this;
+	me.callParent([value]);
+	me.getStore().load();
+    },
+
     initComponent: function() {
 	var me = this;
 
@@ -17,7 +27,7 @@ Ext.define('PVE.storage.VgSelector', {
 	    fields: ['vg', 'size', 'free'],
 	    proxy: {
 		type: 'proxmox',
-		url: '/api2/json/nodes/' + me.nodename + '/scan/lvm',
+		url: `${me.apiBaseUrl}${me.nodename}${me.apiSuffix}`,
 	    },
 	});
 
@@ -103,11 +113,24 @@ Ext.define('PVE.storage.LVMInputPanel', {
 	});
 
 	if (me.isCreate) {
-	    var vgField = Ext.create('PVE.storage.VgSelector', {
+	    let vgField = Ext.create('PVE.storage.VgSelector', {
 		name: 'vgname',
 		fieldLabel: gettext('Volume group'),
+		reference: 'volumeGroupSelector',
 		allowBlank: false,
 	    });
+	    me.column1.push({
+	        xtype: 'pveStorageScanNodeSelector',
+	        listeners: {
+	            change: {
+			fn: function(field, value) {
+			    me.lookup('volumeGroupSelector').setNodeName(value);
+			    me.lookup('storageNodeRestriction')
+				.setValue(value === 'localhost' ? '' : value);
+			},
+		    },
+	        },
+	    });
 
 	    var baseField = Ext.createWidget('pveFileSelector', {
 		name: 'base',
diff --git a/www/manager6/storage/LvmThinEdit.js b/www/manager6/storage/LvmThinEdit.js
index 4eab7740..e9202ffa 100644
--- a/www/manager6/storage/LvmThinEdit.js
+++ b/www/manager6/storage/LvmThinEdit.js
@@ -1,5 +1,5 @@
 Ext.define('PVE.storage.TPoolSelector', {
-    extend: 'Ext.form.field.ComboBox',
+    extend: 'PVE.form.ComboBoxSetStoreNode',
     alias: 'widget.pveTPSelector',
 
     queryParam: 'vg',
@@ -7,6 +7,10 @@ Ext.define('PVE.storage.TPoolSelector', {
     displayField: 'lv',
     editable: false,
 
+    config: {
+	apiSuffix: '/scan/lvmthin',
+    },
+
     doRawQuery: function() {
 	// nothing
     },
@@ -40,7 +44,7 @@ Ext.define('PVE.storage.TPoolSelector', {
 	    fields: ['lv'],
 	    proxy: {
 		type: 'proxmox',
-		url: '/api2/json/nodes/' + me.nodename + '/scan/lvmthin',
+		url: `${me.apiBaseUrl}${me.nodename}${me.apiSuffix}`,
 	    },
 	});
 
@@ -58,13 +62,23 @@ Ext.define('PVE.storage.TPoolSelector', {
 });
 
 Ext.define('PVE.storage.BaseVGSelector', {
-    extend: 'Ext.form.field.ComboBox',
+    extend: 'PVE.form.ComboBoxSetStoreNode',
     alias: 'widget.pveBaseVGSelector',
 
     valueField: 'vg',
     displayField: 'vg',
     queryMode: 'local',
     editable: false,
+    config: {
+	apiSuffix: '/scan/lvm',
+    },
+
+    setNodeName: function(value) {
+	let me = this;
+	me.callParent([value]);
+	me.getStore().load();
+    },
+
     initComponent: function() {
 	var me = this;
 
@@ -77,7 +91,7 @@ Ext.define('PVE.storage.BaseVGSelector', {
 	    fields: ['vg', 'size', 'free'],
 	    proxy: {
 		type: 'proxmox',
-		url: '/api2/json/nodes/' + me.nodename + '/scan/lvm',
+		url: `${me.apiBaseUrl}${me.nodename}${me.apiSuffix}`,
 	    },
 	});
 
@@ -121,27 +135,41 @@ Ext.define('PVE.storage.LvmThinInputPanel', {
 	});
 
 	if (me.isCreate) {
-	    var vgField = Ext.create('PVE.storage.TPoolSelector', {
-		name: 'thinpool',
-		fieldLabel: gettext('Thin Pool'),
-		allowBlank: false,
+	    me.column1.push({
+	        xtype: 'pveStorageScanNodeSelector',
+	        listeners: {
+		    change: {
+			fn: function(field, value) {
+			    me.lookup('thinPoolSelector').setNodeName(value);
+			    me.lookup('volumeGroupSelector').setNodeName(value);
+			    me.lookup('storageNodeRestriction')
+				.setValue(value === 'localhost' ? '' : value);
+			},
+		    },
+		},
 	    });
 
-	    me.column1.push({
-		xtype: 'pveBaseVGSelector',
+	    me.column1.push(Ext.create('PVE.storage.BaseVGSelector', {
 		name: 'vgname',
 		fieldLabel: gettext('Volume group'),
+		reference: 'volumeGroupSelector',
 		listeners: {
 		    change: function(f, value) {
 			if (me.isCreate) {
+			    let vgField = me.lookup('thinPoolSelector');
 			    vgField.setVG(value);
 			    vgField.setValue('');
 			}
 		    },
 		},
-	    });
+	    }));
 
-	    me.column1.push(vgField);
+	    me.column1.push(Ext.create('PVE.storage.TPoolSelector', {
+		name: 'thinpool',
+		fieldLabel: gettext('Thin Pool'),
+		reference: 'thinPoolSelector',
+		allowBlank: false,
+	    }));
 	}
 
 	me.column1.push(vgnameField);
diff --git a/www/manager6/storage/ZFSPoolEdit.js b/www/manager6/storage/ZFSPoolEdit.js
index 8e689f0c..98034de9 100644
--- a/www/manager6/storage/ZFSPoolEdit.js
+++ b/www/manager6/storage/ZFSPoolEdit.js
@@ -1,5 +1,5 @@
 Ext.define('PVE.storage.ZFSPoolSelector', {
-    extend: 'Ext.form.field.ComboBox',
+    extend: 'PVE.form.ComboBoxSetStoreNode',
     alias: 'widget.pveZFSPoolSelector',
     valueField: 'pool',
     displayField: 'pool',
@@ -8,6 +8,16 @@ Ext.define('PVE.storage.ZFSPoolSelector', {
     listConfig: {
 	loadingText: gettext('Scanning...'),
     },
+    config: {
+	apiSuffix: '/scan/zfs',
+    },
+
+    setNodeName: function(value) {
+	let me = this;
+	me.callParent([value]);
+	me.getStore().load();
+    },
+
     initComponent: function() {
 	var me = this;
 
@@ -20,10 +30,9 @@ Ext.define('PVE.storage.ZFSPoolSelector', {
 	    fields: ['pool', 'size', 'free'],
 	    proxy: {
 		type: 'proxmox',
-		url: '/api2/json/nodes/' + me.nodename + '/scan/zfs',
+		url: `${me.apiBaseUrl}${me.nodename}${me.apiSuffix}`,
 	    },
 	});
-
 	store.sort('pool', 'ASC');
 
 	Ext.apply(me, {
@@ -45,9 +54,22 @@ Ext.define('PVE.storage.ZFSPoolInputPanel', {
 	me.column1 = [];
 
 	if (me.isCreate) {
+	    me.column1.push({
+	        xtype: 'pveStorageScanNodeSelector',
+	        listeners: {
+		    change: {
+			fn: function(field, value) {
+			    me.lookup('zfsPoolSelector').setNodeName(value);
+			    me.lookup('storageNodeRestriction')
+				.setValue(value === 'localhost' ? '' : value);
+			},
+		    },
+		},
+	    });
 	    me.column1.push(Ext.create('PVE.storage.ZFSPoolSelector', {
 		name: 'pool',
 		fieldLabel: gettext('ZFS Pool'),
+		reference: 'zfsPoolSelector',
 		allowBlank: false,
 	    }));
 	} else {
-- 
2.30.2





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

* [pve-devel] [PATCH V5 pve-manager 2/2] cleanup: "var" to "let", fix some indentation in related files
  2022-09-23 12:46 [pve-devel] [PATCH V5 pve-manager 0/2] fix #2822: add iscsi, lvm, lvmthin & zfs Stefan Hrdlicka
  2022-09-23 12:46 ` [pve-devel] [PATCH V5 pve-manager 1/2] fix #2822: add iscsi, lvm, lvmthin & zfs storage for all cluster nodes Stefan Hrdlicka
@ 2022-09-23 12:46 ` Stefan Hrdlicka
  1 sibling, 0 replies; 4+ messages in thread
From: Stefan Hrdlicka @ 2022-09-23 12:46 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Stefan Hrdlicka <s.hrdlicka@proxmox.com>
---
 www/manager6/storage/Base.js        | 10 +++++-----
 www/manager6/storage/IScsiEdit.js   |  6 +++---
 www/manager6/storage/LVMEdit.js     | 14 +++++++-------
 www/manager6/storage/LvmThinEdit.js | 18 +++++++++---------
 www/manager6/storage/ZFSPoolEdit.js | 23 +++++++++++------------
 5 files changed, 35 insertions(+), 36 deletions(-)

diff --git a/www/manager6/storage/Base.js b/www/manager6/storage/Base.js
index 1df7a8dd..c8c735f3 100644
--- a/www/manager6/storage/Base.js
+++ b/www/manager6/storage/Base.js
@@ -5,7 +5,7 @@ Ext.define('PVE.panel.StorageBase', {
     type: '',
 
     onGetValues: function(values) {
-	var me = this;
+	let me = this;
 
 	if (me.isCreate) {
 	    values.type = me.type;
@@ -20,7 +20,7 @@ Ext.define('PVE.panel.StorageBase', {
     },
 
     initComponent: function() {
-	var me = this;
+	let me = this;
 
 	me.column1.unshift({
 	    xtype: me.isCreate ? 'textfield' : 'displayfield',
@@ -88,7 +88,7 @@ Ext.define('PVE.storage.BaseEdit', {
     },
 
     initComponent: function() {
-	var me = this;
+	let me = this;
 
 	me.isCreate = !me.storageId;
 
@@ -148,8 +148,8 @@ Ext.define('PVE.storage.BaseEdit', {
 	if (!me.isCreate) {
 	    me.load({
 		success: function(response, options) {
-		    var values = response.result.data;
-		    var ctypes = values.content || '';
+		    let values = response.result.data;
+		    let ctypes = values.content || '';
 
 		    values.content = ctypes.split(',');
 
diff --git a/www/manager6/storage/IScsiEdit.js b/www/manager6/storage/IScsiEdit.js
index 393de3c3..775b58f8 100644
--- a/www/manager6/storage/IScsiEdit.js
+++ b/www/manager6/storage/IScsiEdit.js
@@ -64,7 +64,7 @@ Ext.define('PVE.storage.IScsiInputPanel', {
     onlineHelp: 'storage_open_iscsi',
 
     onGetValues: function(values) {
-	var me = this;
+	let me = this;
 
 	values.content = values.luns ? 'images' : 'none';
 	delete values.luns;
@@ -78,7 +78,7 @@ Ext.define('PVE.storage.IScsiInputPanel', {
     },
 
     initComponent: function() {
-	var me = this;
+	let me = this;
 
 	me.column1 = [
 	    {
@@ -107,7 +107,7 @@ Ext.define('PVE.storage.IScsiInputPanel', {
 		listeners: {
 		    change: function(f, value) {
 			if (me.isCreate) {
-			    var exportField = me.down('field[name=target]');
+			    let exportField = me.down('field[name=target]');
 			    exportField.setPortal(value);
 			    exportField.setValue('');
 			}
diff --git a/www/manager6/storage/LVMEdit.js b/www/manager6/storage/LVMEdit.js
index 04d96786..285a04ac 100644
--- a/www/manager6/storage/LVMEdit.js
+++ b/www/manager6/storage/LVMEdit.js
@@ -16,13 +16,13 @@ Ext.define('PVE.storage.VgSelector', {
     },
 
     initComponent: function() {
-	var me = this;
+	let me = this;
 
 	if (!me.nodename) {
 	    me.nodename = 'localhost';
 	}
 
-	var store = Ext.create('Ext.data.Store', {
+	let store = Ext.create('Ext.data.Store', {
 	    autoLoad: {}, // true,
 	    fields: ['vg', 'size', 'free'],
 	    proxy: {
@@ -55,9 +55,9 @@ Ext.define('PVE.storage.BaseStorageSelector', {
     valueField: 'storage',
     displayField: 'text',
     initComponent: function() {
-	var me = this;
+	let me = this;
 
-	var store = Ext.create('Ext.data.Store', {
+	let store = Ext.create('Ext.data.Store', {
 	    autoLoad: {
 		addRecords: true,
 		params: {
@@ -99,11 +99,11 @@ Ext.define('PVE.storage.LVMInputPanel', {
     onlineHelp: 'storage_lvm',
 
     initComponent: function() {
-	var me = this;
+	let me = this;
 
 	me.column1 = [];
 
-	var vgnameField = Ext.createWidget(me.isCreate ? 'textfield' : 'displayfield', {
+	let vgnameField = Ext.createWidget(me.isCreate ? 'textfield' : 'displayfield', {
 	    name: 'vgname',
 	    hidden: !!me.isCreate,
 	    disabled: !!me.isCreate,
@@ -132,7 +132,7 @@ Ext.define('PVE.storage.LVMInputPanel', {
 	        },
 	    });
 
-	    var baseField = Ext.createWidget('pveFileSelector', {
+	    let baseField = Ext.createWidget('pveFileSelector', {
 		name: 'base',
 		hidden: true,
 		disabled: true,
diff --git a/www/manager6/storage/LvmThinEdit.js b/www/manager6/storage/LvmThinEdit.js
index e9202ffa..39c00e1a 100644
--- a/www/manager6/storage/LvmThinEdit.js
+++ b/www/manager6/storage/LvmThinEdit.js
@@ -16,7 +16,7 @@ Ext.define('PVE.storage.TPoolSelector', {
     },
 
     onTriggerClick: function() {
-	var me = this;
+	let me = this;
 
 	if (!me.queryCaching || me.lastQuery !== me.vg) {
 	    me.store.removeAll();
@@ -28,19 +28,19 @@ Ext.define('PVE.storage.TPoolSelector', {
     },
 
     setVG: function(myvg) {
-	var me = this;
+	let me = this;
 
 	me.vg = myvg;
     },
 
     initComponent: function() {
-	var me = this;
+	let me = this;
 
 	if (!me.nodename) {
 	    me.nodename = 'localhost';
 	}
 
-	var store = Ext.create('Ext.data.Store', {
+	let store = Ext.create('Ext.data.Store', {
 	    fields: ['lv'],
 	    proxy: {
 		type: 'proxmox',
@@ -80,13 +80,13 @@ Ext.define('PVE.storage.BaseVGSelector', {
     },
 
     initComponent: function() {
-	var me = this;
+	let me = this;
 
 	if (!me.nodename) {
 	    me.nodename = 'localhost';
 	}
 
-	var store = Ext.create('Ext.data.Store', {
+	let store = Ext.create('Ext.data.Store', {
 	    autoLoad: {},
 	    fields: ['vg', 'size', 'free'],
 	    proxy: {
@@ -112,11 +112,11 @@ Ext.define('PVE.storage.LvmThinInputPanel', {
     onlineHelp: 'storage_lvmthin',
 
     initComponent: function() {
-	var me = this;
+	let me = this;
 
 	me.column1 = [];
 
-	var vgnameField = Ext.createWidget(me.isCreate ? 'textfield' : 'displayfield', {
+	let vgnameField = Ext.createWidget(me.isCreate ? 'textfield' : 'displayfield', {
 	    name: 'vgname',
 	    hidden: !!me.isCreate,
 	    disabled: !!me.isCreate,
@@ -125,7 +125,7 @@ Ext.define('PVE.storage.LvmThinInputPanel', {
 	    allowBlank: false,
 	});
 
-	var thinpoolField = Ext.createWidget(me.isCreate ? 'textfield' : 'displayfield', {
+	let thinpoolField = Ext.createWidget(me.isCreate ? 'textfield' : 'displayfield', {
 	    name: 'thinpool',
 	    hidden: !!me.isCreate,
 	    disabled: !!me.isCreate,
diff --git a/www/manager6/storage/ZFSPoolEdit.js b/www/manager6/storage/ZFSPoolEdit.js
index 98034de9..7398ee95 100644
--- a/www/manager6/storage/ZFSPoolEdit.js
+++ b/www/manager6/storage/ZFSPoolEdit.js
@@ -19,13 +19,13 @@ Ext.define('PVE.storage.ZFSPoolSelector', {
     },
 
     initComponent: function() {
-	var me = this;
+	let me = this;
 
 	if (!me.nodename) {
 	    me.nodename = 'localhost';
 	}
 
-	var store = Ext.create('Ext.data.Store', {
+	let store = Ext.create('Ext.data.Store', {
 	    autoLoad: {}, // true,
 	    fields: ['pool', 'size', 'free'],
 	    proxy: {
@@ -49,7 +49,7 @@ Ext.define('PVE.storage.ZFSPoolInputPanel', {
     onlineHelp: 'storage_zfspool',
 
     initComponent: function() {
-	var me = this;
+	let me = this;
 
 	me.column1 = [];
 
@@ -83,15 +83,14 @@ Ext.define('PVE.storage.ZFSPoolInputPanel', {
 
 	// value is an array,
 	// while before it was a string
-	me.column1.push(
-	    {
-xtype: 'pveContentTypeSelector',
-	     cts: ['images', 'rootdir'],
-	     fieldLabel: gettext('Content'),
-	     name: 'content',
-	     value: ['images', 'rootdir'],
-	     multiSelect: true,
-	     allowBlank: false,
+	me.column1.push({
+	    xtype: 'pveContentTypeSelector',
+	    cts: ['images', 'rootdir'],
+	    fieldLabel: gettext('Content'),
+	    name: 'content',
+	    value: ['images', 'rootdir'],
+	    multiSelect: true,
+	    allowBlank: false,
 	});
 	me.column2 = [
 	    {
-- 
2.30.2





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

* Re: [pve-devel] [PATCH V5 pve-manager 1/2] fix #2822: add iscsi, lvm, lvmthin & zfs storage for all cluster nodes
  2022-09-23 12:46 ` [pve-devel] [PATCH V5 pve-manager 1/2] fix #2822: add iscsi, lvm, lvmthin & zfs storage for all cluster nodes Stefan Hrdlicka
@ 2022-09-29 12:02   ` Dominik Csapak
  0 siblings, 0 replies; 4+ messages in thread
From: Dominik Csapak @ 2022-09-29 12:02 UTC (permalink / raw)
  To: Proxmox VE development discussion, Stefan Hrdlicka

thanks for the resend, i can now apply it ;)

i played a bit around with this and i'm not completely happy with the ux here.
having 'localhost' as text in the box is not really helpful because the
user might not actually know what 'localhost' actuall is.

also it's possible to empty the field, of which the meaning is also not really
clear?

my suggestion would be to set allowBlank to false, and by default select
the current nodename (available in Proxmox.NodeName)

That way the current node is preselected, and there is no misunderstanding
which node is scanned at any time.

i'd leave the restriction logic like it is though, because if the storage
is available everywhere, you don't have to change the nodename at all

but if you change it you want it probably restricted (like already
mentioned in a previous review)

aside from that the patches LGTM

On 9/23/22 14:46, Stefan Hrdlicka wrote:
> This adds a dropdown box for iSCSI, LVM, LVMThin & ZFS storage options where a
> cluster node needs to be chosen. As default the current node is
> selected. It restricts the the storage to be only availabe on the
> selected node.
> 
> Signed-off-by: Stefan Hrdlicka <s.hrdlicka@proxmox.com>
> ---
>   www/manager6/Makefile                        |  2 +
>   www/manager6/form/ComboBoxSetStoreNode.js    | 16 ++++++
>   www/manager6/form/StorageScanNodeSelector.js | 30 +++++++++++
>   www/manager6/storage/Base.js                 |  1 +
>   www/manager6/storage/IScsiEdit.js            | 32 +++++++++---
>   www/manager6/storage/LVMEdit.js              | 29 +++++++++--
>   www/manager6/storage/LvmThinEdit.js          | 52 +++++++++++++++-----
>   www/manager6/storage/ZFSPoolEdit.js          | 28 +++++++++--
>   8 files changed, 166 insertions(+), 24 deletions(-)
>   create mode 100644 www/manager6/form/ComboBoxSetStoreNode.js
>   create mode 100644 www/manager6/form/StorageScanNodeSelector.js
> 
> diff --git a/www/manager6/Makefile b/www/manager6/Makefile
> index d16770b1..81f5e5d8 100644
> --- a/www/manager6/Makefile
> +++ b/www/manager6/Makefile
> @@ -27,6 +27,7 @@ JSSRC= 							\
>   	form/CalendarEvent.js				\
>   	form/CephPoolSelector.js			\
>   	form/CephFSSelector.js				\
> +	form/ComboBoxSetStoreNode.js			\
>   	form/CompressionSelector.js			\
>   	form/ContentTypeSelector.js			\
>   	form/ControllerSelector.js			\
> @@ -62,6 +63,7 @@ JSSRC= 							\
>   	form/SecurityGroupSelector.js			\
>   	form/SnapshotSelector.js			\
>   	form/SpiceEnhancementSelector.js		\
> +	form/StorageScanNodeSelector.js	    		\
>   	form/StorageSelector.js				\
>   	form/TFASelector.js				\
>   	form/TokenSelector.js 				\
> diff --git a/www/manager6/form/ComboBoxSetStoreNode.js b/www/manager6/form/ComboBoxSetStoreNode.js
> new file mode 100644
> index 00000000..3490ddd7
> --- /dev/null
> +++ b/www/manager6/form/ComboBoxSetStoreNode.js
> @@ -0,0 +1,16 @@
> +Ext.define('PVE.form.ComboBoxSetStoreNode', {
> +    extend: 'Ext.form.field.ComboBox',
> +    config: {
> +	apiBaseUrl: '/api2/json/nodes/',
> +	apiSuffix: '',
> +    },
> +
> +    setNodeName: function(value) {
> +	let me = this;
> +	value ||= Proxmox.NodeName;
> +
> +	me.getStore().getProxy().setUrl(`${me.apiBaseUrl}${value}${me.apiSuffix}`);
> +	this.clearValue();
> +    },
> +
> +});
> diff --git a/www/manager6/form/StorageScanNodeSelector.js b/www/manager6/form/StorageScanNodeSelector.js
> new file mode 100644
> index 00000000..80188707
> --- /dev/null
> +++ b/www/manager6/form/StorageScanNodeSelector.js
> @@ -0,0 +1,30 @@
> +Ext.define('PVE.form.StorageScanNodeSelector', {
> +    extend: 'PVE.form.NodeSelector',
> +    xtype: 'pveStorageScanNodeSelector',
> +
> +    name: 'storageScanNode',
> +    itemId: 'pveStorageScanNodeSelector',
> +    fieldLabel: gettext('Scan node'),
> +    allowBlank: true,
> +    disallowedNodes: undefined,
> +    autoSelect: false,
> +    submitValue: false,
> +    value: 'localhost',
> +    autoEl: {
> +	tag: 'div',
> +	'data-qtip': gettext('Scan for available storages on the selected node'),
> +    },
> +    triggers: {
> +	clear: {
> +	    handler: function() {
> +		let me = this;
> +		me.setValue('localhost');
> +	    },
> +	},
> +    },
> +    setValue: function(value) {
> +	let me = this;
> +	me.callParent([value]);
> +	me.triggers.clear.setVisible(me.triggers.clear.isVisible() && value !== 'localhost');
> +    },
> +});
> diff --git a/www/manager6/storage/Base.js b/www/manager6/storage/Base.js
> index 7f6d7a09..1df7a8dd 100644
> --- a/www/manager6/storage/Base.js
> +++ b/www/manager6/storage/Base.js
> @@ -36,6 +36,7 @@ Ext.define('PVE.panel.StorageBase', {
>   	    {
>   		xtype: 'pveNodeSelector',
>   		name: 'nodes',
> +		reference: 'storageNodeRestriction',
>   		disabled: me.storageId === 'local',
>   		fieldLabel: gettext('Nodes'),
>   		emptyText: gettext('All') + ' (' + gettext('No restrictions') +')',
> diff --git a/www/manager6/storage/IScsiEdit.js b/www/manager6/storage/IScsiEdit.js
> index 2f35f882..393de3c3 100644
> --- a/www/manager6/storage/IScsiEdit.js
> +++ b/www/manager6/storage/IScsiEdit.js
> @@ -1,5 +1,5 @@
>   Ext.define('PVE.storage.IScsiScan', {
> -    extend: 'Ext.form.field.ComboBox',
> +    extend: 'PVE.form.ComboBoxSetStoreNode',
>       alias: 'widget.pveIScsiScan',
>   
>       queryParam: 'portal',
> @@ -10,6 +10,9 @@ Ext.define('PVE.storage.IScsiScan', {
>   	loadingText: gettext('Scanning...'),
>   	width: 350,
>       },
> +    config: {
> +	apiSuffix: '/scan/iscsi',
> +    },
>       doRawQuery: function() {
>   	// do nothing
>       },
> @@ -42,7 +45,7 @@ Ext.define('PVE.storage.IScsiScan', {
>   	    fields: ['target', 'portal'],
>   	    proxy: {
>   		type: 'proxmox',
> -		url: `/api2/json/nodes/${me.nodename}/scan/iscsi`,
> +		url: `${me.apiBaseUrl}${me.nodename}${me.apiSuffix}`,
>   	    },
>   	});
>   	store.sort('target', 'ASC');
> @@ -78,6 +81,23 @@ Ext.define('PVE.storage.IScsiInputPanel', {
>   	var me = this;
>   
>   	me.column1 = [
> +	    {
> +		xtype: 'pveStorageScanNodeSelector',
> +		disabled: !me.isCreate,
> +		hidden: !me.isCreate,
> +		preferredValue: '',
> +		allowBlank: true,
> +		autoSelect: false,
> +		listeners: {
> +		    change: {
> +			fn: function(field, value) {
> +			    me.lookup('iScsiTargetScan').setNodeName(value);
> +			    me.lookup('storageNodeRestriction')
> +				.setValue(value === 'localhost' ? '' : value);
> +			},
> +		    },
> +		},
> +	    },
>   	    {
>   		xtype: me.isCreate ? 'textfield' : 'displayfield',
>   		name: 'portal',
> @@ -94,14 +114,14 @@ Ext.define('PVE.storage.IScsiInputPanel', {
>   		    },
>   		},
>   	    },
> -	    {
> +	    Ext.createWidget(me.isCreate ? 'pveIScsiScan' : 'displayfield', {
>   		readOnly: !me.isCreate,
> -		xtype: me.isCreate ? 'pveIScsiScan' : 'displayfield',
>   		name: 'target',
>   		value: '',
> -		fieldLabel: 'Target',
> +		fieldLabel: gettext('Target'),
>   		allowBlank: false,
> -	    },
> +		reference: 'iScsiTargetScan',
> +	    }),
>   	];
>   
>   	me.column2 = [
> diff --git a/www/manager6/storage/LVMEdit.js b/www/manager6/storage/LVMEdit.js
> index 2a9cd283..04d96786 100644
> --- a/www/manager6/storage/LVMEdit.js
> +++ b/www/manager6/storage/LVMEdit.js
> @@ -1,10 +1,20 @@
>   Ext.define('PVE.storage.VgSelector', {
> -    extend: 'Ext.form.field.ComboBox',
> +    extend: 'PVE.form.ComboBoxSetStoreNode',
>       alias: 'widget.pveVgSelector',
>       valueField: 'vg',
>       displayField: 'vg',
>       queryMode: 'local',
>       editable: false,
> +    config: {
> +	apiSuffix: '/scan/lvm',
> +    },
> +
> +    setNodeName: function(value) {
> +	let me = this;
> +	me.callParent([value]);
> +	me.getStore().load();
> +    },
> +
>       initComponent: function() {
>   	var me = this;
>   
> @@ -17,7 +27,7 @@ Ext.define('PVE.storage.VgSelector', {
>   	    fields: ['vg', 'size', 'free'],
>   	    proxy: {
>   		type: 'proxmox',
> -		url: '/api2/json/nodes/' + me.nodename + '/scan/lvm',
> +		url: `${me.apiBaseUrl}${me.nodename}${me.apiSuffix}`,
>   	    },
>   	});
>   
> @@ -103,11 +113,24 @@ Ext.define('PVE.storage.LVMInputPanel', {
>   	});
>   
>   	if (me.isCreate) {
> -	    var vgField = Ext.create('PVE.storage.VgSelector', {
> +	    let vgField = Ext.create('PVE.storage.VgSelector', {
>   		name: 'vgname',
>   		fieldLabel: gettext('Volume group'),
> +		reference: 'volumeGroupSelector',
>   		allowBlank: false,
>   	    });
> +	    me.column1.push({
> +	        xtype: 'pveStorageScanNodeSelector',
> +	        listeners: {
> +	            change: {
> +			fn: function(field, value) {
> +			    me.lookup('volumeGroupSelector').setNodeName(value);
> +			    me.lookup('storageNodeRestriction')
> +				.setValue(value === 'localhost' ? '' : value);
> +			},
> +		    },
> +	        },
> +	    });
>   
>   	    var baseField = Ext.createWidget('pveFileSelector', {
>   		name: 'base',
> diff --git a/www/manager6/storage/LvmThinEdit.js b/www/manager6/storage/LvmThinEdit.js
> index 4eab7740..e9202ffa 100644
> --- a/www/manager6/storage/LvmThinEdit.js
> +++ b/www/manager6/storage/LvmThinEdit.js
> @@ -1,5 +1,5 @@
>   Ext.define('PVE.storage.TPoolSelector', {
> -    extend: 'Ext.form.field.ComboBox',
> +    extend: 'PVE.form.ComboBoxSetStoreNode',
>       alias: 'widget.pveTPSelector',
>   
>       queryParam: 'vg',
> @@ -7,6 +7,10 @@ Ext.define('PVE.storage.TPoolSelector', {
>       displayField: 'lv',
>       editable: false,
>   
> +    config: {
> +	apiSuffix: '/scan/lvmthin',
> +    },
> +
>       doRawQuery: function() {
>   	// nothing
>       },
> @@ -40,7 +44,7 @@ Ext.define('PVE.storage.TPoolSelector', {
>   	    fields: ['lv'],
>   	    proxy: {
>   		type: 'proxmox',
> -		url: '/api2/json/nodes/' + me.nodename + '/scan/lvmthin',
> +		url: `${me.apiBaseUrl}${me.nodename}${me.apiSuffix}`,
>   	    },
>   	});
>   
> @@ -58,13 +62,23 @@ Ext.define('PVE.storage.TPoolSelector', {
>   });
>   
>   Ext.define('PVE.storage.BaseVGSelector', {
> -    extend: 'Ext.form.field.ComboBox',
> +    extend: 'PVE.form.ComboBoxSetStoreNode',
>       alias: 'widget.pveBaseVGSelector',
>   
>       valueField: 'vg',
>       displayField: 'vg',
>       queryMode: 'local',
>       editable: false,
> +    config: {
> +	apiSuffix: '/scan/lvm',
> +    },
> +
> +    setNodeName: function(value) {
> +	let me = this;
> +	me.callParent([value]);
> +	me.getStore().load();
> +    },
> +
>       initComponent: function() {
>   	var me = this;
>   
> @@ -77,7 +91,7 @@ Ext.define('PVE.storage.BaseVGSelector', {
>   	    fields: ['vg', 'size', 'free'],
>   	    proxy: {
>   		type: 'proxmox',
> -		url: '/api2/json/nodes/' + me.nodename + '/scan/lvm',
> +		url: `${me.apiBaseUrl}${me.nodename}${me.apiSuffix}`,
>   	    },
>   	});
>   
> @@ -121,27 +135,41 @@ Ext.define('PVE.storage.LvmThinInputPanel', {
>   	});
>   
>   	if (me.isCreate) {
> -	    var vgField = Ext.create('PVE.storage.TPoolSelector', {
> -		name: 'thinpool',
> -		fieldLabel: gettext('Thin Pool'),
> -		allowBlank: false,
> +	    me.column1.push({
> +	        xtype: 'pveStorageScanNodeSelector',
> +	        listeners: {
> +		    change: {
> +			fn: function(field, value) {
> +			    me.lookup('thinPoolSelector').setNodeName(value);
> +			    me.lookup('volumeGroupSelector').setNodeName(value);
> +			    me.lookup('storageNodeRestriction')
> +				.setValue(value === 'localhost' ? '' : value);
> +			},
> +		    },
> +		},
>   	    });
>   
> -	    me.column1.push({
> -		xtype: 'pveBaseVGSelector',
> +	    me.column1.push(Ext.create('PVE.storage.BaseVGSelector', {
>   		name: 'vgname',
>   		fieldLabel: gettext('Volume group'),
> +		reference: 'volumeGroupSelector',
>   		listeners: {
>   		    change: function(f, value) {
>   			if (me.isCreate) {
> +			    let vgField = me.lookup('thinPoolSelector');
>   			    vgField.setVG(value);
>   			    vgField.setValue('');
>   			}
>   		    },
>   		},
> -	    });
> +	    }));
>   
> -	    me.column1.push(vgField);
> +	    me.column1.push(Ext.create('PVE.storage.TPoolSelector', {
> +		name: 'thinpool',
> +		fieldLabel: gettext('Thin Pool'),
> +		reference: 'thinPoolSelector',
> +		allowBlank: false,
> +	    }));
>   	}
>   
>   	me.column1.push(vgnameField);
> diff --git a/www/manager6/storage/ZFSPoolEdit.js b/www/manager6/storage/ZFSPoolEdit.js
> index 8e689f0c..98034de9 100644
> --- a/www/manager6/storage/ZFSPoolEdit.js
> +++ b/www/manager6/storage/ZFSPoolEdit.js
> @@ -1,5 +1,5 @@
>   Ext.define('PVE.storage.ZFSPoolSelector', {
> -    extend: 'Ext.form.field.ComboBox',
> +    extend: 'PVE.form.ComboBoxSetStoreNode',
>       alias: 'widget.pveZFSPoolSelector',
>       valueField: 'pool',
>       displayField: 'pool',
> @@ -8,6 +8,16 @@ Ext.define('PVE.storage.ZFSPoolSelector', {
>       listConfig: {
>   	loadingText: gettext('Scanning...'),
>       },
> +    config: {
> +	apiSuffix: '/scan/zfs',
> +    },
> +
> +    setNodeName: function(value) {
> +	let me = this;
> +	me.callParent([value]);
> +	me.getStore().load();
> +    },
> +
>       initComponent: function() {
>   	var me = this;
>   
> @@ -20,10 +30,9 @@ Ext.define('PVE.storage.ZFSPoolSelector', {
>   	    fields: ['pool', 'size', 'free'],
>   	    proxy: {
>   		type: 'proxmox',
> -		url: '/api2/json/nodes/' + me.nodename + '/scan/zfs',
> +		url: `${me.apiBaseUrl}${me.nodename}${me.apiSuffix}`,
>   	    },
>   	});
> -
>   	store.sort('pool', 'ASC');
>   
>   	Ext.apply(me, {
> @@ -45,9 +54,22 @@ Ext.define('PVE.storage.ZFSPoolInputPanel', {
>   	me.column1 = [];
>   
>   	if (me.isCreate) {
> +	    me.column1.push({
> +	        xtype: 'pveStorageScanNodeSelector',
> +	        listeners: {
> +		    change: {
> +			fn: function(field, value) {
> +			    me.lookup('zfsPoolSelector').setNodeName(value);
> +			    me.lookup('storageNodeRestriction')
> +				.setValue(value === 'localhost' ? '' : value);
> +			},
> +		    },
> +		},
> +	    });
>   	    me.column1.push(Ext.create('PVE.storage.ZFSPoolSelector', {
>   		name: 'pool',
>   		fieldLabel: gettext('ZFS Pool'),
> +		reference: 'zfsPoolSelector',
>   		allowBlank: false,
>   	    }));
>   	} else {





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

end of thread, other threads:[~2022-09-29 12:02 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-23 12:46 [pve-devel] [PATCH V5 pve-manager 0/2] fix #2822: add iscsi, lvm, lvmthin & zfs Stefan Hrdlicka
2022-09-23 12:46 ` [pve-devel] [PATCH V5 pve-manager 1/2] fix #2822: add iscsi, lvm, lvmthin & zfs storage for all cluster nodes Stefan Hrdlicka
2022-09-29 12:02   ` Dominik Csapak
2022-09-23 12:46 ` [pve-devel] [PATCH V5 pve-manager 2/2] cleanup: "var" to "let", fix some indentation in related files Stefan Hrdlicka

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