[pve-devel] [PATCH v2 manager 2/2] fix #1451: add mount options to containers in gui

[Thomas Lamprecht]

On 5/21/19 10:48 AM, Oguz Bektas wrote:
> adds atime related options (noatime, nodiratime, relatime,
> strictatime) in the gui for mountpoints/rootfs in cts.
> 
> Signed-off-by: Oguz Bektas <o.bektas at proxmox.com>
> ---
>  www/manager6/lxc/MPEdit.js | 68 ++++++++++++++++++++++++++++++++++++----------
>  1 file changed, 54 insertions(+), 14 deletions(-)
> 
> diff --git a/www/manager6/lxc/MPEdit.js b/www/manager6/lxc/MPEdit.js
> index c7c3870a..db58a6b5 100644
> --- a/www/manager6/lxc/MPEdit.js
> +++ b/www/manager6/lxc/MPEdit.js
> @@ -29,6 +29,11 @@ Ext.define('PVE.lxc.MountPointInputPanel', {
>  
>  	var confid = me.confid || "mp"+values.mpid;
>  	values.file = me.down('field[name=file]').getValue();
> +	if (values.mountoptions) {
> +	    values.mountoptions = values.mountoptions.concat(me.savedoptions).join(';');
> +	} else {
> +	    values.mountoptions = me.savedoptions.join(';');
> +	}
>  
>  	if (me.unused) {
>  	    confid = "mp"+values.mpid;
> @@ -52,6 +57,26 @@ Ext.define('PVE.lxc.MountPointInputPanel', {
>  	var me = this;
>  	var vm = this.getViewModel();
>  	vm.set('mptype', mp.type);
> +	if (mp.mountoptions) {
> +	    mp.mountoptions = mp.mountoptions.split(';');
> +	    var comboitems = me.down('field[name=mountoptions]').comboItems;
> +	    var allowed = {};
> +	    var savedoptions = [];
> +	    var setoptions = [];

sorry, but what are you doing here, what's saved vs set options, if you join
them above anyway?

> +	    comboitems.forEach(function(item) {
> +		allowed[item[0]] = true;
> +	    });
> +	    var i = 0;
> +	    for (i; i < mp.mountoptions.length; i++) {
> +		if (allowed[mp.mountoptions[i]] === true) {
> +		    setoptions.push(mp.mountoptions[i]);
> +		} else {
> +		    savedoptions.push(mp.mountoptions[i]);
> +		}
> +	    }
> +	    me.savedoptions = savedoptions;
> +	    mp.mountoptions = setoptions;
> +	}
>  	me.setValues(mp);
>      },
>  
> @@ -233,20 +258,6 @@ Ext.define('PVE.lxc.MountPointInputPanel', {
>      advancedColumn1: [
>  	{
>  	    xtype: 'proxmoxcheckbox',
> -	    name: 'quota',
> -	    defaultValue: 0,
> -	    bind: {
> -		disabled: '{!quota}'
> -	    },
> -	    fieldLabel: gettext('Enable quota'),
> -	    listeners: {
> -		disable: function() {
> -		    this.reset();
> -		}
> -	    }
> -	},

why you move this? Please note such movements with a short sentence
about the reason in the commit message..

> -	{
> -	    xtype: 'proxmoxcheckbox',
>  	    name: 'ro',
>  	    defaultValue: 0,
>  	    bind: {
> @@ -275,6 +286,35 @@ Ext.define('PVE.lxc.MountPointInputPanel', {
>  	    allowBlank: true
>  	},
>  	{
> +	    xtype: 'proxmoxKVComboBox',
> +	    name: 'mountoptions',
> +	    fieldLabel: 'Mount options',
> +	    deleteEmpty: false,

hmm, don't we want to delete the "mountoptions" field completely if this is empty?

> +	    comboItems: [
> +		['noatime', 'noatime'],
> +		['nodiratime', 'nodiratime'],
> +		['relatime', 'relatime'],
> +		['strictatime', 'strictatime']

so I can set multiple ones of those at the same time?
Also, do all FS types we support have all of those available? I see that VFS
implements that, works it for ZFS too?

Maybe just add a non-multiselect "Access Time" options field for this?

That's the reasonable part of the #1451 request, because the nobarrier one there
I really want to have here.

> +	    ],
> +	    multiSelect: true,
> +	    value: [],
> +	    allowBlank: true
> +	},
> +	{
> +	    xtype: 'proxmoxcheckbox',
> +	    name: 'quota',
> +	    defaultValue: 0,
> +	    bind: {
> +		disabled: '{!quota}'
> +	    },
> +	    fieldLabel: gettext('Enable quota'),
> +	    listeners: {
> +		disable: function() {
> +		    this.reset();
> +		}
> +	    }
> +	},
> +	{
>  	    xtype: 'proxmoxcheckbox',
>  	    inputValue: '0', // reverses the logic
>  	    name: 'replicate',
>