[pve-devel] [PATCH manager 05/17] get storage info from /version api call in gui

[Emmanuel Kasper]

On 07/19/2017 03:45 PM, Dominik Csapak wrote:
> we parse and save the storage type infos in PVE.Utils.storageformats
> and can get them with PVE.Utils.getStorageFormat(type)
> so that we do not have to hardcode the values

Thank you for bringing this forward, especially the refactoring in the
new DiskStorageSelector. Some nitpicks inline.

> the returned object contains:
>  * an array formats, which contains the file/volume formats, e.g. raw,
>    qcow2, vmdk
>  * a defaultFormat
>  * the possible contents as array
>  * a 'select' property which indicates, if we have to
>    select an image instead of creating one (e.g. for iscsi)

since it looks you are going to rewrite the API call, maybe we could use
a more exact name here for the 'select' property like useDeviceDirectly
or useBlockDev ?


> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
> ---
>  www/manager6/Utils.js     | 55 +++++++++++++++++++++++++++++++++++++++++++++++
>  www/manager6/Workspace.js |  4 ++++
>  2 files changed, 59 insertions(+)
> 
> diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
> index 78683e0c..0cc155f4 100644
> --- a/www/manager6/Utils.js
> +++ b/www/manager6/Utils.js
> @@ -1371,6 +1371,59 @@ Ext.define('PVE.Utils', { utilities: {
>  	}
>      },

PVE.Utils has already a long life, so maybe it would make sense,
to create a new Singleton for holding these methods.

> +    setStorageFormats: function(types) {
I would rather name this method setStorageProperties, to avoid
setting format.formats = [];

> +	me.storageformats = {};
nitpick: you do this in the contructor below so this is not needed

> +	for (type in types) {
> +	    if (types.hasOwnProperty(type)) {
> +		var st = types[type];
> +		var format = {
		so this could be var properties = {
> +		    content: st.contents,
> +		    formats: ['raw'],
> +		    defaultFormat: 'raw',
> +		    selecting: st.select === 1
> +		};
> +
> +		if (st.format !== undefined) {
> +		    // if we get formats, we want to have them
> +		    format.formats = [];
> +		    for (sf in st.format[0]) {
> +			if (st.format[0].hasOwnProperty(sf) &&
> +			    sf !== 'subvol') { // we never need this format
> +			    format.formats.push(sf);
> +			}
> +		    }
> +		    format.defaultFormat = st.format[1];
> +		}
> +
> +		me.storageformats[type] = format;
> +	    }
> +	}
> +	/*jslint confusion: false*/
> +    },
> +
> +    getStorageFormat: function(type) {
> +	/*jslint confusion: true*/
> +	// formats/select are functions elsewhere
> +	var me = this;
> +	if (me.storageformats[type] !== undefined) {
> +	    return me.storageformats[type];
> +	} else {
> +	    return {
> +		formats: {
> +		    'qcow2':1,
> +		    'raw':1,
> +		    'vmdk':1
> +		},
> +		defaultFormat: 'raw',
> +		select: 0
> +	    };
> +	}
> +    },
> +
>      singleton: true,
>      constructor: function() {
>  	var me = this;
> @@ -1409,6 +1462,8 @@ Ext.define('PVE.Utils', { utilities: {
>  	me.HostPort_match = new RegExp("^(" + IPV4_REGEXP + "|" + DnsName_REGEXP + ")(:\\d+)?$");
>  	me.HostPortBrackets_match = new RegExp("^\\[(?:" + IPV6_REGEXP + "|" + IPV4_REGEXP + "|" + DnsName_REGEXP + ")\\](:\\d+)?$");
>  	me.IP6_dotnotation_match = new RegExp("^" + IPV6_REGEXP + "(\\.\\d+)?$");
> +
> +	me.storageformats = {};
>      }
>  });
>  
> diff --git a/www/manager6/Workspace.js b/www/manager6/Workspace.js
> index fdade908..1f64b3ff 100644
> --- a/www/manager6/Workspace.js
> +++ b/www/manager6/Workspace.js
> @@ -155,6 +155,10 @@ Ext.define('PVE.StdWorkspace', {
>  		    if (data.cap) {
>  			Ext.state.Manager.set('GuiCap', data.cap);
>  		    }
> +
> +		    if (data.storagetypes) {
> +			PVE.Utils.setStorageFormats(data.storagetypes);
> +		    }

How often do we need to define new storage properties ? Maybe this would
be enough to load this once per HTML page Load ? In that case we could
move the initial dataload into the constructor of the singleton I
suggested above.