[pve-devel] [PATCH V2 manager] Add cifs storage plugin

Dominik Csapak d.csapak at proxmox.com
Fri Mar 16 11:10:54 CET 2018


Some comments inline, rest looks good

On 03/16/2018 10:22 AM, Wolfgang Link wrote:
> ---
>   www/manager6/Makefile            |   1 +
>   www/manager6/Utils.js            |   2 +
>   www/manager6/dc/StorageView.js   |  11 ++
>   www/manager6/storage/CIFSEdit.js | 285 +++++++++++++++++++++++++++++++++++++++
>   4 files changed, 299 insertions(+)
>   create mode 100644 www/manager6/storage/CIFSEdit.js
> 
> diff --git a/www/manager6/Makefile b/www/manager6/Makefile
> index e0c46557..b83adfb0 100644
> --- a/www/manager6/Makefile
> +++ b/www/manager6/Makefile
> @@ -150,6 +150,7 @@ JSSRC= 				                 	\
>   	storage/Browser.js				\
>   	storage/DirEdit.js				\
>   	storage/NFSEdit.js				\
> +	storage/CIFSEdit.js				\
>   	storage/GlusterFsEdit.js			\
>   	storage/IScsiEdit.js				\
>   	storage/LVMEdit.js				\
> diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
> index 87699e56..330822d7 100644
> --- a/www/manager6/Utils.js
> +++ b/www/manager6/Utils.js
> @@ -391,6 +391,8 @@ Ext.define('PVE.Utils', { utilities: {
>   	    return Proxmox.Utils.directoryText;
>   	} else if (value === 'nfs') {
>   	    return 'NFS';
> +	} else if (value === 'cifs') {
> +	    return 'CIFS';
>   	} else if (value === 'glusterfs') {
>   	    return 'GlusterFS';
>   	} else if (value === 'lvm') {
> diff --git a/www/manager6/dc/StorageView.js b/www/manager6/dc/StorageView.js
> index f09733a5..ba6faafb 100644
> --- a/www/manager6/dc/StorageView.js
> +++ b/www/manager6/dc/StorageView.js
> @@ -42,6 +42,8 @@ Ext.define('PVE.dc.StorageView', {
>   		editor = 'PVE.storage.DirEdit';
>   	    } else if (type === 'nfs') {
>   		editor = 'PVE.storage.NFSEdit';
> +	    } else if (type === 'cifs') {
> +		editor = 'PVE.storage.CIFSEdit';
>   	    } else if (type === 'glusterfs') {
>   		editor = 'PVE.storage.GlusterFsEdit';
>   	    } else if (type === 'lvm') {
> @@ -134,6 +136,15 @@ Ext.define('PVE.dc.StorageView', {
>   				}
>   			    },
>   			    {
> +				text:  PVE.Utils.format_storage_type('cifs'),
> +				iconCls: 'fa fa-fw fa-building',
> +				handler: function() {
> +				    var win = Ext.create('PVE.storage.CIFSEdit', {});
> +				    win.on('destroy', reload);
> +				    win.show();
> +				}
> +			    },
> +			    {
>   				text: PVE.Utils.format_storage_type('iscsi'),
>   				iconCls: 'fa fa-fw fa-building',
>   				handler: function() {
> diff --git a/www/manager6/storage/CIFSEdit.js b/www/manager6/storage/CIFSEdit.js
> new file mode 100644
> index 00000000..61e99380
> --- /dev/null
> +++ b/www/manager6/storage/CIFSEdit.js
> @@ -0,0 +1,285 @@
> +Ext.define('PVE.storage.CIFSScan', {
> +    extend: 'Ext.form.field.ComboBox',
> +    alias: 'widget.pveCIFSScan',
> +
> +    queryParam: 'server',
> +
> +    valueField: 'share',
> +    displayField: 'share',
> +    matchFieldWidth: false,
> +    listConfig: {
> +	loadingText: gettext('Scanning...'),
> +	width: 350
> +    },
> +    doRawQuery: function() {
> +    },
> +
> +    onTriggerClick: function() {
> +	var me = this;
> +
> +	if (!me.queryCaching || me.lastQuery !== me.cifsServer) {
> +	    me.store.removeAll();
> +	}
> +
> +	delete me.store.getProxy().setExtraParams({});

the delete is not necessary (also, it does not do anything here)
(could have been detected with jslint)

> +	if (me.cifsUsername && me.cifsPassword) {

i have not looked at the api patches yet, but would it not be good
to allow username only also (e.g. 'guest')

> +	    me.store.getProxy().setExtraParam('username', me.cifsUsername);
> +	    me.store.getProxy().setExtraParam('password', me.cifsPassword);
> +	}
> +
> +	if (me.cifsDomain) {
> +	    me.store.getProxy().setExtraParam('domain', me.cifsDomain);
> +	}

i think it would be better to build an object with the params, and set 
it only once (instead of up to 4 times)

like this:

var params = {};
if (...) {
	params.something = somethingelse;
}

and at the end:

....setExtraParams(params);

this way we minimize the extjs code we call

> +
> +	me.allQuery = me.cifsServer;
> +
> +	me.callParent();
> +    },
> +
> +    setServer: function(server) {
> +	var me = this;
> +
> +	me.cifsServer = server;
> +    },
> +
> +    setUsername: function(username) {
> +	var me = this;
> +
> +	me.cifsUsername = username;
> +    },
> +
> +    setPassword: function(password) {
> +	var me = this;
> +
> +	me.cifsPassword = password;
> +    },
> +
> +    setDomain: function(domain) {
> +	var me = this;
> +
> +	me.cifsDomain = domain;
> +    },
> +
> +    initComponent : function() {
> +	var me = this;
> +
> +	if (!me.nodename) {
> +	    me.nodename = 'localhost';
> +	}
> +
> +	var store = Ext.create('Ext.data.Store', {
> +	    fields: ['description', 'share'],
> +	    proxy: {
> +		type: 'proxmox',
> +		url: '/api2/json/nodes/' + me.nodename + '/scan/cifs'
> +	    }
> +	});
> +	store.sort('share', 'ASC');
> +
> +	Ext.apply(me, {
> +	    store: store
> +	});
> +
> +	me.callParent();
> +    }
> +});
> +
> +Ext.define('PVE.storage.CIFSInputPanel', {
> +    extend: 'Proxmox.panel.InputPanel',
> +    controller: 'storageEdit',
> +
> +    onGetValues: function(values) {
> +	var me = this;
> +
> +	if (me.isCreate) {
> +	    values.type = 'cifs';
> +	} else {
> +	    delete values.storage;
> +	}
> +
> +	values.disable = values.enable ? 0 : 1;
> +	delete values.enable;
> +
> +	return values;
> +    },
> +
> +    initComponent : function() {
> +	var me = this;
> +
> +	var passwordfield = Ext.createWidget(me.isCreate ? 'textfield' : 'displayfield', {
> +	    inputType: 'password',
> +	    name: 'password',
> +	    value: me.isCreate ? '' : '********',
> +	    fieldLabel: gettext('Password'),
> +	    allowBlank: true,
> +	    minLength: 1,
> +	    listeners: {
> +		change: function(f, value) {
> +
> +		    if (me.isCreate) {
> +			var exportField = me.down('field[name=share]');
> +			exportField.setPassword(value);
> +		    }
> +		}
> +	    },

jslint does not like trailing commas, best to run 'make lint' in the 
manager6 directory before commiting

> +	});
> +
> +	me.column1 = [
> +	    {
> +		xtype: me.isCreate ? 'textfield' : 'displayfield',
> +		name: 'storage',
> +		value: me.storageId || '',
> +		fieldLabel: 'ID',
> +		vtype: 'StorageId',
> +		allowBlank: false
> +	    },
> +	    {
> +		xtype: me.isCreate ? 'textfield' : 'displayfield',
> +		name: 'server',
> +		value: '',
> +		fieldLabel: gettext('Server'),
> +		allowBlank: false,
> +		listeners: {
> +		    change: function(f, value) {
> +			if (me.isCreate) {
> +			    var exportField = me.down('field[name=share]');
> +			    exportField.setServer(value);
> +			}
> +		    }
> +		}
> +	    },
> +	    {
> +		xtype: me.isCreate ? 'textfield' : 'displayfield',
> +		name: 'username',
> +		value: '',
> +		fieldLabel: gettext('Username'),
> +		allowBlank: true,
> +		listeners: {
> +		    change: function(f, value) {
> +			if (me.isCreate) {
> +			    var exportField = me.down('field[name=share]');
> +			    exportField.setUsername(value);
> +			}
> +			if (value == "") {
> +			    passwordfield.allowBlank = true;
> +			} else {
> +			    passwordfield.allowBlank = false;
> +			}
> +			passwordfield.validate();
> +		    }
> +		},

trailing comma

> +	    },
> +	    {
> +		xtype: me.isCreate ? 'pveCIFSScan' : 'displayfield',
> +		name: 'share',
> +		value: '',
> +		fieldLabel: 'Share',
> +		allowBlank: false,
> +	    },
> +	    {
> +		xtype: 'pveContentTypeSelector',
> +		name: 'content',
> +		value: 'images',
> +		multiSelect: true,
> +		fieldLabel: gettext('Content'),
> +		allowBlank: false
> +	    },

trailing comma

> +	];
> +
> +	me.column2 = [
> +	    {
> +		xtype: 'pveNodeSelector',
> +		name: 'nodes',
> +		fieldLabel: gettext('Nodes'),
> +		emptyText: gettext('All') + ' (' +
> +		    gettext('No restrictions') +')',
> +		multiSelect: true,
> +		autoSelect: false,
> +	    },
> +	    {
> +		xtype: 'proxmoxcheckbox',
> +		name: 'enable',
> +		checked: true,
> +		uncheckedValue: 0,
> +		fieldLabel: gettext('Enable')
> +	    },
> +	    {
> +		xtype: 'proxmoxintegerfield',
> +		fieldLabel: gettext('Max Backups'),
> +		name: 'maxfiles',
> +		reference: 'maxfiles',
> +		minValue: 0,
> +		maxValue: 365,
> +		value: me.isCreate ? '1' : undefined,
> +		allowBlank: false
> +	    },
> +	    passwordfield,
> +	    {
> +		xtype: me.isCreate ? 'textfield' : 'displayfield',
> +		name: 'domain',
> +		value: me.isCreate ? '' : undefined,
> +		fieldLabel: gettext('Domain'),
> +		allowBlank: true,
> +		listeners: {
> +		    change: function(f, value) {
> +			if (me.isCreate) {
> +
> +			    var exportField = me.down('field[name=share]');
> +			    exportField.setDomain(value);
> +			}
> +		    }
> +		},
> +	    },

trailing commas

> +	];
> +
> +	me.callParent();
> +    }
> +});
> +
> +Ext.define('PVE.storage.CIFSEdit', {
> +    extend: 'Proxmox.window.Edit',
> +
> +    initComponent : function() {
> +	var me = this;
> +
> +	me.isCreate = !me.storageId;
> +
> +	if (me.isCreate) {
> +            me.url = '/api2/extjs/storage';
> +            me.method = 'POST';
> +        } else {
> +            me.url = '/api2/extjs/storage/' + me.storageId;
> +            me.method = 'PUT';
> +        }
> +
> +	var ipanel = Ext.create('PVE.storage.CIFSInputPanel', {
> +	    isCreate: me.isCreate,
> +	    storageId: me.storageId
> +	});
> +
> +	Ext.apply(me, {
> +            subject: 'CIFS',
> +	    isAdd: true,
> +	    items: [ ipanel ]
> +	});
> +
> +	me.callParent();
> +
> +	if (!me.isCreate) {
> +	    me.load({
> +		success:  function(response, options) {
> +		    var values = response.result.data;
> +		    var ctypes = values.content || '';
> +
> +		    values.content = ctypes.split(',');
> +
> +		    if (values.nodes) {
> +			values.nodes = values.nodes.split(',');
> +		    }
> +		    values.enable = values.disable ? 0 : 1;
> +		    ipanel.setValues(values);
> +		}
> +	    });
> +	}
> +    }
> +});
> 





More information about the pve-devel mailing list