From 7aeb251f5486706c1f553da4938cb66cc93268eb Mon Sep 17 00:00:00 2001 From: Ilya Mashchenko Date: Sun, 14 Jul 2024 23:01:07 +0300 Subject: go.d smartctl: use scan-open when "no_check_power_mode" is "never" (#18146) --- src/go/plugin/go.d/modules/smartctl/exec.go | 4 ++ src/go/plugin/go.d/modules/smartctl/scan.go | 47 +++++++++++++++------- src/go/plugin/go.d/modules/smartctl/smartctl.go | 1 + .../plugin/go.d/modules/smartctl/smartctl_test.go | 4 ++ 4 files changed, 42 insertions(+), 14 deletions(-) diff --git a/src/go/plugin/go.d/modules/smartctl/exec.go b/src/go/plugin/go.d/modules/smartctl/exec.go index 48bd11d80e..289501f711 100644 --- a/src/go/plugin/go.d/modules/smartctl/exec.go +++ b/src/go/plugin/go.d/modules/smartctl/exec.go @@ -33,6 +33,10 @@ func (e *smartctlCliExec) scan() (*gjson.Result, error) { return e.execute("smartctl-json-scan") } +func (e *smartctlCliExec) scanOpen() (*gjson.Result, error) { + return e.execute("smartctl-json-scan-open") +} + func (e *smartctlCliExec) deviceInfo(deviceName, deviceType, powerMode string) (*gjson.Result, error) { return e.execute("smartctl-json-device-info", "--deviceName", deviceName, diff --git a/src/go/plugin/go.d/modules/smartctl/scan.go b/src/go/plugin/go.d/modules/smartctl/scan.go index 06c8cdcb7f..58af0b0cad 100644 --- a/src/go/plugin/go.d/modules/smartctl/scan.go +++ b/src/go/plugin/go.d/modules/smartctl/scan.go @@ -6,6 +6,8 @@ import ( "errors" "fmt" "strings" + + "github.com/tidwall/gjson" ) type scanDevice struct { @@ -24,7 +26,22 @@ func (s *scanDevice) shortName() string { } func (s *Smartctl) scanDevices() (map[string]*scanDevice, error) { - resp, err := s.exec.scan() + powerModeNever := s.NoCheckPowerMode == "never" + + var resp *gjson.Result + var err error + + // Issue on Discord: https://discord.com/channels/847502280503590932/1261747175361347644/1261747175361347644 + // "sat" devices being identified as "scsi" with --scan, and then later + // code attempts to validate the type by calling `smartctl` with the "scsi" type. + // This validation can trigger unintended "Enabling discard_zeroes_data" messages in system logs (dmesg). + // To address this specific issue we use `smartctl --scan-open` as a workaround. + // This method reliably identifies device types. + if powerModeNever { + resp, err = s.exec.scanOpen() + } else { + resp, err = s.exec.scan() + } if err != nil { return nil, fmt.Errorf("failed to scan devices: %v", err) } @@ -35,7 +52,7 @@ func (s *Smartctl) scanDevices() (map[string]*scanDevice, error) { dev := &scanDevice{ name: d.Get("name").String(), infoName: d.Get("info_name").String(), - typ: d.Get("type").String(), // guessed type (we do '--scan' not '--scan-open') + typ: d.Get("type").String(), // guessed type when using '--scan' instead of '--scan-open' } if dev.name == "" || dev.typ == "" { @@ -48,19 +65,21 @@ func (s *Smartctl) scanDevices() (map[string]*scanDevice, error) { continue } - if dev.typ == "scsi" { - // `smartctl --scan` attempts to guess the device type based on the path, but this can be unreliable. - // Accurate device type information is crucial because we use the `--device` option to gather data. - // Using the wrong type can lead to issues. - // For example, using 'scsi' for 'sat' devices prevents `smartctl` from issuing the necessary ATA commands. - d := scanDevice{name: dev.name, typ: "sat"} - if _, ok := s.scannedDevices[d.key()]; ok { - dev.typ = "sat" - } else { - resp, _ := s.exec.deviceInfo(dev.name, dev.typ, s.NoCheckPowerMode) - if resp != nil && isExitStatusHasBit(resp, 2) { - s.Debugf("changing device '%s' type 'scsi' -> 'sat'", dev.name) + if !powerModeNever { + if dev.typ == "scsi" { + // `smartctl --scan` attempts to guess the device type based on the path, but this can be unreliable. + // Accurate device type information is crucial because we use the `--device` option to gather data. + // Using the wrong type can lead to issues. + // For example, using 'scsi' for 'sat' devices prevents `smartctl` from issuing the necessary ATA commands. + d := scanDevice{name: dev.name, typ: "sat"} + if _, ok := s.scannedDevices[d.key()]; ok { dev.typ = "sat" + } else { + resp, _ := s.exec.deviceInfo(dev.name, dev.typ, s.NoCheckPowerMode) + if resp != nil && isExitStatusHasBit(resp, 2) { + s.Debugf("changing device '%s' type 'scsi' -> 'sat'", dev.name) + dev.typ = "sat" + } } } } diff --git a/src/go/plugin/go.d/modules/smartctl/smartctl.go b/src/go/plugin/go.d/modules/smartctl/smartctl.go index 0ba0491794..777336d95b 100644 --- a/src/go/plugin/go.d/modules/smartctl/smartctl.go +++ b/src/go/plugin/go.d/modules/smartctl/smartctl.go @@ -83,6 +83,7 @@ type ( } smartctlCli interface { scan() (*gjson.Result, error) + scanOpen() (*gjson.Result, error) deviceInfo(deviceName, deviceType, powerMode string) (*gjson.Result, error) } ) diff --git a/src/go/plugin/go.d/modules/smartctl/smartctl_test.go b/src/go/plugin/go.d/modules/smartctl/smartctl_test.go index 2f8d4d66a1..f66c3739a0 100644 --- a/src/go/plugin/go.d/modules/smartctl/smartctl_test.go +++ b/src/go/plugin/go.d/modules/smartctl/smartctl_test.go @@ -477,6 +477,10 @@ func (m *mockSmartctlCliExec) scan() (*gjson.Result, error) { return &res, nil } +func (m *mockSmartctlCliExec) scanOpen() (*gjson.Result, error) { + return m.scan() +} + func (m *mockSmartctlCliExec) deviceInfo(deviceName, deviceType, powerMode string) (*gjson.Result, error) { if m.deviceDataFunc == nil { return nil, nil -- cgit v1.2.3