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 B29D38AB86 for ; Fri, 21 Oct 2022 11:24:59 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 737B31D0A9 for ; Fri, 21 Oct 2022 11:24:29 +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)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Fri, 21 Oct 2022 11:24:13 +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 6D9D444AF4; Fri, 21 Oct 2022 11:24:12 +0200 (CEST) Message-ID: <5ff01d81-cb4b-fea1-8810-60eb8522f585@proxmox.com> Date: Fri, 21 Oct 2022 11:24:11 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:106.0) Gecko/20100101 Thunderbird/106.0 Content-Language: en-US To: Proxmox VE development discussion , Stefan Hrdlicka , Thomas Lamprecht References: <20220930123802.772865-1-s.hrdlicka@proxmox.com> From: Dominik Csapak In-Reply-To: <20220930123802.772865-1-s.hrdlicka@proxmox.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.068 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 -0.001 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 Subject: Re: [pve-devel] [PATCH V6 pve-manager 0/2] fix #2822: add iscsi, lvm, lvmthin & zfs 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: Fri, 21 Oct 2022 09:24:59 -0000 All in all LGTM, one small UX thing that i am not sure if we can improve without making it way more complicated: We now preselect the current node and leave the restriction out. We then restrict when the node changes, but remove the restriction again when we select the current node. Maybe we want to just leave the restriction out when we load the panel up, but always restrict when a user sets the node? (as in, if i manually select the current node again, restrict it?) not sure if we'd want that or leave it like it is (or something else entirely?) @Thomas do you have any thoughts about that? Aside from that, consider the series: Reviewed-by: Dominik Csapak Tested-by: Dominik Csapak On 9/30/22 14:38, Stefan Hrdlicka wrote: > V1 -> V2: > # pve-storage > * removed because patch is not needed > > # pve-manager (1/3) > * remove storage controller from V1 > * added custom ComboBox with API URL & setNodeName function > * added scan node selection for iSCSI > * scan node selection field no longer send to server > > ## (optional) pve-manager (2/3): cleanup related files > * var to let statement change > * some indentation > > ## ((very) optional) pve-manager (3/3): cleanup all var statements > * replaces all var with let statements > > > V2 -> V3: > # pve-manager (1/2) > * fix broken interface (broken in V2 ) > * improve tooltip > * replace jNodeSelector function with class object > (PVE.panel.StorageBaseWithNodeSelector) > > # other things: > * removed very optional cleanup > * nothing changed for "Base storage" selector. It is still possible to > select for example an iSCSI device only availabe on one node that > isn't availabe on the other ones. I wasn't sure if this should be > changed in this context as well. > > > V3 -> V4: > # pve-manager (1/2) > * "localhost" is set for "Scan Node" > * if another node is selcted and the "X" (clear) is used, localhost is > set as the value again > * use lookupReference > * moved used template literals for building path strings > > > V4 -> V5: > # pve-manager (1/2) > * s/lookupReference/lookup/ > * move ComboBoxSetStoreNode & StrageScanNodeSelector > to www/manager6/form > * move array pushes to initialization of array > > V5 -> V6: > # pve-manager (1/2) > * set default value of "Scan node" to Proxmox.NodeName > * don't allow empty "Scan node" > > Stefan Hrdlicka (2): > fix #2822: add iscsi, lvm, lvmthin & zfs storage for all cluster nodes > cleanup: "var" to "let", fix some indentation in related files > > www/manager6/Makefile | 2 + > www/manager6/form/ComboBoxSetStoreNode.js | 16 +++++ > www/manager6/form/StorageScanNodeSelector.js | 31 +++++++++ > www/manager6/storage/Base.js | 11 +-- > www/manager6/storage/IScsiEdit.js | 35 +++++++--- > www/manager6/storage/LVMEdit.js | 43 +++++++++--- > www/manager6/storage/LvmThinEdit.js | 70 ++++++++++++++------ > www/manager6/storage/ZFSPoolEdit.js | 51 +++++++++----- > 8 files changed, 199 insertions(+), 60 deletions(-) > create mode 100644 www/manager6/form/ComboBoxSetStoreNode.js > create mode 100644 www/manager6/form/StorageScanNodeSelector.js >