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

[Oguz Bektas]

hi,

On Thu, Jul 04, 2019 at 10:11:54AM +0200, Dominik Csapak wrote:
> 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?
yeah sorry i'll remove it
> 
> > +	    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?

also leftover from the previous patches
> 
> 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
ok!
> 
> > +	    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',
> > 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel