[pve-devel] [PATCH v2 manager] ui: lxc: add edit window for device passthrough

[pve-devel] [PATCH v2 manager] ui: lxc: add edit window for device passthrough

[Filip Schauer]

Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
---
Changes since v1:
* Remove usb mapping
* Add mode, uid and gid fields

To get this to build on top of the current master cd731902b7a7,
comment out the call to $notification_config->add_matcher
in PVE/VZDump.pm for now.

 www/manager6/Makefile          |   1 +
 www/manager6/Utils.js          |  11 +++
 www/manager6/lxc/DeviceEdit.js | 166 +++++++++++++++++++++++++++++++++
 www/manager6/lxc/Resources.js  |  28 +++++-
 4 files changed, 205 insertions(+), 1 deletion(-)
 create mode 100644 www/manager6/lxc/DeviceEdit.js

diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index ee09f0b8..e18bb66e 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -185,6 +185,7 @@ JSSRC= 							\
 	lxc/CmdMenu.js					\
 	lxc/Config.js					\
 	lxc/CreateWizard.js				\
+	lxc/DeviceEdit.js				\
 	lxc/DNS.js					\
 	lxc/FeaturesEdit.js				\
 	lxc/MPEdit.js					\
diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index 9f44e560..f028685b 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -1605,6 +1605,17 @@ Ext.define('PVE.Utils', {
 	}
     },
 
+    dev_count: 256,
+
+    forEachDev: function(func) {
+	for (let i = 0; i < PVE.Utils.dev_count; i++) {
+	    let cont = func(i);
+	    if (!cont && cont !== undefined) {
+		return;
+	    }
+	}
+    },
+
     hardware_counts: {
 	net: 32,
 	usb: 14,
diff --git a/www/manager6/lxc/DeviceEdit.js b/www/manager6/lxc/DeviceEdit.js
new file mode 100644
index 00000000..1ee4f309
--- /dev/null
+++ b/www/manager6/lxc/DeviceEdit.js
@@ -0,0 +1,166 @@
+Ext.define('PVE.lxc.DeviceInputPanel', {
+    extend: 'Proxmox.panel.InputPanel',
+    mixins: ['Proxmox.Mixin.CBind'],
+
+    autoComplete: false,
+
+    cbindData: function(initialConfig) {
+	let me = this;
+	if (!me.pveSelNode) {
+	    throw "no pveSelNode given";
+	}
+
+	return { nodename: me.pveSelNode.data.node };
+    },
+
+    viewModel: {
+	data: {},
+    },
+
+    setVMConfig: function(vmconfig) {
+	var me = this;
+	me.vmconfig = vmconfig;
+    },
+
+    onGetValues: function(values) {
+	var me = this;
+	if (!me.confid) {
+	    let max_devices = 256;
+	    for (let i = 0; i < max_devices; i++) {
+		let id = 'dev' + i.toString();
+		if (!me.vmconfig[id]) {
+		    me.confid = id;
+		    break;
+		}
+	    }
+	}
+	var val = values.path;
+	delete values.path;
+
+	if (values.mode) {
+	    val += ',mode=' + values.mode;
+	}
+	delete values.mode;
+
+	if (values.uid) {
+	    val += ',uid=' + values.uid;
+	}
+	delete values.uid;
+
+	if (values.gid) {
+	    val += ',gid=' + values.gid;
+	}
+	delete values.gid;
+
+	values[me.confid] = val;
+	return values;
+    },
+
+    items: [
+	{
+	    xtype: 'textfield',
+	    type: 'device',
+	    name: 'path',
+	    cbind: { pveSelNode: '{pveSelNode}' },
+	    editable: true,
+	    allowBlank: false,
+	    fieldLabel: gettext('Device Path'),
+	    labelAlign: 'right',
+	    validator: function(value) {
+		if (value.startsWith('/dev/')) {
+		    return true;
+		}
+
+		return "Path has to start with /dev/";
+	    },
+	},
+    ],
+
+    advancedColumn1: [
+	{
+	    xtype: 'textfield',
+	    name: 'mode',
+	    cbind: { pveSelNode: '{pveSelNode}' },
+	    editable: true,
+	    fieldLabel: gettext('Access Mode'),
+	    labelAlign: 'right',
+	    validator: function(value) {
+		if (/^0[0-7]{3}$|^$/i.test(value)) {
+		    return true;
+		}
+
+		return "Access mode has to be an octal number";
+	    },
+	},
+	{
+	    xtype: 'proxmoxintegerfield',
+	    name: 'uid',
+	    cbind: { pveSelNode: '{pveSelNode}' },
+	    editable: true,
+	    fieldLabel: 'UID',
+	    minValue: 0,
+	    labelAlign: 'right',
+	},
+	{
+	    xtype: 'proxmoxintegerfield',
+	    name: 'gid',
+	    cbind: { pveSelNode: '{pveSelNode}' },
+	    editable: true,
+	    fieldLabel: 'GID',
+	    minValue: 0,
+	    labelAlign: 'right',
+	},
+    ],
+});
+
+Ext.define('PVE.lxc.DeviceEdit', {
+    extend: 'Proxmox.window.Edit',
+
+    vmconfig: undefined,
+
+    isAdd: true,
+    width: 400,
+    subject: gettext('Device'),
+
+    initComponent: function() {
+	var me = this;
+
+	me.isCreate = !me.confid;
+
+	var ipanel = Ext.create('PVE.lxc.DeviceInputPanel', {
+	    confid: me.confid,
+	    pveSelNode: me.pveSelNode,
+	});
+
+	Ext.apply(me, {
+	    items: [ipanel],
+	});
+
+	me.callParent();
+
+	me.load({
+	    success: function(response, options) {
+		ipanel.setVMConfig(response.result.data);
+		if (me.isCreate) {
+		    return;
+		}
+
+		let data = PVE.Parser.parsePropertyString(response.result.data[me.confid], 'path');
+		let path, mode, uid, gid;
+		path = data.path;
+		mode = data.mode;
+		uid = data.uid;
+		gid = data.gid;
+
+		var values = {
+		    path,
+		    mode,
+		    uid,
+		    gid,
+		};
+
+		ipanel.setValues(values);
+	    },
+	});
+    },
+});
diff --git a/www/manager6/lxc/Resources.js b/www/manager6/lxc/Resources.js
index 85112345..9dcb74eb 100644
--- a/www/manager6/lxc/Resources.js
+++ b/www/manager6/lxc/Resources.js
@@ -135,6 +135,17 @@ Ext.define('PVE.lxc.RessourceView', {
 	    };
 	}, true);
 
+	PVE.Utils.forEachDev(function(i) {
+	    confid = 'dev' + i;
+	    rows[confid] = {
+		group: 7,
+		order: i,
+		tdCls: 'pve-itype-icon-pci',
+		editor: 'PVE.lxc.DeviceEdit',
+		header: gettext('Device') + ' (' + confid + ')',
+	    };
+	});
+
 	var baseurl = 'nodes/' + nodename + '/lxc/' + vmid + '/config';
 
 	me.selModel = Ext.create('Ext.selection.RowModel', {});
@@ -311,6 +322,7 @@ Ext.define('PVE.lxc.RessourceView', {
 	    let isDisk = isRootFS || key.match(/^(mp|unused)\d+/);
 	    let isUnusedDisk = key.match(/^unused\d+/);
 	    let isUsedDisk = isDisk && !isUnusedDisk;
+	    let isDevice = key.match(/^dev\d+/);
 
 	    let noedit = isDelete || !rowdef.editor;
 	    if (!noedit && Proxmox.UserName !== 'root@pam' && key.match(/^mp\d+$/)) {
@@ -326,7 +338,7 @@ Ext.define('PVE.lxc.RessourceView', {
 	    reassign_menuitem.setDisabled(isRootFS);
 	    resize_menuitem.setDisabled(isUnusedDisk);
 
-	    remove_btn.setDisabled(!isDisk || isRootFS || !diskCap || pending);
+	    remove_btn.setDisabled(!(isDisk || isDevice) || isRootFS || !diskCap || pending);
 	    revert_btn.setDisabled(!pending);
 
 	    remove_btn.setText(isUsedDisk ? remove_btn.altText : remove_btn.defaultText);
@@ -380,6 +392,20 @@ Ext.define('PVE.lxc.RessourceView', {
 				    });
 				},
 			    },
+			    {
+				text: gettext('Device Passthrough'),
+				iconCls: 'pve-itype-icon-pci',
+				handler: function() {
+				    Ext.create('PVE.lxc.DeviceEdit', {
+					autoShow: true,
+					url: `/api2/extjs/${baseurl}`,
+					pveSelNode: me.pveSelNode,
+					listeners: {
+					    destroy: () => me.reload(),
+					},
+				    });
+				},
+			    },
 			],
 		    }),
 		},
-- 
2.39.2

Re: [pve-devel] [PATCH v2 manager] ui: lxc: add edit window for device passthrough

[Fiona Ebner]

Am 21.11.23 um 11:21 schrieb Filip Schauer:
> Signed-off-by: Filip Schauer <f.schauer@proxmox.com>

High-level comment:
- would be nice to have the default values as emptyText for
UID/GID/Acces Mode and maybe also have an example text /dev/XYZ for the path
- if I add /dev/doesnotexist I'll get an error but it'll still be added
to the configuration
- I think the device index should be selectible too, to make it more
consistent with adding a mount point
- There's quite a bit of empty space on the right side in the advanced
options, maybe move one of the fields there?
- The menu entry for "Add" should be hidden or disabled for users
without sufficient permissions.
- Maybe add a warning, because it can be dangerous. Should it even be
exposed in the UI?
- Style nit: 'var' shouldn't be used, see
https://pve.proxmox.com/wiki/Javascript_Style_Guide#Variables

> diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
> index 9f44e560..f028685b 100644
> --- a/www/manager6/Utils.js
> +++ b/www/manager6/Utils.js
> @@ -1605,6 +1605,17 @@ Ext.define('PVE.Utils', {
>  	}
>      },
>  
> +    dev_count: 256,
> +
> +    forEachDev: function(func) {
> +	for (let i = 0; i < PVE.Utils.dev_count; i++) {
> +	    let cont = func(i);
> +	    if (!cont && cont !== undefined) {
> +		return;
> +	    }
> +	}
> +    },
> +
>      hardware_counts: {
>  	net: 32,
>  	usb: 14,

Maybe it's time to finally split PVE.Utils? dev_count and forEachDev is
really too generic of a name for such a module, should be at least
mention "lxc".

> diff --git a/www/manager6/lxc/DeviceEdit.js b/www/manager6/lxc/DeviceEdit.js
> new file mode 100644
> index 00000000..1ee4f309
> --- /dev/null
> +++ b/www/manager6/lxc/DeviceEdit.js
> @@ -0,0 +1,166 @@
> +Ext.define('PVE.lxc.DeviceInputPanel', {
> +    extend: 'Proxmox.panel.InputPanel',
> +    mixins: ['Proxmox.Mixin.CBind'],
> +
> +    autoComplete: false,
> +
> +    cbindData: function(initialConfig) {
> +	let me = this;
> +	if (!me.pveSelNode) {
> +	    throw "no pveSelNode given";
> +	}
> +
> +	return { nodename: me.pveSelNode.data.node };
> +    },
> +
> +    viewModel: {
> +	data: {},
> +    },
> +
> +    setVMConfig: function(vmconfig) {
> +	var me = this;
> +	me.vmconfig = vmconfig;
> +    },
> +
> +    onGetValues: function(values) {
> +	var me = this;
> +	if (!me.confid) {
> +	    let max_devices = 256;

Could use the dev_count from PVE.Util

> +	    for (let i = 0; i < max_devices; i++) {
> +		let id = 'dev' + i.toString();
> +		if (!me.vmconfig[id]) {
> +		    me.confid = id;
> +		    break;
> +		}
> +	    }
> +	}
> +	var val = values.path;
> +	delete values.path;
> +
> +	if (values.mode) {
> +	    val += ',mode=' + values.mode;
> +	}
> +	delete values.mode;
> +
> +	if (values.uid) {
> +	    val += ',uid=' + values.uid;
> +	}
> +	delete values.uid;
> +
> +	if (values.gid) {
> +	    val += ',gid=' + values.gid;
> +	}
> +	delete values.gid;
> +

I think you can use PVE.Parser.printPropertyString() to save some lines
here.

> +	values[me.confid] = val;
> +	return values;
> +    },
> +
> +    items: [
> +	{
> +	    xtype: 'textfield',
> +	    type: 'device',
> +	    name: 'path',
> +	    cbind: { pveSelNode: '{pveSelNode}' },

I might be missing something, but isn't this a normal ExtJS text field?
Does this cbind have any actual consequences? Same for the others.

> +	    editable: true,
> +	    allowBlank: false,
> +	    fieldLabel: gettext('Device Path'),
> +	    labelAlign: 'right',
> +	    validator: function(value) {
> +		if (value.startsWith('/dev/')) {
> +		    return true;
> +		}
> +
> +		return "Path has to start with /dev/";
> +	    },
> +	},
> +    ],
> +
>

---cut---

> +	me.load({
> +	    success: function(response, options) {
> +		ipanel.setVMConfig(response.result.data);
> +		if (me.isCreate) {
> +		    return;
> +		}
> +
> +		let data = PVE.Parser.parsePropertyString(response.result.data[me.confid], 'path');
> +		let path, mode, uid, gid;
> +		path = data.path;
> +		mode = data.mode;
> +		uid = data.uid;
> +		gid = data.gid;
> +
> +		var values = {
> +		    path,
> +		    mode,
> +		    uid,
> +		    gid,
> +		};
> +
> +		ipanel.setValues(values);

Couldn't you pass data directly? Or at least make constructing values
quite a bit shorter ;)

Re: [pve-devel] [PATCH v2 manager] ui: lxc: add edit window for device passthrough

[Filip Schauer]

This is a bug in pve-container. I sent a patch for it:

https://lists.proxmox.com/pipermail/pve-devel/2024-January/061513.html


On 26/01/2024 16:23, Fiona Ebner wrote:
> - if I add /dev/doesnotexist I'll get an error but it'll still be added
> to the configuration

Re: [pve-devel] [PATCH v2 manager] ui: lxc: add edit window for device passthrough

[Filip Schauer]

On 26/01/2024 16:23, Fiona Ebner wrote:
> I might be missing something, but isn't this a normal ExtJS text field?
> Does this cbind have any actual consequences? Same for the others.

This cbind is indeed not needed here.

A patch v3 that incorporates your feedback is available:

https://lists.proxmox.com/pipermail/pve-devel/2024-January/061578.html