[pve-devel] [PATCH v6 manager 0/5] ui: lxc/qemu: disk/volume action simplify menu items

[pve-devel] [PATCH v6 manager 0/5] ui: lxc/qemu: disk/volume action simplify menu items

[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 1 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'.

Patch 4 to 5 are optional. 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


Whole Series previously (v4) got a:
Reviewed-by: Fabian Ebner <f.ebner@proxmox.com>

v6: rebased
v5:
* reorder patches:
    - refactor mps to mp first
    - drop simplification of button labes and name them right away
* added icons and renamed buttons to better indicate what move and
  reassign does
     thanks @Thomas for the suggestions. For move I went with the same
     icon we use for the storage as that hopefully makes it clearer,
     like using the container and VM icons for the reassign button
* reworked other style nits, like the logic which buttons are active
    thx @Fabian Ebner for taking another close look at it.

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 (5):
  ui: utils: refactor mps to mp
  ui: utils: add nextFreeMP
  ui: lxc/qemu: add disk reassign and action submenu
  ui: BusTypeSelector: change noVirtIO to withVirtIO
  ui: hdmove: modernize/refactor

 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           |  78 +++++--
 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       |  75 +++++--
 12 files changed, 499 insertions(+), 149 deletions(-)
 create mode 100644 www/manager6/qemu/HDReassign.js

-- 
2.30.2

[pve-devel] [PATCH v6 manager 1/5] ui: utils: 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>
---
changes since:
v4: moved at the start of the series to avoid unnecessary steps

 www/manager6/Utils.js           | 4 ++--
 www/manager6/lxc/MPEdit.js      | 2 +-
 www/manager6/lxc/MultiMPEdit.js | 4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index 735806aa..3351e53b 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -1575,12 +1575,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;
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 = {
-- 
2.30.2

[pve-devel] [PATCH v6 manager 2/5] ui: utils: add nextFreeMP

[Aaron Lauterer]

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

diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index 3351e53b..fe5be283 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -1800,6 +1800,21 @@ Ext.define('PVE.Utils', {
 
 	return undefined;
     },
+
+    nextFreeMP: function(type, config) {
+	for (let i = 0; i < PVE.Utils.mp_counts[type]; 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 v6 manager 3/5] 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
v5: rebased
v4:
* use button labels and icons as suggested by @Thomas
    only for Move Storage I went with the same Icon we use for the
    storages. That should make the connection clearer since we are also
    using the LXC/VM icons for the reassign buttons.
* smaller style nits like reordering button/menuitem definitions to be
  grouped closer together
* reworked logic when to disable buttons/menuitems

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     |  78 +++++++--
 www/manager6/qemu/HDReassign.js   | 272 ++++++++++++++++++++++++++++++
 www/manager6/qemu/HardwareView.js |  74 ++++++--
 4 files changed, 391 insertions(+), 34 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 683be526..9ca33f17 100644
--- a/www/manager6/lxc/Resources.js
+++ b/www/manager6/lxc/Resources.js
@@ -162,7 +162,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;
 	    }
@@ -179,6 +180,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,
@@ -193,13 +212,6 @@ Ext.define('PVE.lxc.RessourceView', {
 	    handler: function() { me.run_editor(); },
 	});
 
-	var resize_btn = new Proxmox.button.Button({
-	    text: gettext('Resize disk'),
-	    selModel: me.selModel,
-	    disabled: true,
-	    handler: run_resize,
-	});
-
 	var remove_btn = new Proxmox.button.Button({
 	    text: gettext('Remove'),
 	    defaultText: gettext('Remove'),
@@ -238,14 +250,41 @@ Ext.define('PVE.lxc.RessourceView', {
 	    },
 	});
 
-	var move_btn = new Proxmox.button.Button({
-	    text: gettext('Move Volume'),
+	let move_menuitem = new Ext.menu.Item({
+	    text: gettext('Move Storage'),
+	    tooltip: gettext('Move volume to another storage'),
+	    iconCls: 'fa fa-database',
 	    selModel: me.selModel,
-	    disabled: true,
-	    dangerous: true,
 	    handler: run_move,
 	});
 
+	let reassign_menuitem = new Ext.menu.Item({
+	    text: gettext('Reassign Owner'),
+	    tooltip: gettext('Reassign volume to another CT'),
+	    iconCls: 'fa fa-cube',
+	    handler: run_reassign,
+	    reference: 'reassing_item',
+	});
+
+	let resize_menuitem = new Ext.menu.Item({
+	    text: gettext('Resize'),
+	    iconCls: 'fa fa-plus',
+	    selModel: me.selModel,
+	    handler: run_resize,
+	});
+
+	let volumeaction_btn = new Proxmox.button.Button({
+	    text: gettext('Volume Action'),
+	    disabled: true,
+	    menu: {
+		items: [
+		    move_menuitem,
+		    reassign_menuitem,
+		    resize_menuitem,
+		],
+	    },
+	});
+
 	var revert_btn = new PVE.button.PendingRevert();
 
 	var set_button_status = function() {
@@ -254,7 +293,7 @@ Ext.define('PVE.lxc.RessourceView', {
 	    if (!rec) {
 		edit_btn.disable();
 		remove_btn.disable();
-		resize_btn.disable();
+		volumeaction_btn.disable();
 		revert_btn.disable();
 		return;
 	    }
@@ -264,6 +303,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;
 
@@ -276,9 +316,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 || !diskCap);
+	    move_menuitem.setDisabled(isUnusedDisk);
+	    reassign_menuitem.setDisabled(isRootFS);
+	    resize_menuitem.setDisabled(isUnusedDisk);
+
+	    remove_btn.setDisabled(!isDisk || isRootFS || !diskCap || pending);
 	    revert_btn.setDisabled(!pending);
 
 	    remove_btn.setText(isUsedDisk ? remove_btn.altText : remove_btn.defaultText);
@@ -340,8 +383,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 e3506e4d..749d355f 100644
--- a/www/manager6/qemu/HardwareView.js
+++ b/www/manager6/qemu/HardwareView.js
@@ -377,16 +377,17 @@ Ext.define('PVE.qemu.HardwareView', {
 	    handler: run_editor,
 	});
 
-	let resize_btn = new Proxmox.button.Button({
-	    text: gettext('Resize disk'),
+	let move_menuitem = new Ext.menu.Item({
+	    text: gettext('Move Storage'),
+	    tooltip: gettext('Move disk to another storage'),
+	    iconCls: 'fa fa-database',
 	    selModel: sm,
-	    disabled: true,
 	    handler: () => {
 		let rec = sm.getSelection()[0];
 		if (!rec) {
 		    return;
 		}
-		Ext.create('PVE.window.HDResize', {
+		Ext.create('PVE.window.HDMove', {
 		    autoShow: true,
 		    disk: rec.data.key,
 		    nodename: nodename,
@@ -398,20 +399,23 @@ Ext.define('PVE.qemu.HardwareView', {
 	    },
 	});
 
-	let move_btn = new Proxmox.button.Button({
-	    text: gettext('Move disk'),
+	let reassign_menuitem = new Ext.menu.Item({
+	    text: gettext('Reassign Owner'),
+	    tooltip: gettext('Reassign disk to another VM'),
+	    iconCls: 'fa fa-desktop',
 	    selModel: sm,
-	    disabled: true,
 	    handler: () => {
-		var rec = sm.getSelection()[0];
+		let rec = sm.getSelection()[0];
 		if (!rec) {
 		    return;
 		}
-		Ext.create('PVE.window.HDMove', {
+
+		Ext.create('PVE.window.HDReassign', {
 		    autoShow: true,
 		    disk: rec.data.key,
 		    nodename: nodename,
 		    vmid: vmid,
+		    type: 'qemu',
 		    listeners: {
 			destroy: () => me.reload(),
 		    },
@@ -419,6 +423,40 @@ Ext.define('PVE.qemu.HardwareView', {
 	    },
 	});
 
+	let resize_menuitem = new Ext.menu.Item({
+	    text: gettext('Resize'),
+	    iconCls: 'fa fa-plus',
+	    selModel: sm,
+	    handler: () => {
+		let rec = sm.getSelection()[0];
+		if (!rec) {
+		    return;
+		}
+		Ext.create('PVE.window.HDResize', {
+		    autoShow: true,
+		    disk: rec.data.key,
+		    nodename: nodename,
+		    vmid: vmid,
+		    listeners: {
+			destroy: () => me.reload(),
+		    },
+		});
+	    },
+	});
+
+	let diskaction_btn = new Proxmox.button.Button({
+	    text: gettext('Disk Action'),
+	    disabled: true,
+	    menu: {
+		items: [
+		    move_menuitem,
+		    reassign_menuitem,
+		    resize_menuitem,
+		],
+	    },
+	});
+
+
 	let remove_btn = new Proxmox.button.Button({
 	    text: gettext('Remove'),
 	    defaultText: gettext('Remove'),
@@ -544,8 +582,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;
 	    }
@@ -572,9 +609,15 @@ Ext.define('PVE.qemu.HardwareView', {
 	    edit_btn.setDisabled(
 	        deleted || !row.editor || isCloudInit || (isCDRom && !cdromCap) || (isDisk && !diskCap));
 
-	    resize_btn.setDisabled(pending || !isUsedDisk || !diskCap);
-
-	    move_btn.setDisabled(pending || !(isUsedDisk || isEfi || tpmMoveable) || !diskCap);
+	    diskaction_btn.setDisabled(
+		pending ||
+		!diskCap ||
+		isCloudInit ||
+		!(isDisk || isEfi || tpmMoveable),
+	    );
+	    move_menuitem.setDisabled(isUnusedDisk);
+	    reassign_menuitem.setDisabled(pending || (isEfi || tpmMoveable));
+	    resize_menuitem.setDisabled(pending || !isUsedDisk);
 
 	    revert_btn.setDisabled(!pending);
 	};
@@ -679,8 +722,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 v6 manager 4/5] 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 v6 manager 5/5] ui: hdmove: modernize/refactor

[Aaron Lauterer]

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
changes since
v5: rebased
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 749d355f..3974d949 100644
--- a/www/manager6/qemu/HardwareView.js
+++ b/www/manager6/qemu/HardwareView.js
@@ -392,6 +392,7 @@ Ext.define('PVE.qemu.HardwareView', {
 		    disk: rec.data.key,
 		    nodename: nodename,
 		    vmid: vmid,
+		    type: 'qemu',
 		    listeners: {
 			destroy: () => me.reload(),
 		    },
-- 
2.30.2

Re: [pve-devel] [PATCH v6 manager 3/5] ui: lxc/qemu: add disk reassign and action submenu

[Fabian Ebner]

Am 05.04.22 um 14:30 schrieb Aaron Lauterer:
> @@ -264,6 +303,7 @@ Ext.define('PVE.lxc.RessourceView', {
>  
>  	    var pending = rec.data.delete || me.hasPendingChanges(key);
>  	    var isDisk = rowdef.tdCls === 'pve-itype-icon-storage';

Pre-existing: this is very brittle and actually broke with commit
fa7980c2bd59612c5cdaa6c4e586b86d30b614c0
I sent a fix, and since there is another small issue and another nit,
see below, you probably want to wait for that and rebase on top for v7.

> +	    let isRootFS = rec.data.key === 'rootfs';
>  	    var isUnusedDisk = key.match(/^unused\d+/);
>  	    var isUsedDisk = isDisk && !isUnusedDisk;
>  

(...)

> 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

Should be .mp.
Or better, the whole expression could now be (untested)
    PVE.Utils.mp_counts[get('mpType')] - 1

> +		: PVE.Utils.mp_counts.unused - 1,
> +	},
> +    },
> +

(...)

> @@ -572,9 +609,15 @@ Ext.define('PVE.qemu.HardwareView', {
>  	    edit_btn.setDisabled(
>  	        deleted || !row.editor || isCloudInit || (isCDRom && !cdromCap) || (isDisk && !diskCap));
>  
> -	    resize_btn.setDisabled(pending || !isUsedDisk || !diskCap);
> -
> -	    move_btn.setDisabled(pending || !(isUsedDisk || isEfi || tpmMoveable) || !diskCap);
> +	    diskaction_btn.setDisabled(
> +		pending ||
> +		!diskCap ||
> +		isCloudInit ||
> +		!(isDisk || isEfi || tpmMoveable),
> +	    );
> +	    move_menuitem.setDisabled(isUnusedDisk);
> +	    reassign_menuitem.setDisabled(pending || (isEfi || tpmMoveable));
> +	    resize_menuitem.setDisabled(pending || !isUsedDisk);

Nit: pending can be dropped from the two expressions for the menu items,
because the whole menu is already disabled if pending is true.

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

Re: [pve-devel] [PATCH v6 manager 0/5] ui: lxc/qemu: disk/volume action simplify menu items

[Fabian Ebner]

Am 05.04.22 um 14:30 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 1 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'.
> 
> Patch 4 to 5 are optional. 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
> 
> 

Series looks good to me, except for the minor issue in 3/6:

Reviewed-by: Fabian Ebner <f.ebner@proxmox.com>

Re: [pve-devel] [PATCH v6 manager 2/5] ui: utils: add nextFreeMP

[Fabian Ebner]

Am 05.04.22 um 14:30 schrieb Aaron Lauterer:
> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
> ---
>  www/manager6/Utils.js | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
> index 3351e53b..fe5be283 100644
> --- a/www/manager6/Utils.js
> +++ b/www/manager6/Utils.js
> @@ -1800,6 +1800,21 @@ Ext.define('PVE.Utils', {
>  
>  	return undefined;
>      },
> +
> +    nextFreeMP: function(type, config) {
> +	for (let i = 0; i < PVE.Utils.mp_counts[type]; i++) {
> +	    let confid = `${type}${i}`;
> +	    if (!Ext.isDefined(config[confid])) {
> +		return {
> +		    type,
> +		    id: i,
> +		    confid,
> +		};
> +	    }
> +	}
> +
> +	return undefined;
> +    },

Just noting that the getNextFreeDisk function in lxc/MultiMPEdit.js
could now re-use this.

[pve-devel] applied-series: [PATCH v6 manager 0/5] ui: lxc/qemu: disk/volume action simplify menu items

[Thomas Lamprecht]

On 05.04.22 14:30, Aaron Lauterer wrote:
> Aaron Lauterer (5):
>   ui: utils: refactor mps to mp
>   ui: utils: add nextFreeMP
>   ui: lxc/qemu: add disk reassign and action submenu
>   ui: BusTypeSelector: change noVirtIO to withVirtIO
>   ui: hdmove: modernize/refactor
> 
>  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           |  78 +++++--
>  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       |  75 +++++--
>  12 files changed, 499 insertions(+), 149 deletions(-)
>  create mode 100644 www/manager6/qemu/HDReassign.js
> 

applied series with merge conflict to current master fixed and a few
small followup on top, thanks!