From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 ; 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 ; 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 ; 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 ; Tue, 7 Jun 2022 16:41:15 +0200 (CEST) Message-ID: 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 , Proxmox VE development discussion 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 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-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 >> --- >> 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', { > >