[pve-devel] [PATCH v3 manager 0/2] add edit window for device passthrough

[pve-devel] [PATCH v3 manager 0/2] add edit window for device passthrough

[Filip Schauer]

Changes since v2:
* Clarify naming of mount point and device passthrough related utils
* Remove unnecessary cbind
* Make the device index selectible
* Add default values as emptyText to the UI elements
* Reorder UI elements to improve the layout
* Disable the Device Passthrough menu entries for non-root users
* Change var to let
* Minor code cleanup of DeviceEdit.js

Changes since v1:
* Remove usb mapping
* Add mode, uid and gid fields

Filip Schauer (2):
  utils: clarify naming of LXC mount point utils
  ui: lxc: add edit window for device passthrough

 www/manager6/Makefile                    |   1 +
 www/manager6/Utils.js                    |  23 ++-
 www/manager6/lxc/DeviceEdit.js           | 190 +++++++++++++++++++++++
 www/manager6/lxc/MPEdit.js               |   4 +-
 www/manager6/lxc/MultiMPEdit.js          |   4 +-
 www/manager6/lxc/Resources.js            |  33 +++-
 www/manager6/window/GuestDiskReassign.js |   6 +-
 7 files changed, 246 insertions(+), 15 deletions(-)
 create mode 100644 www/manager6/lxc/DeviceEdit.js

-- 
2.39.2

[pve-devel] [PATCH v3 manager 1/2] utils: clarify naming of LXC mount point utils

[Filip Schauer]

Clarify the naming of mount point utils to clearly indicate their
relation to LXC containers.

Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
---
 www/manager6/Utils.js                    | 12 ++++++------
 www/manager6/lxc/MPEdit.js               |  4 ++--
 www/manager6/lxc/MultiMPEdit.js          |  4 ++--
 www/manager6/lxc/Resources.js            |  2 +-
 www/manager6/window/GuestDiskReassign.js |  6 +++---
 5 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index 9f44e560..cb460201 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -1580,13 +1580,13 @@ Ext.define('PVE.Utils', {
 	}
     },
 
-    mp_counts: {
+    lxc_mp_counts: {
 	mp: 256,
 	unused: 256,
     },
 
-    forEachMP: function(func, includeUnused) {
-	for (let i = 0; i < PVE.Utils.mp_counts.mp; i++) {
+    forEachLxcMP: function(func, includeUnused) {
+	for (let i = 0; i < PVE.Utils.lxc_mp_counts.mp; i++) {
 	    let cont = func('mp', i);
 	    if (!cont && cont !== undefined) {
 		return;
@@ -1597,7 +1597,7 @@ Ext.define('PVE.Utils', {
 	    return;
 	}
 
-	for (let i = 0; i < PVE.Utils.mp_counts.unused; i++) {
+	for (let i = 0; i < PVE.Utils.lxc_mp_counts.unused; i++) {
 	    let cont = func('unused', i);
 	    if (!cont && cont !== undefined) {
 		return;
@@ -1861,8 +1861,8 @@ Ext.define('PVE.Utils', {
 	return undefined;
     },
 
-    nextFreeMP: function(type, config) {
-	for (let i = 0; i < PVE.Utils.mp_counts[type]; i++) {
+    nextFreeLxcMP: function(type, config) {
+	for (let i = 0; i < PVE.Utils.lxc_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 609447ef..1ea0f10e 100644
--- a/www/manager6/lxc/MPEdit.js
+++ b/www/manager6/lxc/MPEdit.js
@@ -87,7 +87,7 @@ Ext.define('PVE.lxc.MountPointInputPanel', {
 	let me = this;
 
 	me.updateVMConfig(vmconfig);
-	PVE.Utils.forEachMP((bus, i) => {
+	PVE.Utils.forEachLxcMP((bus, i) => {
 	    let name = "mp" + i.toString();
 	    if (!Ext.isDefined(vmconfig[name])) {
 		me.down('field[name=mpid]').setValue(i);
@@ -194,7 +194,7 @@ Ext.define('PVE.lxc.MountPointInputPanel', {
 	    name: 'mpid',
 	    fieldLabel: gettext('Mount Point ID'),
 	    minValue: 0,
-	    maxValue: PVE.Utils.mp_counts.mp - 1,
+	    maxValue: PVE.Utils.lxc_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 36cee4ff..d13dc81f 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.mp + 1,
+	maxCount: PVE.Utils.lxc_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.mp; i++) {
+		for (let i = 0; i < PVE.Utils.lxc_mp_counts.mp; i++) {
 		    let confid = `mp${i}`;
 		    if (!vmconfig[confid]) {
 			nextFreeDisk = {
diff --git a/www/manager6/lxc/Resources.js b/www/manager6/lxc/Resources.js
index 85112345..61182ef3 100644
--- a/www/manager6/lxc/Resources.js
+++ b/www/manager6/lxc/Resources.js
@@ -116,7 +116,7 @@ Ext.define('PVE.lxc.RessourceView', {
 	    },
 	};
 
-	PVE.Utils.forEachMP(function(bus, i) {
+	PVE.Utils.forEachLxcMP(function(bus, i) {
 	    confid = bus + i;
 	    var group = 5;
 	    var header;
diff --git a/www/manager6/window/GuestDiskReassign.js b/www/manager6/window/GuestDiskReassign.js
index f6d08b32..405e7c9e 100644
--- a/www/manager6/window/GuestDiskReassign.js
+++ b/www/manager6/window/GuestDiskReassign.js
@@ -17,8 +17,8 @@ Ext.define('PVE.window.GuestDiskReassign', {
 	},
 	formulas: {
 	    mpMaxCount: get => get('mpType') === 'mp'
-		? PVE.Utils.mp_counts.mps - 1
-		: PVE.Utils.mp_counts.unused - 1,
+		? PVE.Utils.lxc_mp_counts.mps - 1
+		: PVE.Utils.lxc_mp_counts.unused - 1,
 	},
     },
 
@@ -103,7 +103,7 @@ Ext.define('PVE.window.GuestDiskReassign', {
 			view.VMConfig = result.data;
 
 			mpIdSelector.setValue(
-			    PVE.Utils.nextFreeMP(
+			    PVE.Utils.nextFreeLxcMP(
 				view.getViewModel().get('mpType'),
 				view.VMConfig,
 			    ).id,
-- 
2.39.2

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

[Filip Schauer]

Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
---
 www/manager6/Makefile          |   1 +
 www/manager6/Utils.js          |  11 ++
 www/manager6/lxc/DeviceEdit.js | 190 +++++++++++++++++++++++++++++++++
 www/manager6/lxc/Resources.js  |  31 +++++-
 4 files changed, 232 insertions(+), 1 deletion(-)
 create mode 100644 www/manager6/lxc/DeviceEdit.js

diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index dc3c85b1..ca236240 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -187,6 +187,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 cb460201..57528d25 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -1605,6 +1605,17 @@ Ext.define('PVE.Utils', {
 	}
     },
 
+    lxc_dev_count: 256,
+
+    forEachLxcDev: function(func) {
+	for (let i = 0; i < PVE.Utils.lxc_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..445f8607
--- /dev/null
+++ b/www/manager6/lxc/DeviceEdit.js
@@ -0,0 +1,190 @@
+Ext.define('PVE.lxc.DeviceInputPanel', {
+    extend: 'Proxmox.panel.InputPanel',
+
+    autoComplete: false,
+
+    controller: {
+	xclass: 'Ext.app.ViewController',
+	init: function(view) {
+	    let me = this;
+	    let vm = this.getViewModel();
+	    vm.set('confid', view.confid);
+	},
+    },
+
+    viewModel: {
+	data: {
+	    confid: '',
+	},
+
+	formulas: {
+	    isCreate: function(get) {
+		return !get('confid');
+	    },
+	},
+    },
+
+    setVMConfig: function(vmconfig) {
+	let me = this;
+	me.vmconfig = vmconfig;
+
+	if (!me.confid) {
+	    PVE.Utils.forEachLxcDev((i) => {
+		let name = "dev" + i.toString();
+		if (!Ext.isDefined(vmconfig[name])) {
+		    me.confid = name;
+		    me.down('field[name=devid]').setValue(i);
+		    return false;
+		}
+		return undefined;
+	    });
+	}
+    },
+
+    onGetValues: function(values) {
+	let me = this;
+	delete values.devid;
+	let val = PVE.Parser.printPropertyString(values, 'path');
+	let ret = {};
+	ret[me.confid] = val;
+	return ret;
+    },
+
+    items: [
+	{
+	    xtype: 'proxmoxintegerfield',
+	    name: 'devid',
+	    fieldLabel: gettext('Passthrough ID'),
+	    minValue: 0,
+	    maxValue: PVE.Utils.dev_count - 1,
+	    hidden: true,
+	    allowBlank: false,
+	    disabled: true,
+	    labelAlign: 'right',
+	    bind: {
+		hidden: '{!isCreate}',
+		disabled: '{!isCreate}',
+	    },
+	    validator: function(value) {
+		let view = this.up('inputpanel');
+		if (!view.vmconfig) {
+		    return undefined;
+		}
+		if (Ext.isDefined(view.vmconfig["dev" + value])) {
+		    return "Device passthrough is already in use.";
+		}
+		return true;
+	    },
+	},
+	{
+	    xtype: 'textfield',
+	    type: 'device',
+	    name: 'path',
+	    editable: true,
+	    allowBlank: false,
+	    fieldLabel: gettext('Device Path'),
+	    emptyText: '/dev/xyz',
+	    labelAlign: 'right',
+	    validator: function(value) {
+		if (value.startsWith('/dev/')) {
+		    return true;
+		}
+
+		return "Path has to start with /dev/";
+	    },
+	},
+    ],
+
+    advancedColumn1: [
+	{
+	    xtype: 'proxmoxintegerfield',
+	    name: 'uid',
+	    editable: true,
+	    fieldLabel: 'UID',
+	    emptyText: '0',
+	    minValue: 0,
+	    labelAlign: 'right',
+	},
+	{
+	    xtype: 'proxmoxintegerfield',
+	    name: 'gid',
+	    editable: true,
+	    fieldLabel: 'GID',
+	    emptyText: '0',
+	    minValue: 0,
+	    labelAlign: 'right',
+	},
+    ],
+
+    advancedColumn2: [
+	{
+	    xtype: 'textfield',
+	    name: 'mode',
+	    editable: true,
+	    fieldLabel: gettext('Access Mode'),
+	    emptyText: '0660',
+	    labelAlign: 'right',
+	    validator: function(value) {
+		if (/^0[0-7]{3}$|^$/i.test(value)) {
+		    return true;
+		}
+
+		return "Access mode has to be an octal number";
+	    },
+	},
+    ],
+});
+
+Ext.define('PVE.lxc.DeviceEdit', {
+    extend: 'Proxmox.window.Edit',
+
+    vmconfig: undefined,
+
+    isAdd: true,
+    width: 400,
+
+    initComponent: function() {
+	let me = this;
+
+	me.isCreate = !me.confid;
+
+	let ipanel = Ext.create('PVE.lxc.DeviceInputPanel', {
+	    confid: me.confid,
+	    pveSelNode: me.pveSelNode,
+	});
+
+	let subject;
+	if (me.isCreate) {
+	    subject = gettext('Device');
+	} else {
+	    subject = gettext('Device') + ' (' + me.confid + ')';
+	}
+
+	Ext.apply(me, {
+	    subject: subject,
+	    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 values = {
+		    path: data.path,
+		    mode: data.mode,
+		    uid: data.uid,
+		    gid: data.gid,
+		};
+
+		ipanel.setValues(values);
+	    },
+	});
+    },
+});
diff --git a/www/manager6/lxc/Resources.js b/www/manager6/lxc/Resources.js
index 61182ef3..8203d32b 100644
--- a/www/manager6/lxc/Resources.js
+++ b/www/manager6/lxc/Resources.js
@@ -135,6 +135,19 @@ Ext.define('PVE.lxc.RessourceView', {
 	    };
 	}, true);
 
+	let deveditor = Proxmox.UserName === 'root@pam' ? 'PVE.lxc.DeviceEdit' : undefined;
+
+	PVE.Utils.forEachLxcDev(function(i) {
+	    confid = 'dev' + i;
+	    rows[confid] = {
+		group: 7,
+		order: i,
+		tdCls: 'pve-itype-icon-pci',
+		editor: deveditor,
+		header: gettext('Device') + ' (' + confid + ')',
+	    };
+	});
+
 	var baseurl = 'nodes/' + nodename + '/lxc/' + vmid + '/config';
 
 	me.selModel = Ext.create('Ext.selection.RowModel', {});
@@ -311,6 +324,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 +340,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 +394,21 @@ Ext.define('PVE.lxc.RessourceView', {
 				    });
 				},
 			    },
+			    {
+				text: gettext('Device Passthrough'),
+				iconCls: 'pve-itype-icon-pci',
+				disabled: Proxmox.UserName !== 'root@pam',
+				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 v3 manager 1/2] utils: clarify naming of LXC mount point utils

[Fiona Ebner]

Am 31.01.24 um 16:03 schrieb Filip Schauer:
> Clarify the naming of mount point utils to clearly indicate their
> relation to LXC containers.
> 
> Signed-off-by: Filip Schauer <f.schauer@proxmox.com>

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

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

[Fiona Ebner]

Am 31.01.24 um 16:03 schrieb Filip Schauer:
> diff --git a/www/manager6/lxc/DeviceEdit.js b/www/manager6/lxc/DeviceEdit.js
> new file mode 100644
> index 00000000..445f8607
> --- /dev/null
> +++ b/www/manager6/lxc/DeviceEdit.js
> @@ -0,0 +1,190 @@
> +Ext.define('PVE.lxc.DeviceInputPanel', {
> +    extend: 'Proxmox.panel.InputPanel',
> +
> +    autoComplete: false,
> +
> +    controller: {
> +	xclass: 'Ext.app.ViewController',
> +	init: function(view) {
> +	    let me = this;
> +	    let vm = this.getViewModel();
> +	    vm.set('confid', view.confid);

Nit: Is the confid in the viewModel (and therefore the whole view model)
only used for the isCreate formula? That could also be directly passed
in and bound via cbind instead.

> +	},
> +    },
> +
> +    viewModel: {
> +	data: {
> +	    confid: '',
> +	},
> +
> +	formulas: {
> +	    isCreate: function(get) {
> +		return !get('confid');
> +	    },
> +	},
> +    },
> +
> +    setVMConfig: function(vmconfig) {
> +	let me = this;
> +	me.vmconfig = vmconfig;
> +
> +	if (!me.confid) {

Nit: Should this be guarded by isCreate instead? If for whatever reason
setVMConfig() would be called a second time with a different config (not
currently happenening AFAICT), I suppose it would make sense to pick the
first free slot based on the new config again?

> +	    PVE.Utils.forEachLxcDev((i) => {
> +		let name = "dev" + i.toString();
> +		if (!Ext.isDefined(vmconfig[name])) {
> +		    me.confid = name;
> +		    me.down('field[name=devid]').setValue(i);
> +		    return false;
> +		}
> +		return undefined;
> +	    });
> +	}
> +    },
> +

(...)

> +
> +    advancedColumn2: [
> +	{
> +	    xtype: 'textfield',
> +	    name: 'mode',
> +	    editable: true,
> +	    fieldLabel: gettext('Access Mode'),
> +	    emptyText: '0660',
> +	    labelAlign: 'right',
> +	    validator: function(value) {
> +		if (/^0[0-7]{3}$|^$/i.test(value)) {

Should we require the leading zero here? Many users will be familiar
with chown, where it is not required.

> +		    return true;
> +		}
> +
> +		return "Access mode has to be an octal number";
> +	    },
> +	},
> +    ],
> +});
> +

With that said:

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

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

[Filip Schauer]

On 15/04/2024 14:12, Fiona Ebner wrote:
>> +
>> +    advancedColumn2: [
>> +	{
>> +	    xtype: 'textfield',
>> +	    name: 'mode',
>> +	    editable: true,
>> +	    fieldLabel: gettext('Access Mode'),
>> +	    emptyText: '0660',
>> +	    labelAlign: 'right',
>> +	    validator: function(value) {
>> +		if (/^0[0-7]{3}$|^$/i.test(value)) {
> Should we require the leading zero here? Many users will be familiar
> with chown, where it is not required.

We already require the access mode to have a leading zero in
pve-container.

Re: [pve-devel] [PATCH v3 manager 0/2] add edit window for device passthrough

[Filip Schauer]

Superseded by:
https://lists.proxmox.com/pipermail/pve-devel/2024-April/063002.html

On 31/01/2024 16:03, Filip Schauer wrote:
> Changes since v2:
> * Clarify naming of mount point and device passthrough related utils
> * Remove unnecessary cbind
> * Make the device index selectible
> * Add default values as emptyText to the UI elements
> * Reorder UI elements to improve the layout
> * Disable the Device Passthrough menu entries for non-root users
> * Change var to let
> * Minor code cleanup of DeviceEdit.js
>
> Changes since v1:
> * Remove usb mapping
> * Add mode, uid and gid fields
>
> Filip Schauer (2):
>    utils: clarify naming of LXC mount point utils
>    ui: lxc: add edit window for device passthrough
>
>   www/manager6/Makefile                    |   1 +
>   www/manager6/Utils.js                    |  23 ++-
>   www/manager6/lxc/DeviceEdit.js           | 190 +++++++++++++++++++++++
>   www/manager6/lxc/MPEdit.js               |   4 +-
>   www/manager6/lxc/MultiMPEdit.js          |   4 +-
>   www/manager6/lxc/Resources.js            |  33 +++-
>   www/manager6/window/GuestDiskReassign.js |   6 +-
>   7 files changed, 246 insertions(+), 15 deletions(-)
>   create mode 100644 www/manager6/lxc/DeviceEdit.js
>