From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <s.hrdlicka@proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
 key-exchange X25519 server-signature RSA-PSS (2048 bits))
 (No client certificate requested)
 by lists.proxmox.com (Postfix) with ESMTPS id 86B6870FE6
 for <pve-devel@lists.proxmox.com>; Tue,  7 Jun 2022 16:41:47 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
 by firstgate.proxmox.com (Proxmox) with ESMTP id 783ED2F376
 for <pve-devel@lists.proxmox.com>; Tue,  7 Jun 2022 16:41:17 +0200 (CEST)
Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com
 [94.136.29.106])
 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
 key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256)
 (No client certificate requested)
 by firstgate.proxmox.com (Proxmox) with ESMTPS id 0F04A2F36B
 for <pve-devel@lists.proxmox.com>; Tue,  7 Jun 2022 16:41:16 +0200 (CEST)
Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1])
 by proxmox-new.maurer-it.com (Proxmox) with ESMTP id DA04C43A56
 for <pve-devel@lists.proxmox.com>; Tue,  7 Jun 2022 16:41:15 +0200 (CEST)
Message-ID: <d060066a-c134-725c-5dba-fd7bbcbdfbac@proxmox.com>
Date: Tue, 7 Jun 2022 16:41:14 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.9.0
Content-Language: en-US
To: Dominik Csapak <d.csapak@proxmox.com>,
 Proxmox VE development discussion <pve-devel@lists.proxmox.com>
References: <20220602112234.1868726-1-s.hrdlicka@proxmox.com>
 <20220602112234.1868726-3-s.hrdlicka@proxmox.com>
 <0af90ff2-b240-cc4b-5254-d43c98ab53fa@proxmox.com>
From: Stefan Hrdlicka <s.hrdlicka@proxmox.com>
In-Reply-To: <0af90ff2-b240-cc4b-5254-d43c98ab53fa@proxmox.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
X-SPAM-LEVEL: Spam detection results:  0
 AWL 1.680 Adjusted score from AWL reputation of From: address
 BAYES_00                 -1.9 Bayes spam probability is 0 to 1%
 KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
 NICE_REPLY_A           -2.889 Looks like a legit reply (A)
 SPF_HELO_NONE           0.001 SPF: HELO does not publish an SPF Record
 SPF_PASS               -0.001 SPF: sender matches SPF record
 T_SCC_BODY_TEXT_LINE    -0.01 -
Subject: Re: [pve-devel] [PATCH pve-manager 2/3] fix #3967: enable ZFS dRAID
 creation in WebGUI
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
X-List-Received-Date: Tue, 07 Jun 2022 14:41:47 -0000



On 6/3/22 14:24, Dominik Csapak wrote:
> 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
> 

I would say you nearly always want to change at least the number of 
spares. The default values for creating a dRAID without setting anything 
are 0 spares and 8 devices per redundancy group (if disks >=8). The 
advantage of dRAID is fast rebuilding of a broken disk which you will 
only get (as far as I read) when you have spares configured.

The data devices per redundancy group would be possible to hide. But 
they have an impact on performance depending on the number chosen as 
well as storage usage. I'm not sure that it makes sense hiding one value 
of the two. I will think about it ;).


>>               {
>>                   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', {
> 
>