all lists on lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH v5 manager 0/2] add edit window for device passthrough
@ 2024-04-17  8:44 Filip Schauer
  2024-04-17  8:44 ` [pve-devel] [PATCH v5 manager 1/2] utils: clarify naming of LXC mount point utils Filip Schauer
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Filip Schauer @ 2024-04-17  8:44 UTC (permalink / raw)
  To: pve-devel

Changes since v4:
* Simplify cbind
* Fix selection of custom devid not being applied on creation in
  onGetValues

Changes since v3:
* Pass confid in via cbind instead of manually setting it in the view
  model
* Check me.isCreate instead of !me.confid for whether to find the next
  free device slot

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           | 176 +++++++++++++++++++++++
 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, 232 insertions(+), 15 deletions(-)
 create mode 100644 www/manager6/lxc/DeviceEdit.js

-- 
2.39.2




^ permalink raw reply	[flat|nested] 5+ messages in thread

* [pve-devel] [PATCH v5 manager 1/2] utils: clarify naming of LXC mount point utils
  2024-04-17  8:44 [pve-devel] [PATCH v5 manager 0/2] add edit window for device passthrough Filip Schauer
@ 2024-04-17  8:44 ` Filip Schauer
  2024-04-17  8:44 ` [pve-devel] [PATCH v5 manager 2/2] ui: lxc: add edit window for device passthrough Filip Schauer
  2024-04-17  9:54 ` [pve-devel] applied: [PATCH v5 manager 0/2] " Thomas Lamprecht
  2 siblings, 0 replies; 5+ messages in thread
From: Filip Schauer @ 2024-04-17  8:44 UTC (permalink / raw)
  To: pve-devel

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 287d651a..8205598a 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -1590,13 +1590,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;
@@ -1607,7 +1607,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;
@@ -1871,8 +1871,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





^ permalink raw reply	[flat|nested] 5+ messages in thread

* [pve-devel] [PATCH v5 manager 2/2] ui: lxc: add edit window for device passthrough
  2024-04-17  8:44 [pve-devel] [PATCH v5 manager 0/2] add edit window for device passthrough Filip Schauer
  2024-04-17  8:44 ` [pve-devel] [PATCH v5 manager 1/2] utils: clarify naming of LXC mount point utils Filip Schauer
@ 2024-04-17  8:44 ` Filip Schauer
  2024-04-17  9:54 ` [pve-devel] applied: [PATCH v5 manager 0/2] " Thomas Lamprecht
  2 siblings, 0 replies; 5+ messages in thread
From: Filip Schauer @ 2024-04-17  8:44 UTC (permalink / raw)
  To: pve-devel

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

diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index c756cae6..5e16f2a5 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -188,6 +188,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 8205598a..2c3c56ca 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -1615,6 +1615,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..845c452c
--- /dev/null
+++ b/www/manager6/lxc/DeviceEdit.js
@@ -0,0 +1,176 @@
+Ext.define('PVE.lxc.DeviceInputPanel', {
+    extend: 'Proxmox.panel.InputPanel',
+    mixins: ['Proxmox.Mixin.CBind'],
+
+    autoComplete: false,
+
+    controller: {
+	xclass: 'Ext.app.ViewController',
+    },
+
+    setVMConfig: function(vmconfig) {
+	let me = this;
+	me.vmconfig = vmconfig;
+
+	if (me.isCreate) {
+	    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;
+	let confid = me.isCreate ? "dev" + values.devid : me.confid;
+	delete values.devid;
+	let val = PVE.Parser.printPropertyString(values, 'path');
+	let ret = {};
+	ret[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',
+	    cbind: {
+		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,
+	    isCreate: me.isCreate,
+	    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





^ permalink raw reply	[flat|nested] 5+ messages in thread

* [pve-devel] applied: [PATCH v5 manager 0/2] add edit window for device passthrough
  2024-04-17  8:44 [pve-devel] [PATCH v5 manager 0/2] add edit window for device passthrough Filip Schauer
  2024-04-17  8:44 ` [pve-devel] [PATCH v5 manager 1/2] utils: clarify naming of LXC mount point utils Filip Schauer
  2024-04-17  8:44 ` [pve-devel] [PATCH v5 manager 2/2] ui: lxc: add edit window for device passthrough Filip Schauer
@ 2024-04-17  9:54 ` Thomas Lamprecht
  2024-04-17 10:03   ` Filip Schauer
  2 siblings, 1 reply; 5+ messages in thread
From: Thomas Lamprecht @ 2024-04-17  9:54 UTC (permalink / raw)
  To: Proxmox VE development discussion, Filip Schauer

Am 17/04/2024 um 10:44 schrieb Filip Schauer:
> Changes since v4:
> * Simplify cbind
> * Fix selection of custom devid not being applied on creation in
>   onGetValues
> 
> Changes since v3:
> * Pass confid in via cbind instead of manually setting it in the view
>   model
> * Check me.isCreate instead of !me.confid for whether to find the next
>   free device slot
> 
> 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           | 176 +++++++++++++++++++++++
>  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, 232 insertions(+), 15 deletions(-)
>  create mode 100644 www/manager6/lxc/DeviceEdit.js
> 


applied, with a follow-up that made the forEachLxc{MP,Dev} methods also
pass the property ID directly to the closure, thanks!

I also made some clean-ups, e.g. dropping the right-alignment of the field
label, that look just way to odd..
Allowing manual control over the ID also seems to not provide much advantage
here, so I hid that field like we do for VM PCI passthrough.

Also noticed something not related to the UI side: if I enter some bogus path,
like `/dev/enoent`, I correctly get an error that this does not exist, but the
config entry is added nonetheless!
Which then also means that if I keep the dialogue open and correct the dev
path, I won't be able to submit as the config digest changed, I need to close
and re-open the add/edit dialogue again. This is not only bad UX, but seems
completely broken?

Also, if I got some bogus devX entries already, the error I get when saving
a new one is often from them, not from the one I add, but the change also
goes through here...

Please check that out.




^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [pve-devel] applied: [PATCH v5 manager 0/2] add edit window for device passthrough
  2024-04-17  9:54 ` [pve-devel] applied: [PATCH v5 manager 0/2] " Thomas Lamprecht
@ 2024-04-17 10:03   ` Filip Schauer
  0 siblings, 0 replies; 5+ messages in thread
From: Filip Schauer @ 2024-04-17 10:03 UTC (permalink / raw)
  To: Thomas Lamprecht, Proxmox VE development discussion

That is already fixed by this commit to pve-container:
https://git.proxmox.com/?p=pve-container.git;a=commitdiff;h=556ddd393165d51653fe32c1f8fe8628d1802219

On 17/04/2024 11:54, Thomas Lamprecht wrote:
> Also noticed something not related to the UI side: if I enter some bogus path,
> like `/dev/enoent`, I correctly get an error that this does not exist, but the
> config entry is added nonetheless!
> Which then also means that if I keep the dialogue open and correct the dev
> path, I won't be able to submit as the config digest changed, I need to close
> and re-open the add/edit dialogue again. This is not only bad UX, but seems
> completely broken?
>
> Also, if I got some bogus devX entries already, the error I get when saving
> a new one is often from them, not from the one I add, but the change also
> goes through here...
>
> Please check that out.




^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2024-04-17 10:04 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-17  8:44 [pve-devel] [PATCH v5 manager 0/2] add edit window for device passthrough Filip Schauer
2024-04-17  8:44 ` [pve-devel] [PATCH v5 manager 1/2] utils: clarify naming of LXC mount point utils Filip Schauer
2024-04-17  8:44 ` [pve-devel] [PATCH v5 manager 2/2] ui: lxc: add edit window for device passthrough Filip Schauer
2024-04-17  9:54 ` [pve-devel] applied: [PATCH v5 manager 0/2] " Thomas Lamprecht
2024-04-17 10:03   ` Filip Schauer

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal