From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id E030C1FF0E0 for ; Thu, 23 Jul 2026 17:03:23 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 2E18021469; Thu, 23 Jul 2026 17:03:23 +0200 (CEST) From: Elias Huhsovitz To: pve-devel@lists.proxmox.com Subject: [PATCH manager v2] fix #6855: ui: storage: add mount options field for nfs amd smb/cifs Date: Thu, 23 Jul 2026 17:03:16 +0200 Message-ID: <20260723150316.168440-1-e.huhsovitz@proxmox.com> X-Mailer: git-send-email 2.47.3 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1784818968474 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.155 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_LOW -0.7 Sender listed at https://www.dnswl.org/, low trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: 24I63BE7ZQGPUGAPRA6P4NAPBPG3I5WE X-Message-ID-Hash: 24I63BE7ZQGPUGAPRA6P4NAPBPG3I5WE X-MailFrom: e.huhsovitz@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: Elias Huhsovitz X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Add a text field for custom mount options to the NFS and CIFS storage configuration dialogs. This allows users to configure advanced mount parameters (like 'nconnect', 'rdma', or 'seal') directly via the web interface, without needing to use the CLI or manually edit storage.cfg. When nfs version or mount option is being changed: Display a hint to the user that changes will require remounting the storage to take effect. Reduce the usage of legacy JS practices, such as 'var' & "+ string concatination" in affected functions. Signed-off-by: Elias Huhsovitz --- v1: https://lore.proxmox.com/pve-devel/20260721121334.115027-1-e.huhsovitz@proxmox.com/ This v2 patch addresses comments made in [1]. changes v1->v2: - add dynamic UI hint when user changes mount config or nfs version to inform them them that a remount is required for changes to take effect. - update emptyText placeholders to align with existing PVE UI conventions comments that have not been implemented: - reboot after passsing invalid option: Regarding the concern about correcting an invalid option (e.g., port=42069) and the storage failing to reconnect: I attempted to reproduce this behavior in my test environment but was unable to do so. In my testing, correcting the option and saving successfully updated the configuration, and the storage reconnected after a few seconds. If this auto-reconnection behavior is indeed a reproducible edge case in specific scenarios, I believe it would be best addressed in a separate, dedicated patch. - automatic re-mount: I made the deliberate decision not to implement automatic re-mounting upon configuration changes in this patch. Automatically triggering a remount could introduce unnecessary complexity and risk disrupting active I/O operations (reads/writes) if a user modifies options while the storage is currently in use by running guests or backup jobs. [1] https://lore.proxmox.com/pve-devel/DK4A5D7GDH1S.1WY2YTP8AF8MG@proxmox.com/ www/manager6/storage/CIFSEdit.js | 70 +++++++++++++++++-- www/manager6/storage/NFSEdit.js | 111 +++++++++++++++++++++++++------ 2 files changed, 154 insertions(+), 27 deletions(-) diff --git a/www/manager6/storage/CIFSEdit.js b/www/manager6/storage/CIFSEdit.js index 5fe3fefe..313bc7ff 100644 --- a/www/manager6/storage/CIFSEdit.js +++ b/www/manager6/storage/CIFSEdit.js @@ -121,7 +121,7 @@ Ext.define('PVE.storage.CIFSInputPanel', { onlineHelp: 'storage_cifs', onGetValues: function (values) { - let me = this; + const me = this; if (values.password?.length === 0) { delete values.password; @@ -133,11 +133,39 @@ Ext.define('PVE.storage.CIFSInputPanel', { delete values.subdir; } + if (values.options) { + values.options = values.options + .split(',') + .map((opt) => opt.trim()) + .filter((opt, index, self) => opt !== '' && self.indexOf(opt) === index) + .join(','); + } + + if (!values.options) { + delete values.options; + if (!me.isCreate) { + values.delete = values.delete ? `${values.delete},options` : 'options'; + } + } + return me.callParent([values]); }, + setValues: function (values) { + const me = this; + + me.originalOptions = values.options || ''; + + me.callParent([values]); + + const hint = me.down('#mountOptionsHint'); + if (hint) { + hint.setHidden(true); + } + }, + initComponent: function () { - var me = this; + const me = this; me.column1 = [ { @@ -149,7 +177,7 @@ Ext.define('PVE.storage.CIFSInputPanel', { listeners: { change: function (f, value) { if (me.isCreate) { - let exportField = me.down('field[name=share]'); + const exportField = me.down('field[name=share]'); exportField.setServer(value); } }, @@ -166,7 +194,7 @@ Ext.define('PVE.storage.CIFSInputPanel', { if (!me.isCreate) { return; } - var exportField = me.down('field[name=share]'); + const exportField = me.down('field[name=share]'); exportField.setUsername(value); }, }, @@ -181,7 +209,7 @@ Ext.define('PVE.storage.CIFSInputPanel', { minLength: 1, listeners: { change: function (f, value) { - let exportField = me.down('field[name=share]'); + const exportField = me.down('field[name=share]'); exportField.setPassword(value); }, }, @@ -213,7 +241,7 @@ Ext.define('PVE.storage.CIFSInputPanel', { listeners: { change: function (f, value) { if (me.isCreate) { - let exportField = me.down('field[name=share]'); + const exportField = me.down('field[name=share]'); exportField.setDomain(value); } }, @@ -229,6 +257,36 @@ Ext.define('PVE.storage.CIFSInputPanel', { }, ]; + me.advancedColumn2 = [ + { + xtype: 'textfield', + name: 'options', + fieldLabel: gettext('Mount Options'), + emptyText: `${gettext('Example')}: rdma, seal`, + allowBlank: true, + deleteEmpty: !me.isCreate, + listeners: { + change: function (field, newValue) { + const hint = me.down('#mountOptionsHint'); + if (hint) { + hint.setHidden((newValue || '') === me.originalOptions); + } + }, + }, + }, + ]; + + me.advancedColumnB = me.advancedColumnB || []; + me.advancedColumnB.unshift({ + xtype: 'displayfield', + itemId: 'mountOptionsHint', + userCls: 'pmx-hint', + hidden: true, + value: gettext( + 'Changes to mount options require remounting the storage to take effect.', + ), + }); + me.callParent(); }, }); diff --git a/www/manager6/storage/NFSEdit.js b/www/manager6/storage/NFSEdit.js index 72db156f..c4756fe8 100644 --- a/www/manager6/storage/NFSEdit.js +++ b/www/manager6/storage/NFSEdit.js @@ -57,7 +57,6 @@ Ext.define('PVE.storage.NFSScan', { me.callParent(); }, }); - Ext.define('PVE.storage.NFSInputPanel', { extend: 'PVE.panel.StorageBase', @@ -66,25 +65,31 @@ Ext.define('PVE.storage.NFSInputPanel', { options: [], onGetValues: function (values) { - var me = this; - - var i; - var res = []; - for (i = 0; i < me.options.length; i++) { - let item = me.options[i]; - if (!item.match(/^vers=(.*)$/)) { - res.push(item); - } + const me = this; + const res = []; + + if (values.options && values.options.trim() !== '') { + const opts = values.options.split(','); + opts.forEach((opt) => { + const cleanOpt = opt.trim(); + if (cleanOpt !== '' && !cleanOpt.match(/^vers=(.*)$/) && !res.includes(cleanOpt)) { + res.push(cleanOpt); + } + }); } + if (values.nfsversion && values.nfsversion !== '__default__') { - res.push('vers=' + values.nfsversion); + res.push(`vers=${values.nfsversion}`); } + delete values.nfsversion; - values.options = res.join(','); - if (values.options === '') { + + if (res.length > 0) { + values.options = res.join(','); + } else { delete values.options; if (!me.isCreate) { - values.delete = 'options'; + values.delete = values.delete ? `${values.delete},options` : 'options'; } } @@ -92,21 +97,42 @@ Ext.define('PVE.storage.NFSInputPanel', { }, setValues: function (values) { - var me = this; + const me = this; + if (values.options) { - me.options = values.options.split(','); - me.options.forEach(function (item) { - var match = item.match(/^vers=(.*)$/); + const opts = values.options.split(',').map((item) => item.trim()); + + opts.forEach((item) => { + const match = item.match(/^vers=(.*)$/); if (match) { values.nfsversion = match[1]; } }); + + // Filter out "vers=..." and empty strings + const otherOptions = opts.filter( + (item, index, self) => item !== '' && !item.match(/^vers=(.*)$/), + ); + values.options = otherOptions.join(','); + + if (values.options === '') { + delete values.options; + } + } + + me.originalOptions = values.options || ''; + me.originalNfsVersion = values.nfsversion || '__default__'; + + me.callParent([values]); + + const hint = me.down('#mountOptionsHint'); + if (hint) { + hint.setHidden(true); } - return me.callParent([values]); }, initComponent: function () { - var me = this; + const me = this; me.column1 = [ { @@ -118,7 +144,7 @@ Ext.define('PVE.storage.NFSInputPanel', { listeners: { change: function (f, value) { if (me.isCreate) { - let exportField = me.down('field[name=export]'); + const exportField = me.down('field[name=export]'); exportField.setServer(value); exportField.setValue(''); } @@ -143,6 +169,26 @@ Ext.define('PVE.storage.NFSInputPanel', { ]; me.advancedColumn2 = [ + { + xtype: 'textfield', + name: 'options', + fieldLabel: gettext('Mount Options'), + emptyText: `${gettext('Example')}: nconnect=4, soft`, + allowBlank: true, + deleteEmpty: !me.isCreate, + listeners: { + change: function (field, newValue) { + const hint = me.down('#mountOptionsHint'); + if (hint) { + const currentVersion = + me.down('field[name=nfsversion]')?.getValue() || '__default__'; + const isOptionsChanged = (newValue || '') !== me.originalOptions; + const isVersionChanged = currentVersion !== me.originalNfsVersion; + hint.setHidden(!(isOptionsChanged || isVersionChanged)); + } + }, + }, + }, { xtype: 'proxmoxKVComboBox', fieldLabel: gettext('NFS Version'), @@ -156,9 +202,32 @@ Ext.define('PVE.storage.NFSInputPanel', { ['4.1', '4.1'], ['4.2', '4.2'], ], + listeners: { + change: function (field, newValue) { + const hint = me.down('#mountOptionsHint'); + if (hint) { + const currentOptions = me.down('field[name=options]')?.getValue() || ''; + const isOptionsChanged = currentOptions !== me.originalOptions; + const isVersionChanged = + (newValue || '__default__') !== me.originalNfsVersion; + hint.setHidden(!(isOptionsChanged || isVersionChanged)); + } + }, + }, }, ]; + me.advancedColumnB = me.advancedColumnB || []; + me.advancedColumnB.unshift({ + xtype: 'displayfield', + itemId: 'mountOptionsHint', + userCls: 'pmx-hint', + hidden: true, + value: gettext( + 'Changes to mount options require remounting the storage to take effect.', + ), + }); + me.callParent(); }, }); -- 2.47.3