[pve-devel] [PATCH manager 1/2] fix #1451: allow some extra mount options for lxc

[Dominik Csapak]

comments inline

On 7/4/19 9:05 AM, Oguz Bektas wrote:
> this allows the following mount options for lxc container rootfs or
> mountpoints:
> * noatime
> * nosuid
> * noexec
> * nodev
> 
> Signed-off-by: Oguz Bektas <o.bektas at proxmox.com>
> ---
>   www/manager6/lxc/MPEdit.js | 28 ++++++++++++++++++++++++++++
>   1 file changed, 28 insertions(+)
> 
> diff --git a/www/manager6/lxc/MPEdit.js b/www/manager6/lxc/MPEdit.js
> index c7c3870a..1c84eb5b 100644
> --- a/www/manager6/lxc/MPEdit.js
> +++ b/www/manager6/lxc/MPEdit.js
> @@ -29,6 +29,9 @@ 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.join(';');
> +	}
>   
>   	if (me.unused) {
>   	    confid = "mp"+values.mpid;
> @@ -52,6 +55,16 @@ 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;

i guess this is a leftover, since you do not use comboitems anywhere?

> +	    var setoptions = [];
> +	    var i = 0;
> +	    for (i; i < mp.mountoptions.length; i++) {
> +		setoptions.push(mp.mountoptions[i]);
> +	    }
> +	    mp.mountoptions = setoptions;

why are you doing this? you get an array after splitting..
then you copy the whole content of that array into another array
and then you assign the new array on the old array.. with the same content?

my guess is that this is not needed at all, simply:

if (mp.mountoptions) {
	mp.mountoptions = mp.mountoptions.split(';');
}

> +	}
>   	me.setValues(mp);
>       },
>   
> @@ -275,6 +288,21 @@ Ext.define('PVE.lxc.MountPointInputPanel', {
>   	    allowBlank: true
>   	},
>   	{
> +	    xtype: 'proxmoxKVComboBox',
> +	    name: 'mountoptions',
> +	    fieldLabel: 'Mount options',

gettext would be useful here

> +	    deleteEmpty: false,
> +	    comboItems: [
> +		['noatime', 'noatime'],
> +		['nodev', 'nodev'],
> +		['noexec', 'noexec'],
> +		['nosuid', 'nosuid']
> +	    ],
> +	    multiSelect: true,
> +	    value: [],
> +	    allowBlank: true
> +	},
> +	{
>   	    xtype: 'proxmoxcheckbox',
>   	    inputValue: '0', // reverses the logic
>   	    name: 'replicate',
>