[pve-devel] [PATCH v2 manager] fix #2185: add option to change nfs version on gui

[Dominik Csapak]

On 5/13/19 8:23 AM, Thomas Lamprecht wrote:
> Am 5/10/19 um 2:52 PM schrieb Oguz Bektas:
>> this enables us to specify an nfs version while editing/creating an nfs
>> mount. it used to default to vers=3 without the ability to change it in
>> gui. now it supports: 3, 4, 4.1 and 4.2
>>
>> it should also be possible to add further options in the future (rsize,
>> wsize, timeo, etc.) on this screen.
>>
>> Signed-off-by: Oguz Bektas <o.bektas at proxmox.com>
>> ---
>>
>> v1->v2:
>> * add missing space after for loop
> 
> may make sense to await a first review before resending for such trivial things
> 
> But looks OK, in general.
> 
>>
>>
>>
>>   www/manager6/storage/NFSEdit.js | 43 +++++++++++++++++++++++++++++++++++++----
>>   1 file changed, 39 insertions(+), 4 deletions(-)
>>
>> diff --git a/www/manager6/storage/NFSEdit.js b/www/manager6/storage/NFSEdit.js
>> index 9eaa8bc5..26a158a8 100644
>> --- a/www/manager6/storage/NFSEdit.js
>> +++ b/www/manager6/storage/NFSEdit.js
>> @@ -65,15 +65,35 @@ Ext.define('PVE.storage.NFSInputPanel', {
>>       onGetValues: function(values) {
>>   	var me = this;
>>   
>> -	if (me.isCreate) {
>> -	    // hack: for now we always create nvf v3
>> -	    // fixme: make this configurable
>> -	    values.options = 'vers=3';
>> +	var i;
>> +	for (i = 0; i < me.options.length; i++) {
>> +	    var match;
>> +	    var item = me.options[i];
>> +	    if (match = item.match(/^vers=(.*)$/)) {
> 
> match is set but never used here
> (I can remove this when applying, so no need to send v3 just for this)
> 
>> +		me.options[i] = 'vers=' + values.nfsversion;
>> +		delete values.nfsversion;
>> +	    }

here one case is missing, namely if me.options is not set/has no 'vers' 
entry, we have to add the version in all cases

>>   	}
>> +	values.options = me.options.join(',');
>>   
>>   	return me.callParent([values]);
>>       },
>>   
>> +    setValues: function(values) {
>> +	var me = this;
>> +	if (values.options) {
>> +	    var res = values.options;
>> +	    me.options = values.options.split(',');
>> +	    me.options.forEach(function(item) {
>> +		var match;
>> +		if (match = item.match(/^vers=(.*)$/)) {
>> +		    values.nfsversion = match[1];
>> +		}
>> +	    });
>> +	}
>> +	return me.callParent([values]);
>> +    },
>> +
>>       initComponent : function() {
>>   	var me = this;
>>   
>> @@ -126,6 +146,21 @@ Ext.define('PVE.storage.NFSInputPanel', {
>>   	    }
>>   	];
>>   
>> +	me.advancedColumn1 = [
>> +	    {
>> +		xtype: 'proxmoxKVComboBox',
>> +		fieldLabel: gettext('NFS Version'),
>> +		name: 'nfsversion',
>> +		value: '3',
>> +		comboItems: [
> 
> do we want a __default__ in here?

mhmm, we could add one that amounts to no version option at all
would it also make nicer if no version is in the options

['__default__',Proxmox.Utils.defaultText ],

?

we have to handle that of course while setting/getting
and maybe we want to keep the gui default of '3' ?
although it would be weird to have a 'default' setting
that is not the default?

> 
>> +			['3', '3'],
>> +			['4', '4'],
>> +			['4.1', '4.1'],
>> +			['4.2', '4.2']
>> +		],
>> +	    }
>> +	];
>> +
>>   	me.callParent();
>>       }
>>   });
>>
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>