[pve-devel] [PATCH v4 manager 0/6] ui: lxc/qemu: add reassign for disks and volumes

[pve-devel] [PATCH v4 manager 0/6] ui: lxc/qemu: add reassign for disks and volumes

[Aaron Lauterer]

This series adds the UI to reassign a disk / volume from one guest to another.

To avoid button clutter, the Move, Reassing and Resize buttons are moved
into a new submenu called "Disk/Volume Action".

Patch 3 to 6 are optional. Patch 3 changes the labels for Move, Reassign
and Resize to remove Volume & Disk as we already have this in the
context of the submenu.

Patch 4 only changes a double negated option.
Patch 5 happend in the process of working on an interface for the
reassign functionality. Since the work of modernizing this componend is
done, why not use it

Patch 6 changes how we store the max number of MPs possible because I
ran into the issue, that I cannot easily match against the object key if
it is 'mps' instead of 'mp'.

v4:
* add PVE.Util.nextFreeMP helper
* filter templates in the reassign target guest list
* code cleanup on multiple locations
* fix padding

v3:
* change to Edit window, removing quite some boilerplate code
* create new submenu for disk/volume actions
* incorporate smaller style nits
* simplify other labels as well, removing 'Volume' and 'Disk' as the
  context gives that away already

v2: incorporated feedback I got off list, mainly
* using more modern approaches
* more arrow functions
* reducing use of predefined cbind values and using inline functions
  when possible

Aaron Lauterer (6):
  ui: utils: add nextFreeMP
  ui: lxc/qemu: add disk reassign and action submenu
  ui: lxc/qemu: disk/volume action simplify menu items
  ui: BusTypeSelector: change noVirtIO to withVirtIO
  ui: hdmove: modernize/refactor
  ui: util: refactor mps to mp

 www/manager6/Makefile                   |   1 +
 www/manager6/Utils.js                   |  19 +-
 www/manager6/form/BusTypeSelector.js    |   4 +-
 www/manager6/form/ControllerSelector.js |   4 +-
 www/manager6/lxc/MPEdit.js              |   2 +-
 www/manager6/lxc/MultiMPEdit.js         |   4 +-
 www/manager6/lxc/Resources.js           |  66 ++++--
 www/manager6/qemu/CDEdit.js             |   2 +-
 www/manager6/qemu/CIDriveEdit.js        |   2 +-
 www/manager6/qemu/HDMove.js             | 185 +++++++---------
 www/manager6/qemu/HDReassign.js         | 272 ++++++++++++++++++++++++
 www/manager6/qemu/HardwareView.js       |  69 ++++--
 12 files changed, 490 insertions(+), 140 deletions(-)
 create mode 100644 www/manager6/qemu/HDReassign.js

-- 
2.30.2

[pve-devel] [PATCH v4 manager 1/6] ui: utils: add nextFreeMP

[Aaron Lauterer]

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
 www/manager6/Utils.js | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index aafe359a..5190f750 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -1803,6 +1803,23 @@ Ext.define('PVE.Utils', {
 
 	return undefined;
     },
+
+    nextFreeMP: function(type, config) {
+	let mptype = type === "mp" ? "mps" : type;
+
+	for (let i = 0; i < PVE.Utils.mp_counts[mptype]; i++) {
+	    let confid = `${type}${i}`;
+	    if (!Ext.isDefined(config[confid])) {
+		return {
+		    type,
+		    id: i,
+		    confid,
+		};
+	    }
+	}
+
+	return undefined;
+    },
 },
 
     singleton: true,
-- 
2.30.2

[pve-devel] [PATCH v4 manager 2/6] ui: lxc/qemu: add disk reassign and action submenu

[Aaron Lauterer]

For the new HDReassign component, we follow the approach of HDMove to
have one componend for qemu and lxc.

To avoid button clutter, a new "Disk/Volume action" button is
introduced. It holds the Move, Reassign and Resize buttons in a submenu.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
changes since
v3:
* use PVE.Util.nextFreeMP
* create 'url' in one place, no need for submitURL
* filter out templates in target guest selection
* fix padding
* code cleanup changes

v2:
* switch from generic to Proxmox Edit window
* add new submenu for disk/volume specific actions
* code style improvements
* simplify some labels, removing "disk" and "volume" as the context
  already gives this away


v1: incorporated feedback I got off list

 www/manager6/Makefile             |   1 +
 www/manager6/lxc/Resources.js     |  62 +++++--
 www/manager6/qemu/HDReassign.js   | 272 ++++++++++++++++++++++++++++++
 www/manager6/qemu/HardwareView.js |  64 +++++--
 4 files changed, 378 insertions(+), 21 deletions(-)
 create mode 100644 www/manager6/qemu/HDReassign.js

diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index e6e01bd1..a7101553 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -213,6 +213,7 @@ JSSRC= 							\
 	qemu/HDEfi.js					\
 	qemu/HDTPM.js					\
 	qemu/HDMove.js					\
+	qemu/HDReassign.js				\
 	qemu/HDResize.js				\
 	qemu/HardwareView.js				\
 	qemu/IPConfigEdit.js				\
diff --git a/www/manager6/lxc/Resources.js b/www/manager6/lxc/Resources.js
index 15ee3c67..4d47679d 100644
--- a/www/manager6/lxc/Resources.js
+++ b/www/manager6/lxc/Resources.js
@@ -151,7 +151,8 @@ Ext.define('PVE.lxc.RessourceView', {
 	    });
 	};
 
-	var run_move = function(b, e, rec) {
+	let run_move = function() {
+	    let rec = me.selModel.getSelection()[0];
 	    if (!rec) {
 		return;
 	    }
@@ -168,6 +169,24 @@ Ext.define('PVE.lxc.RessourceView', {
 	    win.on('destroy', me.reload, me);
 	};
 
+	let run_reassign = function() {
+	    let rec = me.selModel.getSelection()[0];
+	    if (!rec) {
+		return;
+	    }
+
+	    Ext.create('PVE.window.HDReassign', {
+		disk: rec.data.key,
+		nodename: nodename,
+		autoShow: true,
+		vmid: vmid,
+		type: 'lxc',
+		listeners: {
+		    destroy: () => me.reload(),
+		},
+	    });
+	};
+
 	var edit_btn = new Proxmox.button.Button({
 	    text: gettext('Edit'),
 	    selModel: me.selModel,
@@ -182,7 +201,7 @@ Ext.define('PVE.lxc.RessourceView', {
 	    handler: function() { me.run_editor(); },
 	});
 
-	var resize_btn = new Proxmox.button.Button({
+	let resize_menuitem = new Ext.menu.Item({
 	    text: gettext('Resize disk'),
 	    selModel: me.selModel,
 	    disabled: true,
@@ -227,14 +246,34 @@ Ext.define('PVE.lxc.RessourceView', {
 	    },
 	});
 
-	var move_btn = new Proxmox.button.Button({
+	let reassign_menuitem = new Ext.menu.Item({
+	    text: gettext('Reassign Volume'),
+	    tooltip: gettext('Reassign volume to another CT'),
+	    handler: run_reassign,
+	    reference: 'reassing_item',
+	    disabled: true,
+	});
+
+	let move_menuitem = new Ext.menu.Item({
 	    text: gettext('Move Volume'),
 	    selModel: me.selModel,
 	    disabled: true,
-	    dangerous: true,
 	    handler: run_move,
 	});
 
+	let volumeaction_btn = new Proxmox.button.Button({
+	    text: gettext('Volume Action'),
+	    disabled: true,
+	    menu: {
+		plain: true,
+		items: [
+		    move_menuitem,
+		    reassign_menuitem,
+		    resize_menuitem,
+		],
+	    },
+	});
+
 	var revert_btn = new PVE.button.PendingRevert();
 
 	var set_button_status = function() {
@@ -243,7 +282,7 @@ Ext.define('PVE.lxc.RessourceView', {
 	    if (!rec) {
 		edit_btn.disable();
 		remove_btn.disable();
-		resize_btn.disable();
+		volumeaction_btn.disable();
 		revert_btn.disable();
 		return;
 	    }
@@ -253,6 +292,7 @@ Ext.define('PVE.lxc.RessourceView', {
 
 	    var pending = rec.data.delete || me.hasPendingChanges(key);
 	    var isDisk = rowdef.tdCls === 'pve-itype-icon-storage';
+	    let isRootFS = rec.data.key === 'rootfs';
 	    var isUnusedDisk = key.match(/^unused\d+/);
 	    var isUsedDisk = isDisk && !isUnusedDisk;
 
@@ -265,9 +305,12 @@ Ext.define('PVE.lxc.RessourceView', {
 	    }
 	    edit_btn.setDisabled(noedit);
 
-	    remove_btn.setDisabled(!isDisk || rec.data.key === 'rootfs' || !diskCap || pending);
-	    resize_btn.setDisabled(!isDisk || !diskCap || isUnusedDisk);
-	    move_btn.setDisabled(!isDisk || !diskCap);
+	    volumeaction_btn.setDisabled(!isDisk);
+	    reassign_menuitem.setDisabled(isRootFS);
+
+	    remove_btn.setDisabled(!isDisk || isRootFS || !diskCap || pending);
+	    resize_menuitem.setDisabled(!isDisk || !diskCap || isUnusedDisk);
+	    move_menuitem.setDisabled(!isDisk || !diskCap);
 	    revert_btn.setDisabled(!pending);
 
 	    remove_btn.setText(isUsedDisk ? remove_btn.altText : remove_btn.defaultText);
@@ -327,8 +370,7 @@ Ext.define('PVE.lxc.RessourceView', {
 		},
 		edit_btn,
 		remove_btn,
-		resize_btn,
-		move_btn,
+		volumeaction_btn,
 		revert_btn,
 	    ],
 	    rows: rows,
diff --git a/www/manager6/qemu/HDReassign.js b/www/manager6/qemu/HDReassign.js
new file mode 100644
index 00000000..b6c67964
--- /dev/null
+++ b/www/manager6/qemu/HDReassign.js
@@ -0,0 +1,272 @@
+Ext.define('PVE.window.HDReassign', {
+    extend: 'Proxmox.window.Edit',
+    mixins: ['Proxmox.Mixin.CBind'],
+
+    resizable: false,
+    modal: true,
+    width: 350,
+    border: false,
+    layout: 'fit',
+    showReset: false,
+    showProgress: true,
+    method: 'POST',
+
+    viewModel: {
+	data: {
+	    mpType: '',
+	},
+	formulas: {
+	    mpMaxCount: get => get('mpType') === 'mp'
+		? PVE.Utils.mp_counts.mps - 1
+		: PVE.Utils.mp_counts.unused - 1,
+	},
+    },
+
+    cbindData: function() {
+	let me = this;
+	return {
+	    vmid: me.vmid,
+	    disk: me.disk,
+	    isQemu: me.type === 'qemu',
+	    nodename: me.nodename,
+	    url: () => {
+		let endpoint = me.type === 'qemu' ? 'move_disk' : 'move_volume';
+		return `/nodes/${me.nodename}/${me.type}/${me.vmid}/${endpoint}`;
+	    },
+	};
+    },
+
+    cbind: {
+	title: get => get('isQemu') ? gettext('Reassign disk') : gettext('Reassign volume'),
+	submitText: get => get('title'),
+	qemu: '{isQemu}',
+	url: '{url}',
+    },
+
+    getValues: function() {
+	let me = this;
+	let values = me.formPanel.getForm().getValues();
+
+	let params = {
+	    vmid: me.vmid,
+	    'target-vmid': values.targetVmid,
+	};
+
+	params[me.qemu ? 'disk' : 'volume'] = me.disk;
+
+	if (me.qemu) {
+	    params['target-disk'] = `${values.controller}${values.deviceid}`;
+	} else {
+	    params['target-volume'] = `${values.mpType}${values.mpId}`;
+	}
+	return params;
+    },
+
+    controller: {
+	xclass: 'Ext.app.ViewController',
+
+	initViewModel: function(model) {
+	    let view = this.getView();
+	    let mpTypeValue = view.disk.match(/^unused\d+/) ? 'unused' : 'mp';
+	    model.set('mpType', mpTypeValue);
+	},
+
+	onMpTypeChange: function(value) {
+	    this.getView().getViewModel().set('mpType', value.getValue());
+	    this.getView().lookup('mpIdSelector').validate();
+	},
+
+	onTargetVMChange: function(f, vmid) {
+	    let me = this;
+	    let view = me.getView();
+	    let diskSelector = view.lookup('diskSelector');
+	    if (!vmid) {
+		diskSelector.setVMConfig(null);
+		me.VMConfig = null;
+		return;
+	    }
+
+	    let type = view.qemu ? 'qemu' : 'lxc';
+
+	    let url = `/nodes/${view.nodename}/${type}/${vmid}/config`;
+	    Proxmox.Utils.API2Request({
+		url: url,
+		method: 'GET',
+		failure: response => Ext.Msg.alert(gettext('Error'), response.htmlStatus),
+		success: function(response, options) {
+		    if (view.qemu) {
+			diskSelector.setVMConfig(response.result.data);
+			diskSelector.setDisabled(false);
+		    } else {
+			let mpIdSelector = view.lookup('mpIdSelector');
+			let mpType = view.lookup('mpType');
+
+			view.VMConfig = response.result.data;
+
+			mpIdSelector.setValue(
+			    PVE.Utils.nextFreeMP(
+				view.getViewModel().get('mpType'),
+				view.VMConfig,
+			    ).id,
+			);
+
+			mpType.setDisabled(false);
+			mpIdSelector.setDisabled(false);
+			mpIdSelector.validate();
+		    }
+		},
+	    });
+	},
+    },
+
+    items: [
+	{
+	    xtype: 'form',
+	    reference: 'moveFormPanel',
+	    border: false,
+	    fieldDefaults: {
+		labelWidth: 100,
+		anchor: '100%',
+	    },
+	    items: [
+		{
+		    xtype: 'displayfield',
+		    name: 'sourceDisk',
+		    fieldLabel: gettext('Source'),
+		    cbind: {
+			name: get => get('isQemu') ? 'disk' : 'volume',
+			value: '{disk}',
+		    },
+		    allowBlank: false,
+		},
+		{
+		    xtype: 'vmComboSelector',
+		    reference: 'targetVMID',
+		    name: 'targetVmid',
+		    allowBlank: false,
+		    fieldLabel: gettext('Target'),
+		    bind: {
+			value: '{targetVMID}',
+		    },
+		    store: {
+			model: 'PVEResources',
+			autoLoad: true,
+			sorters: 'vmid',
+			cbind: {}, // for nested cbinds
+			filters: [
+			    {
+				property: 'type',
+				cbind: {
+				    value: get => get('isQemu') ? 'qemu' : 'lxc',
+				},
+			    },
+			    {
+				property: 'node',
+				cbind: {
+				    value: '{nodename}',
+				},
+			    },
+			    {
+				property: 'vmid',
+				operator: '!=',
+				cbind: {
+				    value: '{vmid}',
+				},
+			    },
+			    {
+				property: 'template',
+				value: 0,
+			    },
+			],
+		    },
+		    listeners: { change: 'onTargetVMChange' },
+		},
+		{
+		    xtype: 'pveControllerSelector',
+		    reference: 'diskSelector',
+		    withUnused: true,
+		    disabled: true,
+		    cbind: {
+			hidden: '{!isQemu}',
+		    },
+		},
+		{
+		    xtype: 'container',
+		    layout: 'hbox',
+		    cbind: {
+			hidden: '{isQemu}',
+			disabled: '{isQemu}',
+		    },
+		    items: [
+			{
+			    xtype: 'pmxDisplayEditField',
+			    cbind: {
+				editable: get => !get('disk').match(/^unused\d+/),
+				value: get => get('disk').match(/^unused\d+/) ? 'unused' : 'mp',
+			    },
+			    disabled: true,
+			    name: 'mpType',
+			    reference: 'mpType',
+			    fieldLabel: gettext('Add as'),
+			    submitValue: true,
+			    flex: 4,
+			    editConfig: {
+				xtype: 'proxmoxKVComboBox',
+				name: 'mpTypeCombo',
+				reference: 'mpTypeCombo',
+				deleteEmpty: false,
+				cbind: {
+				    hidden: '{isQemu}',
+				},
+				comboItems: [
+				    ['mp', gettext('Mount Point')],
+				    ['unused', gettext('Unused')],
+				],
+				listeners: { change: 'onMpTypeChange' },
+			    },
+			},
+			{
+			    xtype: 'proxmoxintegerfield',
+			    name: 'mpId',
+			    reference: 'mpIdSelector',
+			    minValue: 0,
+			    flex: 1,
+			    allowBlank: false,
+			    validateOnChange: true,
+			    disabled: true,
+			    bind: {
+				maxValue: '{mpMaxCount}',
+			    },
+			    validator: function(value) {
+				let view = this.up('window');
+				let type = view.getViewModel().get('mpType');
+				if (Ext.isDefined(view.VMConfig[`${type}${value}`])) {
+				    return "Mount point is already in use.";
+				}
+				return true;
+			    },
+			},
+		    ],
+		},
+	    ],
+	},
+    ],
+
+    initComponent: function() {
+	let me = this;
+
+	if (!me.nodename) {
+	    throw "no node name specified";
+	}
+
+	if (!me.vmid) {
+	    throw "no VM ID specified";
+	}
+
+	if (!me.type) {
+	    throw "no type specified";
+	}
+
+	me.callParent();
+    },
+});
diff --git a/www/manager6/qemu/HardwareView.js b/www/manager6/qemu/HardwareView.js
index 6cea4287..af84fb3f 100644
--- a/www/manager6/qemu/HardwareView.js
+++ b/www/manager6/qemu/HardwareView.js
@@ -400,8 +400,8 @@ Ext.define('PVE.qemu.HardwareView', {
 	    win.on('destroy', me.reload, me);
 	};
 
-	var run_move = function() {
-	    var rec = sm.getSelection()[0];
+	let run_move = function() {
+	    let rec = sm.getSelection()[0];
 	    if (!rec) {
 		return;
 	    }
@@ -417,6 +417,24 @@ Ext.define('PVE.qemu.HardwareView', {
 	    win.on('destroy', me.reload, me);
 	};
 
+	let run_reassign = function() {
+	    let rec = sm.getSelection()[0];
+	    if (!rec) {
+		return;
+	    }
+
+	    Ext.create('PVE.window.HDReassign', {
+		disk: rec.data.key,
+		nodename: nodename,
+		autoShow: true,
+		vmid: vmid,
+		type: 'qemu',
+		listeners: {
+		    destroy: () => me.reload(),
+		},
+	    });
+	};
+
 	var edit_btn = new Proxmox.button.Button({
 	    text: gettext('Edit'),
 	    selModel: sm,
@@ -424,20 +442,40 @@ Ext.define('PVE.qemu.HardwareView', {
 	    handler: run_editor,
         });
 
-	var resize_btn = new Proxmox.button.Button({
+	let resize_menuitem = new Ext.menu.Item({
 	    text: gettext('Resize disk'),
 	    selModel: sm,
 	    disabled: true,
 	    handler: run_resize,
 	});
 
-	var move_btn = new Proxmox.button.Button({
+	let move_menuitem = new Ext.menu.Item({
 	    text: gettext('Move disk'),
 	    selModel: sm,
-	    disabled: true,
 	    handler: run_move,
 	});
 
+	let reassign_menuitem = new Ext.menu.Item({
+	    text: gettext('Reassign Disk'),
+	    tooltip: gettext('Reassign disk to another VM'),
+	    handler: run_reassign,
+	    selModel: sm,
+	});
+
+	let diskaction_btn = new Proxmox.button.Button({
+	    text: gettext('Disk Action'),
+	    disabled: true,
+	    menu: {
+		plain: true,
+		items: [
+		    move_menuitem,
+		    reassign_menuitem,
+		    resize_menuitem,
+		],
+	    },
+	});
+
+
 	var remove_btn = new Proxmox.button.Button({
 	    text: gettext('Remove'),
 	    defaultText: gettext('Remove'),
@@ -573,8 +611,7 @@ Ext.define('PVE.qemu.HardwareView', {
 	    if (!rec) {
 		remove_btn.disable();
 		edit_btn.disable();
-		resize_btn.disable();
-		move_btn.disable();
+		diskaction_btn.disable();
 		revert_btn.disable();
 		return;
 	    }
@@ -611,9 +648,15 @@ Ext.define('PVE.qemu.HardwareView', {
 	        (isDisk && !diskCap),
 	    );
 
-	    resize_btn.setDisabled(pending || !isUsedDisk || !diskCap);
+	    resize_menuitem.setDisabled(pending || !isUsedDisk || !diskCap);
+	    reassign_menuitem.setDisabled(pending || (isEfi || tpmMoveable));
 
-	    move_btn.setDisabled(pending || !(isUsedDisk || isEfi || tpmMoveable) || !diskCap);
+	    diskaction_btn.setDisabled(
+		pending ||
+		isCloudInit ||
+		!(isDisk || isEfi || tpmMoveable) ||
+		!diskCap,
+	    );
 
 	    revert_btn.setDisabled(!pending);
 	};
@@ -775,8 +818,7 @@ Ext.define('PVE.qemu.HardwareView', {
 		},
 		remove_btn,
 		edit_btn,
-		resize_btn,
-		move_btn,
+		diskaction_btn,
 		revert_btn,
 	    ],
 	    rows: rows,
-- 
2.30.2

[pve-devel] [PATCH v4 manager 3/6] ui: lxc/qemu: disk/volume action simplify menu items

[Aaron Lauterer]

We already know that we are acting upon a disk / volume due to the
submenu we are in.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
 www/manager6/lxc/Resources.js     | 6 +++---
 www/manager6/qemu/HardwareView.js | 6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/www/manager6/lxc/Resources.js b/www/manager6/lxc/Resources.js
index 4d47679d..a456fdf2 100644
--- a/www/manager6/lxc/Resources.js
+++ b/www/manager6/lxc/Resources.js
@@ -202,7 +202,7 @@ Ext.define('PVE.lxc.RessourceView', {
 	});
 
 	let resize_menuitem = new Ext.menu.Item({
-	    text: gettext('Resize disk'),
+	    text: gettext('Resize'),
 	    selModel: me.selModel,
 	    disabled: true,
 	    handler: run_resize,
@@ -247,7 +247,7 @@ Ext.define('PVE.lxc.RessourceView', {
 	});
 
 	let reassign_menuitem = new Ext.menu.Item({
-	    text: gettext('Reassign Volume'),
+	    text: gettext('Reassign'),
 	    tooltip: gettext('Reassign volume to another CT'),
 	    handler: run_reassign,
 	    reference: 'reassing_item',
@@ -255,7 +255,7 @@ Ext.define('PVE.lxc.RessourceView', {
 	});
 
 	let move_menuitem = new Ext.menu.Item({
-	    text: gettext('Move Volume'),
+	    text: gettext('Move'),
 	    selModel: me.selModel,
 	    disabled: true,
 	    handler: run_move,
diff --git a/www/manager6/qemu/HardwareView.js b/www/manager6/qemu/HardwareView.js
index af84fb3f..3da7499d 100644
--- a/www/manager6/qemu/HardwareView.js
+++ b/www/manager6/qemu/HardwareView.js
@@ -443,20 +443,20 @@ Ext.define('PVE.qemu.HardwareView', {
         });
 
 	let resize_menuitem = new Ext.menu.Item({
-	    text: gettext('Resize disk'),
+	    text: gettext('Resize'),
 	    selModel: sm,
 	    disabled: true,
 	    handler: run_resize,
 	});
 
 	let move_menuitem = new Ext.menu.Item({
-	    text: gettext('Move disk'),
+	    text: gettext('Move'),
 	    selModel: sm,
 	    handler: run_move,
 	});
 
 	let reassign_menuitem = new Ext.menu.Item({
-	    text: gettext('Reassign Disk'),
+	    text: gettext('Reassign'),
 	    tooltip: gettext('Reassign disk to another VM'),
 	    handler: run_reassign,
 	    selModel: sm,
-- 
2.30.2

[pve-devel] [PATCH v4 manager 4/6] ui: BusTypeSelector: change noVirtIO to withVirtIO

[Aaron Lauterer]

Double negated properties make it harder than necessary to parse
conditions.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
Reviewed-by: Fabian Ebner <f.ebner@proxmox.com>
---
 www/manager6/form/BusTypeSelector.js    | 4 ++--
 www/manager6/form/ControllerSelector.js | 4 ++--
 www/manager6/qemu/CDEdit.js             | 2 +-
 www/manager6/qemu/CIDriveEdit.js        | 2 +-
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/www/manager6/form/BusTypeSelector.js b/www/manager6/form/BusTypeSelector.js
index a420e56f..0f040229 100644
--- a/www/manager6/form/BusTypeSelector.js
+++ b/www/manager6/form/BusTypeSelector.js
@@ -2,7 +2,7 @@ Ext.define('PVE.form.BusTypeSelector', {
     extend: 'Proxmox.form.KVComboBox',
     alias: 'widget.pveBusSelector',
 
-    noVirtIO: false,
+    withVirtIO: true,
     withUnused: false,
 
     initComponent: function() {
@@ -10,7 +10,7 @@ Ext.define('PVE.form.BusTypeSelector', {
 
 	me.comboItems = [['ide', 'IDE'], ['sata', 'SATA']];
 
-	if (!me.noVirtIO) {
+	if (me.withVirtIO) {
 	    me.comboItems.push(['virtio', 'VirtIO Block']);
 	}
 
diff --git a/www/manager6/form/ControllerSelector.js b/www/manager6/form/ControllerSelector.js
index 798dc4b2..d84c49d6 100644
--- a/www/manager6/form/ControllerSelector.js
+++ b/www/manager6/form/ControllerSelector.js
@@ -2,7 +2,7 @@ Ext.define('PVE.form.ControllerSelector', {
     extend: 'Ext.form.FieldContainer',
     alias: 'widget.pveControllerSelector',
 
-    noVirtIO: false,
+    withVirtIO: true,
     withUnused: false,
 
     vmconfig: {}, // used to check for existing devices
@@ -73,7 +73,7 @@ Ext.define('PVE.form.ControllerSelector', {
 		    name: 'controller',
 		    itemId: 'controller',
 		    value: PVE.qemu.OSDefaults.generic.busType,
-		    noVirtIO: me.noVirtIO,
+		    withVirtIO: me.withVirtIO,
 		    withUnused: me.withUnused,
 		    allowBlank: false,
 		    flex: 2,
diff --git a/www/manager6/qemu/CDEdit.js b/www/manager6/qemu/CDEdit.js
index 72c01037..fc7a59cc 100644
--- a/www/manager6/qemu/CDEdit.js
+++ b/www/manager6/qemu/CDEdit.js
@@ -71,7 +71,7 @@ Ext.define('PVE.qemu.CDInputPanel', {
 
 	if (!me.confid) {
 	    me.bussel = Ext.create('PVE.form.ControllerSelector', {
-		noVirtIO: true,
+		withVirtIO: false,
 	    });
 	    items.push(me.bussel);
 	}
diff --git a/www/manager6/qemu/CIDriveEdit.js b/www/manager6/qemu/CIDriveEdit.js
index 754b8353..a9ca8bf1 100644
--- a/www/manager6/qemu/CIDriveEdit.js
+++ b/www/manager6/qemu/CIDriveEdit.js
@@ -36,7 +36,7 @@ Ext.define('PVE.qemu.CIDriveInputPanel', {
 	me.items = [
 	    {
 		xtype: 'pveControllerSelector',
-		noVirtIO: true,
+		withVirtIO: false,
 		itemId: 'drive',
 		fieldLabel: gettext('CloudInit Drive'),
 		name: 'drive',
-- 
2.30.2

[pve-devel] [PATCH v4 manager 5/6] ui: hdmove: modernize/refactor

[Aaron Lauterer]

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
changes since

v3:
* code cleanup
* fix padding
* create 'url' in one place, no need for submitURL

v2:
* switch from generic window to proxmox edit

v1: much of the feedback to the HDReassign.js from the
first patch has been incorporated here as well.

 www/manager6/qemu/HDMove.js       | 185 +++++++++++++-----------------
 www/manager6/qemu/HardwareView.js |   1 +
 2 files changed, 82 insertions(+), 104 deletions(-)

diff --git a/www/manager6/qemu/HDMove.js b/www/manager6/qemu/HDMove.js
index 181b7bdc..bddbbb20 100644
--- a/www/manager6/qemu/HDMove.js
+++ b/www/manager6/qemu/HDMove.js
@@ -1,48 +1,97 @@
 Ext.define('PVE.window.HDMove', {
-    extend: 'Ext.window.Window',
+    extend: 'Proxmox.window.Edit',
+    mixins: ['Proxmox.Mixin.CBind'],
 
     resizable: false,
+    modal: true,
+    width: 350,
+    border: false,
+    layout: 'fit',
+    showReset: false,
+    showProgress: true,
+    method: 'POST',
+
+    cbindData: function() {
+	let me = this;
+	return {
+	    disk: me.disk,
+	    isQemu: me.type === 'qemu',
+	    nodename: me.nodename,
+	    url: () => {
+		let endpoint = me.type === 'qemu' ? 'move_disk' : 'move_volume';
+		return `/nodes/${me.nodename}/${me.type}/${me.vmid}/${endpoint}`;
+	    },
+	};
+    },
+
+    cbind: {
+	title: get => get('isQemu') ? gettext("Move disk") : gettext('Move Volume'),
+	submitText: get => get('title'),
+	qemu: '{isQemu}',
+	url: '{url}',
+    },
 
+    getValues: function() {
+	let me = this;
+	let values = me.formPanel.getForm().getValues();
 
-    move_disk: function(disk, storage, format, delete_disk) {
-	var me = this;
-	var qemu = me.type === 'qemu';
-	var params = {};
-	params.storage = storage;
-	params[qemu ? 'disk':'volume'] = disk;
+	let params = {
+	    storage: values.hdstorage,
+	};
+	params[me.qemu ? 'disk' : 'volume'] = me.disk;
 
-	if (format && qemu) {
-	    params.format = format;
+	if (values.diskformat && me.qemu) {
+	    params.format = values.diskformat;
 	}
 
-	if (delete_disk) {
+	if (values.deleteDisk) {
 	    params.delete = 1;
 	}
+	return params;
+    },
 
-	var url = '/nodes/' + me.nodename + '/' + me.type + '/' + me.vmid + '/';
-	url += qemu ? 'move_disk' : 'move_volume';
-
-	Proxmox.Utils.API2Request({
-	    params: params,
-	    url: url,
-	    waitMsgTarget: me,
-	    method: 'POST',
-	    failure: function(response, opts) {
-		Ext.Msg.alert('Error', response.htmlStatus);
-	    },
-	    success: function(response, options) {
-		var upid = response.result.data;
-		var win = Ext.create('Proxmox.window.TaskViewer', {
-		    upid: upid,
-		});
-		win.show();
-		win.on('destroy', function() { me.close(); });
+    items: [
+	{
+	    xtype: 'form',
+	    reference: 'moveFormPanel',
+	    border: false,
+	    fieldDefaults: {
+		labelWidth: 100,
+		anchor: '100%',
 	    },
-	});
-    },
+	    items: [
+		{
+		    xtype: 'displayfield',
+		    cbind: {
+			name: get => get('isQemu') ? 'disk' : 'volume',
+			fieldLabel: get => get('isQemu') ? gettext('Disk') : gettext('Mount Point'),
+			value: '{disk}',
+		    },
+		    allowBlank: false,
+		},
+		{
+		    xtype: 'pveDiskStorageSelector',
+		    storageLabel: gettext('Target Storage'),
+		    cbind: {
+			nodename: '{nodename}',
+			storageContent: get => get('isQemu') ? 'images' : 'rootdir',
+			hideFormat: get => get('disk') === 'tpmstate0',
+		    },
+		    hideSize: true,
+		},
+		{
+		    xtype: 'proxmoxcheckbox',
+		    fieldLabel: gettext('Delete source'),
+		    name: 'deleteDisk',
+		    uncheckedValue: 0,
+		    checked: false,
+		},
+	    ],
+	},
+    ],
 
     initComponent: function() {
-	var me = this;
+	let me = this;
 
 	if (!me.nodename) {
 	    throw "no node name specified";
@@ -53,81 +102,9 @@ Ext.define('PVE.window.HDMove', {
 	}
 
 	if (!me.type) {
-	    me.type = 'qemu';
+	    throw "no type specified";
 	}
 
-	var qemu = me.type === 'qemu';
-
-        var items = [
-            {
-                xtype: 'displayfield',
-                name: qemu ? 'disk' : 'volume',
-                value: me.disk,
-                fieldLabel: qemu ? gettext('Disk') : gettext('Mount Point'),
-                vtype: 'StorageId',
-                allowBlank: false,
-            },
-        ];
-
-	items.push({
-	    xtype: 'pveDiskStorageSelector',
-	    storageLabel: gettext('Target Storage'),
-	    nodename: me.nodename,
-	    storageContent: qemu ? 'images' : 'rootdir',
-	    hideSize: true,
-	    hideFormat: me.disk === 'tpmstate0',
-	});
-
-	items.push({
-	    xtype: 'proxmoxcheckbox',
-	    fieldLabel: gettext('Delete source'),
-	    name: 'deleteDisk',
-	    uncheckedValue: 0,
-	    checked: false,
-	});
-
-	me.formPanel = Ext.create('Ext.form.Panel', {
-	    bodyPadding: 10,
-	    border: false,
-	    fieldDefaults: {
-		labelWidth: 100,
-		anchor: '100%',
-	    },
-	    items: items,
-	});
-
-	var form = me.formPanel.getForm();
-
-	var submitBtn;
-
-	me.title = qemu ? gettext("Move disk") : gettext('Move Volume');
-	submitBtn = Ext.create('Ext.Button', {
-	    text: me.title,
-	    handler: function() {
-		if (form.isValid()) {
-		    var values = form.getValues();
-		    me.move_disk(me.disk, values.hdstorage, values.diskformat,
-				 values.deleteDisk);
-		}
-	    },
-	});
-
-	Ext.apply(me, {
-	    modal: true,
-	    width: 350,
-	    border: false,
-	    layout: 'fit',
-	    buttons: [submitBtn],
-	    items: [me.formPanel],
-	});
-
-
 	me.callParent();
-
-	me.mon(me.formPanel, 'validitychange', function(fp, isValid) {
-	    submitBtn.setDisabled(!isValid);
-	});
-
-	me.formPanel.isValid();
     },
 });
diff --git a/www/manager6/qemu/HardwareView.js b/www/manager6/qemu/HardwareView.js
index 3da7499d..1537f236 100644
--- a/www/manager6/qemu/HardwareView.js
+++ b/www/manager6/qemu/HardwareView.js
@@ -410,6 +410,7 @@ Ext.define('PVE.qemu.HardwareView', {
 		disk: rec.data.key,
 		nodename: nodename,
 		vmid: vmid,
+		type: 'qemu',
 	    });
 
 	    win.show();
-- 
2.30.2

[pve-devel] [PATCH v4 manager 6/6] ui: util: refactor mps to mp

[Aaron Lauterer]

Using the actual config key instead of the pluralization, makes it
easier in the situations where we need to match against it.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
 www/manager6/Utils.js           | 8 +++-----
 www/manager6/lxc/MPEdit.js      | 2 +-
 www/manager6/lxc/MultiMPEdit.js | 4 ++--
 www/manager6/qemu/HDReassign.js | 2 +-
 4 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index 5190f750..519faac5 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -1578,12 +1578,12 @@ Ext.define('PVE.Utils', {
     },
 
     mp_counts: {
-	mps: 256,
+	mp: 256,
 	unused: 256,
     },
 
     forEachMP: function(func, includeUnused) {
-	for (let i = 0; i < PVE.Utils.mp_counts.mps; i++) {
+	for (let i = 0; i < PVE.Utils.mp_counts.mp; i++) {
 	    let cont = func('mp', i);
 	    if (!cont && cont !== undefined) {
 		return;
@@ -1805,9 +1805,7 @@ Ext.define('PVE.Utils', {
     },
 
     nextFreeMP: function(type, config) {
-	let mptype = type === "mp" ? "mps" : type;
-
-	for (let i = 0; i < PVE.Utils.mp_counts[mptype]; i++) {
+	for (let i = 0; i < PVE.Utils.mp_counts[type]; i++) {
 	    let confid = `${type}${i}`;
 	    if (!Ext.isDefined(config[confid])) {
 		return {
diff --git a/www/manager6/lxc/MPEdit.js b/www/manager6/lxc/MPEdit.js
index b8c6b6cb..609447ef 100644
--- a/www/manager6/lxc/MPEdit.js
+++ b/www/manager6/lxc/MPEdit.js
@@ -194,7 +194,7 @@ Ext.define('PVE.lxc.MountPointInputPanel', {
 	    name: 'mpid',
 	    fieldLabel: gettext('Mount Point ID'),
 	    minValue: 0,
-	    maxValue: PVE.Utils.mp_counts.mps - 1,
+	    maxValue: PVE.Utils.mp_counts.mp - 1,
 	    hidden: true,
 	    allowBlank: false,
 	    disabled: true,
diff --git a/www/manager6/lxc/MultiMPEdit.js b/www/manager6/lxc/MultiMPEdit.js
index 709dacb1..36cee4ff 100644
--- a/www/manager6/lxc/MultiMPEdit.js
+++ b/www/manager6/lxc/MultiMPEdit.js
@@ -8,7 +8,7 @@ Ext.define('PVE.lxc.MultiMPPanel', {
 	xclass: 'Ext.app.ViewController',
 
 	// count of mps + rootfs
-	maxCount: PVE.Utils.mp_counts.mps + 1,
+	maxCount: PVE.Utils.mp_counts.mp + 1,
 
 	getNextFreeDisk: function(vmconfig) {
 	    let nextFreeDisk;
@@ -17,7 +17,7 @@ Ext.define('PVE.lxc.MultiMPPanel', {
 		    confid: 'rootfs',
 		};
 	    } else {
-		for (let i = 0; i < PVE.Utils.mp_counts.mps; i++) {
+		for (let i = 0; i < PVE.Utils.mp_counts.mp; i++) {
 		    let confid = `mp${i}`;
 		    if (!vmconfig[confid]) {
 			nextFreeDisk = {
diff --git a/www/manager6/qemu/HDReassign.js b/www/manager6/qemu/HDReassign.js
index b6c67964..ef04a8cf 100644
--- a/www/manager6/qemu/HDReassign.js
+++ b/www/manager6/qemu/HDReassign.js
@@ -17,7 +17,7 @@ Ext.define('PVE.window.HDReassign', {
 	},
 	formulas: {
 	    mpMaxCount: get => get('mpType') === 'mp'
-		? PVE.Utils.mp_counts.mps - 1
+		? PVE.Utils.mp_counts.mp - 1
 		: PVE.Utils.mp_counts.unused - 1,
 	},
     },
-- 
2.30.2

Re: [pve-devel] [PATCH v4 manager 2/6] ui: lxc/qemu: add disk reassign and action submenu

[Fabian Ebner]

Am 14.03.22 um 10:35 schrieb Aaron Lauterer:
> @@ -227,14 +246,34 @@ Ext.define('PVE.lxc.RessourceView', {
>  	    },
>  	});
>  
> -	var move_btn = new Proxmox.button.Button({
> +	let reassign_menuitem = new Ext.menu.Item({
> +	    text: gettext('Reassign Volume'),
> +	    tooltip: gettext('Reassign volume to another CT'),


Nit: if there is a tooltip for reassign maybe there should also be one
for move? Otherwise, it feels inconsistent from the perspective of a new
user ;) Same for VMs.

> +	    handler: run_reassign,
> +	    reference: 'reassing_item',
> +	    disabled: true,
> +	});
> +
> +	let move_menuitem = new Ext.menu.Item({
>  	    text: gettext('Move Volume'),
>  	    selModel: me.selModel,
>  	    disabled: true,
> -	    dangerous: true,
>  	    handler: run_move,
>  	});
>  

(...)

> @@ -265,9 +305,12 @@ Ext.define('PVE.lxc.RessourceView', {
>  	    }
>  	    edit_btn.setDisabled(noedit);
>  
> -	    remove_btn.setDisabled(!isDisk || rec.data.key === 'rootfs' || !diskCap || pending);
> -	    resize_btn.setDisabled(!isDisk || !diskCap || isUnusedDisk);
> -	    move_btn.setDisabled(!isDisk || !diskCap);
> +	    volumeaction_btn.setDisabled(!isDisk);

Shouldn't this also check for diskCap?

> +	    reassign_menuitem.setDisabled(isRootFS);
> +
> +	    remove_btn.setDisabled(!isDisk || isRootFS || !diskCap || pending);
> +	    resize_menuitem.setDisabled(!isDisk || !diskCap || isUnusedDisk);
> +	    move_menuitem.setDisabled(!isDisk || !diskCap);

Nit: please group the menu and menuitems together. And since the whole
menu is disabled if !isDisk, you don't need to repeat that for the
menuitems (like you already don't for reassign ;)).

>  	    revert_btn.setDisabled(!pending);
>  
>  	    remove_btn.setText(isUsedDisk ? remove_btn.altText : remove_btn.defaultText);

(...)

> diff --git a/www/manager6/qemu/HardwareView.js b/www/manager6/qemu/HardwareView.js
> index 6cea4287..af84fb3f 100644
> --- a/www/manager6/qemu/HardwareView.js
> +++ b/www/manager6/qemu/HardwareView.js
> @@ -400,8 +400,8 @@ Ext.define('PVE.qemu.HardwareView', {
>  	    win.on('destroy', me.reload, me);
>  	};
>  
> -	var run_move = function() {
> -	    var rec = sm.getSelection()[0];
> +	let run_move = function() {
> +	    let rec = sm.getSelection()[0];
>  	    if (!rec) {
>  		return;
>  	    }

Nit: While it is an improvement, it doesn't actually interact with
anything else in the patch, and hence doesn't really belong here.

(...)

> @@ -611,9 +648,15 @@ Ext.define('PVE.qemu.HardwareView', {
>  	        (isDisk && !diskCap),
>  	    );
>  
> -	    resize_btn.setDisabled(pending || !isUsedDisk || !diskCap);
> +	    resize_menuitem.setDisabled(pending || !isUsedDisk || !diskCap);
> +	    reassign_menuitem.setDisabled(pending || (isEfi || tpmMoveable));
>  
> -	    move_btn.setDisabled(pending || !(isUsedDisk || isEfi || tpmMoveable) || !diskCap);
> +	    diskaction_btn.setDisabled(
> +		pending ||
> +		isCloudInit ||
> +		!(isDisk || isEfi || tpmMoveable) ||
> +		!diskCap,
> +	    );

Here you are using the fact that "move action disabled" implies "resize
and reassign action disabled". Might be worth giving the following a
shot hoping it's cleaner:

if pending || !diskCap || other common criteria
  disable menu
else
  enable menu
  disable resize based on resize-specific criteria (here the common
criteria don't need to be repeated)
  disable reassign based on reassign-specific criteria
  disable move based on move-specific criteria

and if the common criteria are collected correctly, there should never
be a case where all entries are disabled but the menu isn't.

>  
>  	    revert_btn.setDisabled(!pending);
>  	};

Re: [pve-devel] [PATCH v4 manager 6/6] ui: util: refactor mps to mp

[Fabian Ebner]

Am 14.03.22 um 10:35 schrieb Aaron Lauterer:
> @@ -1805,9 +1805,7 @@ Ext.define('PVE.Utils', {
>      },
>  
>      nextFreeMP: function(type, config) {
> -	let mptype = type === "mp" ? "mps" : type;
> -
> -	for (let i = 0; i < PVE.Utils.mp_counts[mptype]; i++) {
> +	for (let i = 0; i < PVE.Utils.mp_counts[type]; i++) {
>  	    let confid = `${type}${i}`;
>  	    if (!Ext.isDefined(config[confid])) {
>  		return {

Nit: If this patch were ordered before the first one, we could start out
with the simpler version.

Re: [pve-devel] [PATCH v4 manager 0/6] ui: lxc/qemu: add reassign for disks and volumes

[Fabian Ebner]

Am 14.03.22 um 10:35 schrieb Aaron Lauterer:
> This series adds the UI to reassign a disk / volume from one guest to another.
> 
> To avoid button clutter, the Move, Reassing and Resize buttons are moved
> into a new submenu called "Disk/Volume Action".
> 
> Patch 3 to 6 are optional. Patch 3 changes the labels for Move, Reassign
> and Resize to remove Volume & Disk as we already have this in the
> context of the submenu.
> 
> Patch 4 only changes a double negated option.
> Patch 5 happend in the process of working on an interface for the
> reassign functionality. Since the work of modernizing this componend is
> done, why not use it
> 
> Patch 6 changes how we store the max number of MPs possible because I
> ran into the issue, that I cannot easily match against the object key if
> it is 'mps' instead of 'mp'.
> 

Just nits, so
Reviewed-by: Fabian Ebner <f.ebner@proxmox.com>

Re: [pve-devel] [PATCH v4 manager 6/6] ui: util: refactor mps to mp

[Aaron Lauterer]

On 3/22/22 12:18, Fabian Ebner wrote:
> Am 14.03.22 um 10:35 schrieb Aaron Lauterer:
>> @@ -1805,9 +1805,7 @@ Ext.define('PVE.Utils', {
>>       },
>>   
>>       nextFreeMP: function(type, config) {
>> -	let mptype = type === "mp" ? "mps" : type;
>> -
>> -	for (let i = 0; i < PVE.Utils.mp_counts[mptype]; i++) {
>> +	for (let i = 0; i < PVE.Utils.mp_counts[type]; i++) {
>>   	    let confid = `${type}${i}`;
>>   	    if (!Ext.isDefined(config[confid])) {
>>   		return {
> 
> Nit: If this patch were ordered before the first one, we could start out
> with the simpler version.

True but I went with the approach to make this change optional. That's why it is the last in the series.

Re: [pve-devel] [PATCH v4 manager 2/6] ui: lxc/qemu: add disk reassign and action submenu

[Aaron Lauterer]

Thanks for the nits. Especially regarding the button (de)activation logic.

I can send a follow-up patch or a new version of the whole series, or just this patch.

Whatever is preferred :)

On 3/22/22 12:18, Fabian Ebner wrote:
> Am 14.03.22 um 10:35 schrieb Aaron Lauterer:
>> @@ -227,14 +246,34 @@ Ext.define('PVE.lxc.RessourceView', {
>>   	    },
>>   	});
>>   
>> -	var move_btn = new Proxmox.button.Button({
>> +	let reassign_menuitem = new Ext.menu.Item({
>> +	    text: gettext('Reassign Volume'),
>> +	    tooltip: gettext('Reassign volume to another CT'),
> 
> 
> Nit: if there is a tooltip for reassign maybe there should also be one
> for move? Otherwise, it feels inconsistent from the perspective of a new
> user ;) Same for VMs.
> 
>> +	    handler: run_reassign,
>> +	    reference: 'reassing_item',
>> +	    disabled: true,
>> +	});
>> +
>> +	let move_menuitem = new Ext.menu.Item({
>>   	    text: gettext('Move Volume'),
>>   	    selModel: me.selModel,
>>   	    disabled: true,
>> -	    dangerous: true,
>>   	    handler: run_move,
>>   	});
>>   
> 
> (...)
> 
>> @@ -265,9 +305,12 @@ Ext.define('PVE.lxc.RessourceView', {
>>   	    }
>>   	    edit_btn.setDisabled(noedit);
>>   
>> -	    remove_btn.setDisabled(!isDisk || rec.data.key === 'rootfs' || !diskCap || pending);
>> -	    resize_btn.setDisabled(!isDisk || !diskCap || isUnusedDisk);
>> -	    move_btn.setDisabled(!isDisk || !diskCap);
>> +	    volumeaction_btn.setDisabled(!isDisk);
> 
> Shouldn't this also check for diskCap?
> 
>> +	    reassign_menuitem.setDisabled(isRootFS);
>> +
>> +	    remove_btn.setDisabled(!isDisk || isRootFS || !diskCap || pending);
>> +	    resize_menuitem.setDisabled(!isDisk || !diskCap || isUnusedDisk);
>> +	    move_menuitem.setDisabled(!isDisk || !diskCap);
> 
> Nit: please group the menu and menuitems together. And since the whole
> menu is disabled if !isDisk, you don't need to repeat that for the
> menuitems (like you already don't for reassign ;)).
> 
>>   	    revert_btn.setDisabled(!pending);
>>   
>>   	    remove_btn.setText(isUsedDisk ? remove_btn.altText : remove_btn.defaultText);
> 
> (...)
> 
>> diff --git a/www/manager6/qemu/HardwareView.js b/www/manager6/qemu/HardwareView.js
>> index 6cea4287..af84fb3f 100644
>> --- a/www/manager6/qemu/HardwareView.js
>> +++ b/www/manager6/qemu/HardwareView.js
>> @@ -400,8 +400,8 @@ Ext.define('PVE.qemu.HardwareView', {
>>   	    win.on('destroy', me.reload, me);
>>   	};
>>   
>> -	var run_move = function() {
>> -	    var rec = sm.getSelection()[0];
>> +	let run_move = function() {
>> +	    let rec = sm.getSelection()[0];
>>   	    if (!rec) {
>>   		return;
>>   	    }
> 
> Nit: While it is an improvement, it doesn't actually interact with
> anything else in the patch, and hence doesn't really belong here.
> 
> (...)
> 
>> @@ -611,9 +648,15 @@ Ext.define('PVE.qemu.HardwareView', {
>>   	        (isDisk && !diskCap),
>>   	    );
>>   
>> -	    resize_btn.setDisabled(pending || !isUsedDisk || !diskCap);
>> +	    resize_menuitem.setDisabled(pending || !isUsedDisk || !diskCap);
>> +	    reassign_menuitem.setDisabled(pending || (isEfi || tpmMoveable));
>>   
>> -	    move_btn.setDisabled(pending || !(isUsedDisk || isEfi || tpmMoveable) || !diskCap);
>> +	    diskaction_btn.setDisabled(
>> +		pending ||
>> +		isCloudInit ||
>> +		!(isDisk || isEfi || tpmMoveable) ||
>> +		!diskCap,
>> +	    );
> 
> Here you are using the fact that "move action disabled" implies "resize
> and reassign action disabled". Might be worth giving the following a
> shot hoping it's cleaner:
> 
> if pending || !diskCap || other common criteria
>    disable menu
> else
>    enable menu
>    disable resize based on resize-specific criteria (here the common
> criteria don't need to be repeated)
>    disable reassign based on reassign-specific criteria
>    disable move based on move-specific criteria
> 
> and if the common criteria are collected correctly, there should never
> be a case where all entries are disabled but the menu isn't.
> 
>>   
>>   	    revert_btn.setDisabled(!pending);
>>   	};

Re: [pve-devel] [PATCH v4 manager 3/6] ui: lxc/qemu: disk/volume action simplify menu items

[Thomas Lamprecht]

On 14.03.22 10:35, Aaron Lauterer wrote:
> We already know that we are acting upon a disk / volume due to the
> submenu we are in.

ok with that in general, but some proposal over conveying better what "move" and
"reassign" respectively means while not getting a long label.

Also, a (separate) suggestions for possible icons.

> 
> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
> ---
>  www/manager6/lxc/Resources.js     | 6 +++---
>  www/manager6/qemu/HardwareView.js | 6 +++---
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/www/manager6/lxc/Resources.js b/www/manager6/lxc/Resources.js
> index 4d47679d..a456fdf2 100644
> --- a/www/manager6/lxc/Resources.js
> +++ b/www/manager6/lxc/Resources.js
> @@ -202,7 +202,7 @@ Ext.define('PVE.lxc.RessourceView', {
>  	});
>  
>  	let resize_menuitem = new Ext.menu.Item({
> -	    text: gettext('Resize disk'),
> +	    text: gettext('Resize'),

+ fa-plus icon

>  	    selModel: me.selModel,
>  	    disabled: true,
>  	    handler: run_resize,
> @@ -247,7 +247,7 @@ Ext.define('PVE.lxc.RessourceView', {
>  	});
>  
>  	let reassign_menuitem = new Ext.menu.Item({
> -	    text: gettext('Reassign Volume'),
> +	    text: gettext('Reassign'),

"Reassign Owner"

+ LXC box icon


>  	    tooltip: gettext('Reassign volume to another CT'),
>  	    handler: run_reassign,
>  	    reference: 'reassing_item',
> @@ -255,7 +255,7 @@ Ext.define('PVE.lxc.RessourceView', {
>  	});
>  
>  	let move_menuitem = new Ext.menu.Item({
> -	    text: gettext('Move Volume'),
> +	    text: gettext('Move'),

"Move Storage"

+ arrow-right icon

>  	    selModel: me.selModel,
>  	    disabled: true,
>  	    handler: run_move,
> diff --git a/www/manager6/qemu/HardwareView.js b/www/manager6/qemu/HardwareView.js
> index af84fb3f..3da7499d 100644
> --- a/www/manager6/qemu/HardwareView.js
> +++ b/www/manager6/qemu/HardwareView.js
> @@ -443,20 +443,20 @@ Ext.define('PVE.qemu.HardwareView', {
>          });
>  
>  	let resize_menuitem = new Ext.menu.Item({
> -	    text: gettext('Resize disk'),
> +	    text: gettext('Resize'),

+ fa-plus icon

>  	    selModel: sm,
>  	    disabled: true,
>  	    handler: run_resize,
>  	});
>  
>  	let move_menuitem = new Ext.menu.Item({
> -	    text: gettext('Move disk'),
> +	    text: gettext('Move'),

"Move Storage"

+ arrow-right icon

>  	    selModel: sm,
>  	    handler: run_move,
>  	});
>  
>  	let reassign_menuitem = new Ext.menu.Item({
> -	    text: gettext('Reassign Disk'),
> +	    text: gettext('Reassign'),


"Reassign Owner"

+ VM "display" icon

>  	    tooltip: gettext('Reassign disk to another VM'),
>  	    handler: run_reassign,
>  	    selModel: sm,