merge get_smart_data/health

instead of parsing the output of smart in two places,
give get_smart_data a flag if we only want health

this fixes a bug (not on the bugtracker), where
an ssd with disabled smart had an empty string as health
in the gui

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>

diff --git a/PVE/API2/Disks.pm b/PVE/API2/Disks.pm
index 208485f6ab465b51892f3c7cf10be32ba2af425f..366cd629137839930c555216445c8297cd9b8101 100644 (file)
--- a/PVE/API2/Disks.pm
+++ b/PVE/API2/Disks.pm
@@ -140,13 +140,10 @@ __PACKAGE__->register_method ({
 
        my $result = {};
 
-       if ($param->{healthonly}) {
-           $result = { health => PVE::Diskmanage::get_smart_health($disk) };
-       } else {
-           $result = PVE::Diskmanage::get_smart_data($disk);
-       }
+       my $result = PVE::Diskmanage::get_smart_data($disk, $param->{healthonly});
 
        $result->{health} = 'UNKNOWN' if !defined $result->{health};
+       $result = { health => $result->{health} } if $param->{healthonly};
 
        return $result;
     }});

diff --git a/PVE/Diskmanage.pm b/PVE/Diskmanage.pm
index 36a5b5b7cbac26ea1255143dc902c2a755710a22..4ee89ae89860c03300c40044b5c0a7e32f5969d6 100644 (file)
--- a/PVE/Diskmanage.pm
+++ b/PVE/Diskmanage.pm
@@ -70,7 +70,7 @@ sub disk_is_used {
 }
 
 sub get_smart_data {
-    my ($disk) = @_;
+    my ($disk, $healthonly) = @_;
 
     assert_blockdev($disk);
     my $smartdata = {};
@@ -81,8 +81,12 @@ sub get_smart_data {
     $disk =~ s/n\d+$//
         if $disk =~ m!^/dev/nvme\d+n\d+$!;
 
+    my $cmd = [$SMARTCTL, '-H'];
+    push @$cmd, '-A', '-f', 'brief' if !$healthonly;
+    push @$cmd, $disk;
+
     eval {
-       $returncode = run_command([$SMARTCTL, '-H', '-A', '-f', 'brief', $disk], noerr => 1, outfunc => sub{
+       $returncode = run_command($cmd, noerr => 1, outfunc => sub{
            my ($line) = @_;
 
 # ATA SMART attributes, e.g.:
@@ -115,6 +119,8 @@ sub get_smart_data {
            } elsif (defined($type) && $type eq 'text') {
                $smartdata->{text} = '' if !defined $smartdata->{text};
                $smartdata->{text} .= "$line\n";
+           } elsif ($line =~ m/SMART Disabled/) {
+               $smartdata->{health} = "SMART Disabled";
            }
        });
     };
@@ -132,30 +138,6 @@ sub get_smart_data {
     return $smartdata;
 }
 
-sub get_smart_health {
-    my ($disk) = @_;
-
-    return "NOT A DEVICE" if !assert_blockdev($disk, 1);
-
-    my $message;
-    $disk =~ s/n\d+$//
-        if $disk =~ m!^/dev/nvme\d+n\d+$!;
-
-    run_command([$SMARTCTL, '-H', $disk], noerr => 1, outfunc => sub {
-       my ($line) = @_;
-
-       if ($line =~ m/test result: (.*)$/) {
-           $message = $1;
-       } elsif ($line =~ m/open device: (.*) failed: (.*)$/) {
-           $message = "FAILED TO OPEN";
-       } elsif ($line =~ m/^SMART Disabled/) {
-           $message = "SMART DISABLED";
-       }
-    });
-
-    return $message;
-}
-
 sub get_zfs_devices {
     my $list = {};
 
@@ -388,11 +370,12 @@ sub get_disks {
 
        if (!$nosmart) {
            eval {
+               my $smartdata = get_smart_data($devpath, ($type ne 'ssd'));
+               $health = $smartdata->{health} if $smartdata->{health};
+
                if ($type eq 'ssd') {
                    # if we have an ssd we try to get the wearout indicator
                    $wearout = 'N/A';
-                   my $smartdata = get_smart_data($devpath);
-                   $health = $smartdata->{health};
                    foreach my $attr (@{$smartdata->{attributes}}) {
                        # ID 233 is media wearout indicator on intel and sandisk
                        # ID 177 is media wearout indicator on samsung
@@ -403,9 +386,6 @@ sub get_disks {
                        # prefer the 233 value
                        last if ($attr->{id} == 233);
                    }
-               } else {
-                   # else we just get the health
-                   $health = get_smart_health($devpath);
                }
            };
        }