go.d smarctl simplify scan open (#18180)
Ilya Mashchenko committed
Jul 16, 2024 at 12:01 UTC
cf990899c711981afafc625e88f7917f6304a87e
4 files changed
+22
-39
src/go/plugin/go.d/modules/smartctl/exec.go
+4
-5
@@ -29,14 +29,13 @@ type smartctlCliExec struct {
29
timeout time.Duration
30
}
31
32
-func (e *smartctlCliExec) scan() (*gjson.Result, error) {
32
+func (e *smartctlCliExec) scan(open bool) (*gjson.Result, error) {
33
+ if open {
34
+ return e.execute("smartctl-json-scan-open")
35
+ }
36
return e.execute("smartctl-json-scan")
37
}
38
36
-func (e *smartctlCliExec) scanOpen() (*gjson.Result, error) {
37
- return e.execute("smartctl-json-scan-open")
38
-}
39
-
39
func (e *smartctlCliExec) deviceInfo(deviceName, deviceType, powerMode string) (*gjson.Result, error) {
40
return e.execute("smartctl-json-device-info",
41
"--deviceName", deviceName,
src/go/plugin/go.d/modules/smartctl/scan.go
+16
-27
@@ -6,8 +6,6 @@ import (
6
"errors"
7
"fmt"
8
"strings"
9
-
10
- "github.com/tidwall/gjson"
9
)
10
11
type scanDevice struct {
@@ -26,22 +24,15 @@ func (s *scanDevice) shortName() string {
24
}
25
26
func (s *Smartctl) scanDevices() (map[string]*scanDevice, error) {
29
- powerModeNever := s.NoCheckPowerMode == "never"
30
-
31
- var resp *gjson.Result
32
- var err error
33
-
27
// Issue on Discord: https://discord.com/channels/847502280503590932/1261747175361347644/1261747175361347644
28
// "sat" devices being identified as "scsi" with --scan, and then later
29
// code attempts to validate the type by calling `smartctl` with the "scsi" type.
30
// This validation can trigger unintended "Enabling discard_zeroes_data" messages in system logs (dmesg).
31
// To address this specific issue we use `smartctl --scan-open` as a workaround.
32
// This method reliably identifies device types.
40
- if powerModeNever {
41
- resp, err = s.exec.scanOpen()
42
- } else {
43
- resp, err = s.exec.scan()
44
- }
33
+ scanOpen := s.NoCheckPowerMode == "never"
34
+
35
+ resp, err := s.exec.scan(scanOpen)
36
if err != nil {
37
return nil, fmt.Errorf("failed to scan devices: %v", err)
38
}
@@ -52,7 +43,7 @@ func (s *Smartctl) scanDevices() (map[string]*scanDevice, error) {
43
dev := &scanDevice{
44
name: d.Get("name").String(),
45
infoName: d.Get("info_name").String(),
55
- typ: d.Get("type").String(), // guessed type when using '--scan' instead of '--scan-open'
46
+ typ: d.Get("type").String(),
47
}
48
49
if dev.name == "" || dev.typ == "" {
@@ -65,21 +56,19 @@ func (s *Smartctl) scanDevices() (map[string]*scanDevice, error) {
56
continue
57
}
58
68
- if !powerModeNever {
69
- if dev.typ == "scsi" {
70
- // `smartctl --scan` attempts to guess the device type based on the path, but this can be unreliable.
71
- // Accurate device type information is crucial because we use the `--device` option to gather data.
72
- // Using the wrong type can lead to issues.
73
- // For example, using 'scsi' for 'sat' devices prevents `smartctl` from issuing the necessary ATA commands.
74
- d := scanDevice{name: dev.name, typ: "sat"}
75
- if _, ok := s.scannedDevices[d.key()]; ok {
59
+ if !scanOpen && dev.typ == "scsi" {
60
+ // `smartctl --scan` attempts to guess the device type based on the path, but this can be unreliable.
61
+ // Accurate device type information is crucial because we use the `--device` option to gather data.
62
+ // Using the wrong type can lead to issues.
63
+ // For example, using 'scsi' for 'sat' devices prevents `smartctl` from issuing the necessary ATA commands.
64
+ d := scanDevice{name: dev.name, typ: "sat"}
65
+ if _, ok := s.scannedDevices[d.key()]; ok {
66
+ dev.typ = "sat"
67
+ } else {
68
+ resp, _ := s.exec.deviceInfo(dev.name, dev.typ, s.NoCheckPowerMode)
69
+ if resp != nil && isExitStatusHasBit(resp, 2) {
70
+ s.Debugf("changing device '%s' type 'scsi' -> 'sat'", dev.name)
71
dev.typ = "sat"
77
- } else {
78
- resp, _ := s.exec.deviceInfo(dev.name, dev.typ, s.NoCheckPowerMode)
79
- if resp != nil && isExitStatusHasBit(resp, 2) {
80
- s.Debugf("changing device '%s' type 'scsi' -> 'sat'", dev.name)
81
- dev.typ = "sat"
82
- }
72
}
73
}
74
}
src/go/plugin/go.d/modules/smartctl/smartctl.go
+1
-2
@@ -82,8 +82,7 @@ type (
82
mx map[string]int64
83
}
84
smartctlCli interface {
85
- scan() (*gjson.Result, error)
86
- scanOpen() (*gjson.Result, error)
85
+ scan(open bool) (*gjson.Result, error)
86
deviceInfo(deviceName, deviceType, powerMode string) (*gjson.Result, error)
87
}
88
)
src/go/plugin/go.d/modules/smartctl/smartctl_test.go
+1
-5
@@ -469,7 +469,7 @@ type mockSmartctlCliExec struct {
469
deviceDataFunc func(deviceName, deviceType, powerMode string) ([]byte, error)
470
}
471
472
-func (m *mockSmartctlCliExec) scan() (*gjson.Result, error) {
472
+func (m *mockSmartctlCliExec) scan(_ bool) (*gjson.Result, error) {
473
if m.errOnScan {
474
return nil, fmt.Errorf("mock.scan() error")
475
}
@@ -477,10 +477,6 @@ func (m *mockSmartctlCliExec) scan() (*gjson.Result, error) {
477
return &res, nil
478
}
479
480
-func (m *mockSmartctlCliExec) scanOpen() (*gjson.Result, error) {
481
- return m.scan()
482
-}
483
-
480
func (m *mockSmartctlCliExec) deviceInfo(deviceName, deviceType, powerMode string) (*gjson.Result, error) {
481
if m.deviceDataFunc == nil {
482
return nil, nil