Re: [pve-devel] [PATCH pve-manager 2/3] fix #3967: enable ZFS dRAID creation in WebGUI

[Dominik Csapak <d.csapak@proxmox.com>]

comments inline

On 6/2/22 13:22, Stefan Hrdlicka wrote:
> add fields for additional settings required by ZFS dRAID
> 
> Signed-off-by: Stefan Hrdlicka <s.hrdlicka@proxmox.com>
> ---
> requires the changes in pve-storageto work
> 
>   www/manager6/node/ZFS.js | 47 ++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 47 insertions(+)
> 
> diff --git a/www/manager6/node/ZFS.js b/www/manager6/node/ZFS.js
> index 5b3bdbda..5f3bbfef 100644
> --- a/www/manager6/node/ZFS.js
> +++ b/www/manager6/node/ZFS.js
> @@ -42,6 +42,9 @@ Ext.define('PVE.node.CreateZFS', {
>   			    fieldLabel: gettext('RAID Level'),
>   			    name: 'raidlevel',
>   			    value: 'single',
> +			    listeners: {
> +				change: 'onTargetChange',
> +			    },
>   			    comboItems: [
>   				['single', gettext('Single Disk')],
>   				['mirror', 'Mirror'],
> @@ -49,8 +52,29 @@ Ext.define('PVE.node.CreateZFS', {
>   				['raidz', 'RAIDZ'],
>   				['raidz2', 'RAIDZ2'],
>   				['raidz3', 'RAIDZ3'],
> +				['draid', 'dRAID'],
> +				['draid2', 'dRAID2'],
> +				['draid3', 'dRAID3'],
>   			    ],
>   			},
> +			{
> +			    xtype: 'proxmoxintegerfield',
> +			    fieldLabel: gettext('DRAID data devices'),
> +			    minValue: 1,
> +			    disabled: true,
> +			    hidden: true,
> +			    reference: 'draiddata',
> +			    name: 'draiddata',
> +			},
> +			{
> +			    xtype: 'proxmoxintegerfield',
> +			    fieldLabel: gettext('DRAID spares'),
> +			    minValue: 0,
> +			    disabled: true,
> +			    hidden: true,
> +			    reference: 'draidspares',
> +			    name: 'draidspares',
> +			},

is that something someone always wants to configure?
or more an advanced thing? in the latter case, i'd
probably put them in the advanced options

>   			{
>   			    xtype: 'proxmoxKVComboBox',
>   			    fieldLabel: gettext('Compression'),
> @@ -101,6 +125,29 @@ Ext.define('PVE.node.CreateZFS', {
>   
>           me.callParent();
>       },
> +
> +    controller: {

nit: normally we put the controller  on top of the class, not at the
bottom

> +	xclass: 'Ext.app.ViewController',
> +
> +	onTargetChange: function(selection) {
> +	    var me = this;
> +	    var dataField = me.lookupReference('draiddata');
> +	    var sparesField = me.lookupReference('draidspares');
> +	    if (selection.value.startsWith("draid")) {
> +		//show draid settings
> +		dataField.setVisible(true);
> +		dataField.setDisabled(false);
> +		sparesField.setVisible(true);
> +		sparesField.setDisabled(false);
> +	    } else {
> +		//hide draid settings
> +		dataField.setVisible(false);
> +		dataField.setDisabled(true);
> +		sparesField.setVisible(false);
> +		sparesField.setDisabled(true);
> +	    }

this could be more elegantly solved in two other ways:
1. use a viewmodel with a formula that returns true/false
    depending on the startsWith('draid') and then use a
    bind on the hidden/disabled setting of the fields

2. put the bool into a variable and use that, like this

let isDraid = ...startsWith('draid');
dataField.setVisible(isDraid);
dataField.setDisabled(!isDraid);
...



> +	},
> +    },
>   });
>   
>   Ext.define('PVE.node.ZFSList', {