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 6E3C47AD13 for ; Mon, 10 May 2021 14:27:34 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 3FEBA185C2 for ; Mon, 10 May 2021 14:27:04 +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 id EE62A185B6 for ; Mon, 10 May 2021 14:27:02 +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 1634945B6E for ; Mon, 10 May 2021 14:27:02 +0200 (CEST) Date: Mon, 10 May 2021 14:27:00 +0200 From: Oguz Bektas To: Dominik Csapak Cc: Proxmox VE development discussion Message-ID: <20210510122700.GA9727@gaia.proxmox.com> Mail-Followup-To: Oguz Bektas , Dominik Csapak , Proxmox VE development discussion References: <20210401134055.701355-1-o.bektas@proxmox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) X-SPAM-LEVEL: Spam detection results: 1 AWL 1.226 Adjusted score from AWL reputation of From: address KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment 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 storage 1/2] smartctl: remove to-be-replaced disk_tests 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: Mon, 10 May 2021 12:27:34 -0000 On Mon, May 10, 2021 at 02:15:37PM +0200, Dominik Csapak wrote: > hi, > > sorry for the long wait on a review, but better > late than never ;) > > so one high level comments: > > why do you remove all disk tests ?? > they did not only test the smart output, but also the > usage, ssd/hdd detection, vendor/model etc... > > i understand that you do not have the original disks > for the smartctl -j output, but it should not be > *that* hard to fake it ;) at the time i couldn't figure out a way to keep all the tests working while also adding the json changes, so for proof of concept i went for the simpler option. thomas thought it would also be acceptable to incrementally add the new tests over time, making sure that everything works with the new changes. i have to admit i haven't really tried to fake the outputs, mainly because there were a lot of disk types that produce different kind of results (one issue stefan noticed at an earlier version was that his disk's smart result returned an array with the temperature sensors, while the disks i've tested did not have these.) all these vendor-specific quirks make it really hard to produce reliable tests without the real hardware. > > and even if it is, please change the tests so that > only the smart part is not tested anymore, e.g. with a flag > in the test itself.... that can be an option, indeed > > so please find a way to keep the tests (as much as you can) > > i'll comment more on the second patch > >