[pve-devel] [PATCH series 0/4] Add GUI for disk reassignment

[pve-devel] [PATCH series 0/4] Add GUI for disk reassignment

[Aaron Lauterer]

This patch series adds the GUI to the recent patch series [0] which
enables the reassignment of disks between VMs.

For this to work, the previous patch series [0] needs to be applied and
installed.

[0] https://lists.proxmox.com/pipermail/pve-devel/2020-August/044668.html

widget-toolkit: Aaron Lauterer (1):
  window/edit: add option to disable reset button

 src/window/Edit.js | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)


manager: Aaron Lauterer (3):
  ui: qemu/HardwareView: white space fixes
  ui: utils: add method to get VM data from resource store
  ui: qemu: Add disk reassign dialog

 www/manager6/Makefile             |  1 +
 www/manager6/Utils.js             | 15 +++++-
 www/manager6/qemu/HDReassign.js   | 79 +++++++++++++++++++++++++++++++
 www/manager6/qemu/HardwareView.js | 40 ++++++++++++++--
 4 files changed, 129 insertions(+), 6 deletions(-)
 create mode 100644 www/manager6/qemu/HDReassign.js

-- 
2.20.1

[pve-devel] [PATCH widget-toolkit 1/4] window/edit: add option to disable reset button

[Aaron Lauterer]

Sometimes the reset button does not make sense and the isCreate option
does not fit as well because with it, the submit button will be enabled
right away instead of waiting for the form to be valid.

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

This helps to reuse the PVE.window.Edit component in a modern viewModel
and controller way instead of building it all by hand the old way with a
large initComponent method.

A possible candidate for refactoring after this would be the
qemu/HDMove.js component.

 src/window/Edit.js | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/window/Edit.js b/src/window/Edit.js
index 2dfab19..9ca314d 100644
--- a/src/window/Edit.js
+++ b/src/window/Edit.js
@@ -22,6 +22,9 @@ Ext.define('Proxmox.window.Edit', {
     // set to true if you want an Remove button (instead of Create)
     isRemove: false,
 
+    // set to false, if you don't want the reset button present
+    showReset: true,
+
     // custom submitText
     submitText: undefined,
 
@@ -326,7 +329,7 @@ Ext.define('Proxmox.window.Edit', {
 	    me.title = Proxmox.Utils.dialog_title(me.subject, me.isCreate, me.isAdd);
 	}
 
-	if (me.isCreate) {
+	if (me.isCreate || !me.showReset) {
 		me.buttons = [submitBtn];
 	} else {
 		me.buttons = [submitBtn, resetBtn];
-- 
2.20.1

[pve-devel] [PATCH manager 2/4] ui: qemu/HardwareView: white space fixes

[Aaron Lauterer]

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

diff --git a/www/manager6/qemu/HardwareView.js b/www/manager6/qemu/HardwareView.js
index 40b3fe86..b641317d 100644
--- a/www/manager6/qemu/HardwareView.js
+++ b/www/manager6/qemu/HardwareView.js
@@ -48,7 +48,7 @@ Ext.define('PVE.qemu.HardwareView', {
 	var i, confid;
 
 	var nodename = me.pveSelNode.data.node;
-	if (!nodename) { 
+	if (!nodename) {
 	    throw "no node name specified";
 	}
 
@@ -93,7 +93,7 @@ Ext.define('PVE.qemu.HardwareView', {
 	    sockets: {
 		header: gettext('Processors'),
 		never_delete: true,
-		editor: (caps.vms['VM.Config.CPU'] || caps.vms['VM.Config.HWType']) ? 
+		editor: (caps.vms['VM.Config.CPU'] || caps.vms['VM.Config.HWType']) ?
 		    'PVE.qemu.ProcessorEdit' : undefined,
 		tdCls: 'pve-itype-icon-processor',
 		group: 3,
@@ -151,7 +151,7 @@ Ext.define('PVE.qemu.HardwareView', {
 		iconCls: 'desktop',
 		group:5,
 		defaultValue: '',
-		renderer: PVE.Utils.render_kvm_vga_driver		
+		renderer: PVE.Utils.render_kvm_vga_driver
 	    },
 	    machine: {
 		header: gettext('Machine'),
@@ -318,7 +318,7 @@ Ext.define('PVE.qemu.HardwareView', {
 	    if ((g1 - g2) !== 0) {
 		return g1 - g2;
 	    }
-	    
+
 	    if ((order1 - order2) !== 0) {
 		return order1 - order2;
 	    }
@@ -612,7 +612,7 @@ Ext.define('PVE.qemu.HardwareView', {
 	    interval: 5000,
 	    selModel: sm,
 	    run_editor: run_editor,
-	    tbar: [ 
+	    tbar: [
 		{
 		    text: gettext('Add'),
 		    menu: new Ext.menu.Menu({
-- 
2.20.1

[pve-devel] [PATCH manager 3/4] ui: utils: add method to get VM data from resource store

[Aaron Lauterer]

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

diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index bf9ceda9..c96c60d4 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -1566,6 +1566,19 @@ Ext.define('PVE.Utils', { utilities: {
     constructor: function() {
 	var me = this;
 	Ext.apply(me, me.utilities);
-    }
+    },
+
+    getNodeVMs: function(nodename) {
+	let rstore = PVE.data.ResourceStore;
+	let vms = {}
+	rstore.data.items.forEach((item) => {
+	    if (!item.id.startsWith('qemu/')) { return; }
+	    let vmdata = item.data;
+	    if (vmdata.node != nodename) { return; }
+
+	    vms[vmdata.vmid] = vmdata;
+	});
+	return vms;
+    },
 
 });
-- 
2.20.1

Re: [pve-devel] [PATCH manager 3/4] ui: utils: add method to get VM data from resource store

[Dominic Jäger]

Not 100% sure if I currently have the right eslint version

On Tue, Aug 25, 2020 at 05:14:25PM +0200, Aaron Lauterer wrote:
> +    getNodeVMs: function(nodename) {
> +	let rstore = PVE.data.ResourceStore;
> +	let vms = {}
WARN: line 1573 col 14: semi - Missing semicolon. (*)
> +	rstore.data.items.forEach((item) => {
> +	    if (!item.id.startsWith('qemu/')) { return; }
> +	    let vmdata = item.data;
> +	    if (vmdata.node != nodename) { return; }
ERR : line 1577 col 22: eqeqeq - Expected '!==' and instead saw '!='.
> +
> +	    vms[vmdata.vmid] = vmdata;
> +	});
> +	return vms;
> +    },

Looking at the forEach: Functional programming can be fun :)
return Object.fromEntries(rstore.data.items.filter(x => x.id.startsWith('qemu/')).filter(x => x.data.node === nodename).map(x => [x.data.vmid, x.data]));

[pve-devel] [PATCH manager 4/4] ui: qemu: Add disk reassign dialog

[Aaron Lauterer]

Adds a new button to the hardware panel labeled 'Reassign disk' and
enables a user to reassign a disk to another VM.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
This patch needs the previous patch series [0] applied which added the
backend for disk reassignments.

[0] https://lists.proxmox.com/pipermail/pve-devel/2020-August/044668.html

 www/manager6/Makefile             |  1 +
 www/manager6/qemu/HDReassign.js   | 79 +++++++++++++++++++++++++++++++
 www/manager6/qemu/HardwareView.js | 30 ++++++++++++
 3 files changed, 110 insertions(+)
 create mode 100644 www/manager6/qemu/HDReassign.js

diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index 4288acdd..244f3c1b 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -181,6 +181,7 @@ JSSRC= 							\
 	qemu/HDEdit.js					\
 	qemu/HDEfi.js					\
 	qemu/HDMove.js					\
+	qemu/HDReassign.js				\
 	qemu/HDResize.js				\
 	qemu/HardwareView.js				\
 	qemu/IPConfigEdit.js				\
diff --git a/www/manager6/qemu/HDReassign.js b/www/manager6/qemu/HDReassign.js
new file mode 100644
index 00000000..3c5460fd
--- /dev/null
+++ b/www/manager6/qemu/HDReassign.js
@@ -0,0 +1,79 @@
+Ext.define('PVE.window.HDReassign', {
+    extend: 'Proxmox.window.Edit',
+
+    resizeable: false,
+    title: gettext('Reassign disk'),
+    submitText: gettext('Reassign disk'),
+    showReset: false,
+    method: 'POST',
+    showProgress: true,
+    width: 350,
+
+    viewModel: {
+	data: {
+	    targetVMData: {},
+	    show_running_hint: false,
+	},
+    },
+
+    items: [
+	{
+	    xtype: 'combobox',
+	    name: 'target_vmid',
+	    reference: 'target_vmid',
+	    fieldLabel: gettext('Target VMID'),
+	    bind: {
+		store: '{targetVMData}',
+	    },
+	    queryMode: 'local',
+	    displayField: 'name',
+	    valueField: 'vmid',
+	    anyMatch: true,
+	    emptyText: gettext('Select VM'),
+	},
+	{
+	    xtype: 'displayfield',
+	    padding: '5 0 0 0',
+	    userCls: 'pmx-hint',
+	    value: gettext('This disk cannot be reassigned while the VM is running'),
+	    bind: {
+		hidden: '{!show_running_hint}',
+	    },
+	},
+    ],
+
+    controller: {
+	xclass: 'Ext.app.ViewController',
+	init: function(view) {
+	    let me = view;
+	    let vm = me.getViewModel();
+	    let vms = PVE.Utils.getNodeVMs(me.nodename);
+
+	    let show_running_hint = vms[me.vmid].running && !me.disk.startsWith('unused');
+	    vm.set('show_running_hint', show_running_hint);
+
+	    if (show_running_hint) {
+	        me.lookup('target_vmid').setDisabled(true);
+	    }
+
+	    let dropdownData = [];
+	    for ([vmid, data] of Object.entries(vms)) {
+	        if (vmid === me.vmid) { continue; }
+	        dropdownData.push({
+		    vmid: vmid,
+		    name: `${vmid} ${data.name}`,
+	        });
+	    }
+
+	    vm.set('targetVMData', { data: dropdownData });
+	},
+    },
+
+    getValues: function() {
+	let me = this;
+	let values = me.callParent(arguments);
+	values.disk = me.disk;
+
+	return values;
+    },
+});
diff --git a/www/manager6/qemu/HardwareView.js b/www/manager6/qemu/HardwareView.js
index b641317d..909647dc 100644
--- a/www/manager6/qemu/HardwareView.js
+++ b/www/manager6/qemu/HardwareView.js
@@ -415,6 +415,25 @@ Ext.define('PVE.qemu.HardwareView', {
 	    win.on('destroy', me.reload, me);
 	};
 
+	var run_reassign = function() {
+	    let rec = sm.getSelection()[0];
+	    if (!rec) {
+		return;
+	    }
+
+	    let win = Ext.create('PVE.window.HDReassign', {
+		disk: rec.data.key,
+		vmid: vmid,
+		nodename: nodename,
+		title: gettext('Reassign disk') + ': ' + rec.data.key,
+		url: '/api2/extjs/nodes/' + nodename + '/qemu/' + vmid + '/reassign_disk',
+	    });
+
+	    win.show();
+
+	    win.on('destroy', me.reload, me);
+	};
+
 	var edit_btn = new Proxmox.button.Button({
 	    text: gettext('Edit'),
 	    selModel: sm,
@@ -436,6 +455,13 @@ Ext.define('PVE.qemu.HardwareView', {
 	    handler: run_move
 	});
 
+	var reassign_btn = new Proxmox.button.Button({
+	    text: gettext('Reassign disk'),
+	    selModel: sm,
+	    disabled: true,
+	    handler: run_reassign
+	});
+
 	var remove_btn = new Proxmox.button.Button({
 	    text: gettext('Remove'),
 	    defaultText: gettext('Remove'),
@@ -577,6 +603,7 @@ Ext.define('PVE.qemu.HardwareView', {
 		edit_btn.disable();
 		resize_btn.disable();
 		move_btn.disable();
+		reassign_btn.disable();
 		revert_btn.disable();
 		return;
 	    }
@@ -603,6 +630,8 @@ Ext.define('PVE.qemu.HardwareView', {
 
 	    move_btn.setDisabled(pending || !(isUsedDisk || isEfi) || !diskCap);
 
+	    reassign_btn.setDisabled(pending || !(isUsedDisk || isEfi || isUnusedDisk) || !diskCap);
+
 	    revert_btn.setDisabled(!pending);
 
 	};
@@ -752,6 +781,7 @@ Ext.define('PVE.qemu.HardwareView', {
 		edit_btn,
 		resize_btn,
 		move_btn,
+		reassign_btn,
 		revert_btn
 	    ],
 	    rows: rows,
-- 
2.20.1

Re: [pve-devel] [PATCH manager 4/4] ui: qemu: Add disk reassign dialog

[Dominic Jäger]

Somehow my eslint complains about undefined variables?
Maybe the "const" from the example [0] is required?

On Tue, Aug 25, 2020 at 05:14:26PM +0200, Aaron Lauterer wrote:
> +	    let dropdownData = [];
> +	    for ([vmid, data] of Object.entries(vms)) {
> +	        if (vmid === me.vmid) { continue; }
> +	        dropdownData.push({
> +		    vmid: vmid,
> +		    name: `${vmid} ${data.name}`,
> +	        });
> +	    }

[0] https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/entries

Re: [pve-devel] [PATCH series 0/4] Add GUI for disk reassignment

[Dominic Jäger]

I reassigned some disks and it seemed to work nice for me :)
Error messages about unavailable disks for running VMs and equal source and
target VM made sense to me, correct possible targets were displayed, multiple
storages worked.

One thing that came to my mind: It might make sense to display what name the
reassigned disk got in the target VM in the log.
For example "Reassigned to VM 123 as Unused Disk 7 [maybe with volid]".
Because if there are multiple unused disks on the target VM it could happen
that it's not the one with the highest number?

On Tue, Aug 25, 2020 at 05:14:22PM +0200, Aaron Lauterer wrote:
> This patch series adds the GUI to the recent patch series [0] which
> enables the reassignment of disks between VMs.
> 
> For this to work, the previous patch series [0] needs to be applied and
> installed.

Re: [pve-devel] [PATCH series 0/4] Add GUI for disk reassignment

[Aaron Lauterer]

Thx for the review and the problems found.


On 8/31/20 1:04 PM, Dominic Jäger wrote:
> I reassigned some disks and it seemed to work nice for me :)
> Error messages about unavailable disks for running VMs and equal source and
> target VM made sense to me, correct possible targets were displayed, multiple
> storages worked.
> 
> One thing that came to my mind: It might make sense to display what name the
> reassigned disk got in the target VM in the log.
> For example "Reassigned to VM 123 as Unused Disk 7 [maybe with volid]".
> Because if there are multiple unused disks on the target VM it could happen
> that it's not the one with the highest number?


Sounds good. I will incorporate that in the other patch series that does deal with the backend.


> 
> On Tue, Aug 25, 2020 at 05:14:22PM +0200, Aaron Lauterer wrote:
>> This patch series adds the GUI to the recent patch series [0] which
>> enables the reassignment of disks between VMs.
>>
>> For this to work, the previous patch series [0] needs to be applied and
>> installed.