Re: [pve-devel] [PATCH v2 storage] Diskmanage: Use S.M.A.R.T. attributes for SSDs wearout lookup

["Jan-Jonas Sämann" <sprinterfreak@binary-kitchen.de>]

On 27.10.20 09:08, Thomas Lamprecht wrote:
> On 24.10.20 21:27, Jan-Jonas Sämann wrote:
>> This replaces a locally maintained hardware map in
>> get_wear_leveling_info() by commonly used register names of
>> smartmontool. Smartmontool maintains a labeled register database that
>> contains a majority of drives (including versions). The current lookup
>> produces false estimates, this approach hopefully provides more reliable
>> data.
>>
>> Signed-off-by: Jan-Jonas Sämann <sprinterfreak@binary-kitchen.de>
>> ---
>>  PVE/Diskmanage.pm | 58 +++++++++++++++++++++++------------------------
>>  1 file changed, 28 insertions(+), 30 deletions(-)
>>
> 
> seems like a nicer approach in general, could you please point to some code/reference
> from where you took those register names below?

It's directly from the sources of smartmontools:
 https://www.smartmontools.org/browser/trunk/smartmontools/drivedb.h

Most of them correspond to the old value from vendormap. There are a couple exceptions
where some vendors/drives do not have a direct equivalent. There I estimated the closest possible
match. It's still a mess. Would be really cool if at least smartmontools provided a common fixed
attribute for each drive. So maybe I missed some, hard to say.
The register names ensure we don't acidently interpret unrelated data in some weird circumstances.

> 
>> diff --git a/PVE/Diskmanage.pm b/PVE/Diskmanage.pm
>> index 79aafcc..20dbeeb 100644
>> --- a/PVE/Diskmanage.pm
>> +++ b/PVE/Diskmanage.pm
>> @@ -396,7 +396,7 @@ sub get_sysdir_info {
>>  }
>>  
>>  sub get_wear_leveling_info {
>> -    my ($smartdata, $model) = @_;
>> +    my ($smartdata) = @_;
>>      my $attributes = $smartdata->{attributes};
>>  
>>      if (defined($smartdata->{wearout})) {
>> @@ -405,37 +405,35 @@ sub get_wear_leveling_info {
>>  
>>      my $wearout;
>>  
>> -    my $vendormap = {
>> -	'kingston' => 231,
>> -	'samsung' => 177,
>> -	'intel' => 233,
>> -	'sandisk' => 233,
>> -	'crucial' => 202,
>> -	'default' => 233,
>> -    };
>> -
>> -    # find target attr id
>> -
>> -    my $attrid;
>> -
>> -    foreach my $vendor (keys %$vendormap) {
>> -	if ($model =~ m/$vendor/i) {
>> -	    $attrid = $vendormap->{$vendor};
>> -	    # found the attribute
>> -	    last;
>> +    # Common register names that represent percentage values of potential
>> +    # failure indicators used in drivedb.h of smartmontool's. Order matters,
>> +    # as some drives may have multiple definitions
>> +    my @wearoutregisters = (
>> +	"SSD_Life_Left",
>> +	"Wear_Leveling_Count",
>> +	"Perc_Write\/Erase_Ct_BC",
>> +	"Perc_Rated_Life_Remain",
>> +	"Remaining_Lifetime_Perc",
>> +	"Percent_Lifetime_Remain",
>> +	"Lifetime_Left",
>> +	"PCT_Life_Remaining",
>> +	"Lifetime_Remaining",
>> +	"Percent_Life_Remaining",
>> +	"Percent_Lifetime_Used",
>> +	"Perc_Rated_Life_Used"
>> +    );
>> +
>> +    # Search for S.M.A.R.T. attributes for known register
>> +    foreach my $register (@wearoutregisters) {
>> +	last if defined $wearout;
>> +	foreach my $attr (@$attributes) {
>> +	   next if $attr->{name} !~ m/$register/;
>> +	   # Store wearout value, invert value if register matches "Used"
>> +	   $wearout = ($attr->{name} =~ /Used/) ? 100 - $attr->{value} : $attr->{value};
>> +	   last;
>>  	}
>>      }
>>  
>> -    if (!$attrid) {
>> -	$attrid = $vendormap->{default};
>> -    }
>> -
>> -    foreach my $attr (@$attributes) {
>> -	next if $attr->{id} != $attrid;
>> -	$wearout = $attr->{value};
>> -	last;
>> -    }
>> -
>>      return $wearout;
>>  }
>>  
>> @@ -559,7 +557,7 @@ sub get_disks {
>>  
>>  		if (is_ssdlike($type)) {
>>  		    # if we have an ssd we try to get the wearout indicator
>> -		    my $wearval = get_wear_leveling_info($smartdata, $data->{model} || $sysdata->{model});
>> +		    my $wearval = get_wear_leveling_info($smartdata);
>>  		    $wearout = $wearval if defined($wearval);
>>  		}
>>  	    };
>>
> 
> 
>