[pve-devel] [PATCH manager] Introduce check to disable quota usage for unprivileged containers

[Thomas Lamprecht]

some comments inline.

On 08/23/2017 04:31 PM, Philip Abernethy wrote:
> Disables the quota checkbox for unprivileged containers in the creation wizard,
> as well as when editing or adding mountpoints.
> ---
>   www/manager6/lxc/CreateWizard.js | 20 +++++++++++++++-----
>   www/manager6/lxc/ResourceEdit.js | 10 ++++++++--
>   www/manager6/lxc/Resources.js    | 27 +++++++++++++++++++++------
>   3 files changed, 44 insertions(+), 13 deletions(-)
> 
> diff --git a/www/manager6/lxc/CreateWizard.js b/www/manager6/lxc/CreateWizard.js
> index c2f16a1a..a8f6ef9f 100644
> --- a/www/manager6/lxc/CreateWizard.js
> +++ b/www/manager6/lxc/CreateWizard.js
> @@ -60,7 +60,8 @@ Ext.define('PVE.lxc.CreateWizard', {
>   	    insideWizard: true,
>   	    isCreate: true,
>   	    unused: false,
> -	    confid: 'rootfs'
> +	    confid: 'rootfs',
> +	    unprivileged: false

nitpick: I know JS standard is weird for not allowing trailing commas,
but you can always add it in between to avoid touching multiple lines.

>   	});
>   
>   	var networkpanel = Ext.create('PVE.lxc.NetworkInputPanel', {
> @@ -148,7 +149,7 @@ Ext.define('PVE.lxc.CreateWizard', {
>   		validator: function(value) {
>   		    var pw = me.down('field[name=password]').getValue();
>   		    if (pw !== value) {
> -			return "Passwords does not match!";
> +			return "Passwords do not match!";
>   		    }
>   		    return true;
>   		}
> @@ -218,7 +219,16 @@ Ext.define('PVE.lxc.CreateWizard', {
>   			    xtype: 'pvecheckbox',
>   			    name: 'unprivileged',
>   			    value: '',
> -			    fieldLabel: gettext('Unprivileged container')
> +			    fieldLabel: gettext('Unprivileged container'),
> +			    listeners: {
> +				change: function(f, value) {
> +				    if (value) {
> +					rootfspanel.down('field[name=quota]').setValue(false);
> +				    }
> +				    rootfspanel.unprivileged = value;
> +				    rootfspanel.down('field[name=quota]').setDisabled(value);
> +				}
> +			    }
>   			}
>   		    ],
>   		    column2: column2,
> @@ -309,8 +319,8 @@ Ext.define('PVE.lxc.CreateWizard', {
>   			    params: kv,
>   			    success: function(response, opts){
>   				var upid = response.result.data;
> -		
> -				var win = Ext.create('PVE.window.TaskViewer', {
> +
> +				var win = Ext.create('PVE.window.TaskViewer', {
>   				    upid: upid
>   				});
>   				win.show();
> diff --git a/www/manager6/lxc/ResourceEdit.js b/www/manager6/lxc/ResourceEdit.js
> index 9efb5116..893e10e1 100644
> --- a/www/manager6/lxc/ResourceEdit.js
> +++ b/www/manager6/lxc/ResourceEdit.js
> @@ -39,6 +39,8 @@ Ext.define('PVE.lxc.CPUEdit', {
>   Ext.define('PVE.lxc.MountPointEdit', {
>       extend: 'PVE.window.Edit',
>   
> +    unprivileged: false,
> +
>       initComponent : function() {
>   	var me = this;
>   
> @@ -55,6 +57,7 @@ Ext.define('PVE.lxc.MountPointEdit', {
>   	    confid: me.confid,
>   	    nodename: nodename,
>   	    unused: unused,
> +	    unprivileged: me.unprivileged,
>   	    isCreate: me.isCreate
>   	});
>   
> @@ -221,6 +224,8 @@ Ext.define('PVE.lxc.MountPointInputPanel', {
>   
>       unused: false, // ADD usused disk imaged
>   
> +    unprivileged: false,
> +
>       vmconfig: {}, // used to select usused disks
>   
>       onGetValues: function(values) {
> @@ -405,7 +410,7 @@ Ext.define('PVE.lxc.MountPointInputPanel', {
>   		        rec.data.type === 'zfspool')) {
>   			me.quota.setDisabled(true);
>   			me.quota.setValue(false);
> -		    } else {
> +		    } else if (!me.unprivileged) {
>   			me.quota.setDisabled(false);
>   		    }
>   		    if (me.unused || !me.isCreate) {
> @@ -502,7 +507,8 @@ Ext.define('PVE.lxc.MountPointInputPanel', {
>   	me.quota = Ext.createWidget('pvecheckbox', {
>   	    name: 'quota',
>   	    defaultValue: 0,
> -	    fieldLabel: gettext('Enable quota')
> +	    fieldLabel: gettext('Enable quota'),
same

> +	    disabled: me.unprivileged
>   	});
>   
>   	me.column2 = [
> diff --git a/www/manager6/lxc/Resources.js b/www/manager6/lxc/Resources.js
> index c54c7001..23570a9c 100644
> --- a/www/manager6/lxc/Resources.js
> +++ b/www/manager6/lxc/Resources.js
> @@ -94,6 +94,9 @@ Ext.define('PVE.lxc.RessourceView', {
>   		defaultValue: PVE.Utils.noneText,
>   		editor: mpeditor,
>   		tdCls: 'pve-itype-icon-storage'
> +	    },
> +	    unprivileged: {
> +		visible: false
>   	    }
>   	};
>   
> @@ -138,11 +141,22 @@ Ext.define('PVE.lxc.RessourceView', {
>   
>   	    var editor = rowdef.editor;
>   
> -	    var win = Ext.create(editor, {
> -		pveSelNode: me.pveSelNode,
> -		confid: rec.data.key,
> -		url: '/api2/extjs/' + baseurl
> -	    });
> +	    var win;
> +	    // Only containers can be unprivileged, handle them separately

we are already in lxc/Resources so this is all for CTs? confusing comment...

Why not just add
unprivileged: me.getObjectValue('unprivileged')
or
unprivileged: !!me.getObjectValue('unprivileged') // (if booleanes is a problem)

to the Ext.create editor config above, omitting the if completely?


> +	    if (me.getObjectValue('unprivileged')) {
> +		win = Ext.create(editor, {
> +		    pveSelNode: me.pveSelNode,
> +		    confid: rec.data.key,
> +		    unprivileged: true,
> +		    url: '/api2/extjs/' + baseurl
> +		});
> +	    } else {
> +		win = Ext.create(editor, {
> +		    pveSelNode: me.pveSelNode,
> +		    confid: rec.data.key,
> +		    url: '/api2/extjs/' + baseurl
> +		});
> +	    }
>   
>   	    win.show();
>   	    win.on('destroy', reload);
> @@ -266,7 +280,8 @@ Ext.define('PVE.lxc.RessourceView', {
>   				handler: function() {
>   				    var win = Ext.create('PVE.lxc.MountPointEdit', {
>   					url: '/api2/extjs/' + baseurl,
> -					pveSelNode: me.pveSelNode
> +					pveSelNode: me.pveSelNode,
> +					unprivileged: me.getObjectValue('unprivileged')
>   				    });
>   				    win.on('destroy', reload);
>   				    win.show();
>