Skip to content

Commit

Permalink
go.d smartctl: use scan-open when "no_check_power_mode" is "never" (n…
Browse files Browse the repository at this point in the history
  • Loading branch information
ilyam8 authored Jul 14, 2024
1 parent cb126dc commit 7aeb251
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 14 deletions.
4 changes: 4 additions & 0 deletions src/go/plugin/go.d/modules/smartctl/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
47 changes: 33 additions & 14 deletions src/go/plugin/go.d/modules/smartctl/scan.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import (
"errors"
"fmt"
"strings"

"github.com/tidwall/gjson"
)

type scanDevice struct {
Expand All @@ -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)
}
Expand All @@ -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 == "" {
Expand All @@ -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"
}
}
}
}
Expand Down
1 change: 1 addition & 0 deletions src/go/plugin/go.d/modules/smartctl/smartctl.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ type (
}
smartctlCli interface {
scan() (*gjson.Result, error)
scanOpen() (*gjson.Result, error)
deviceInfo(deviceName, deviceType, powerMode string) (*gjson.Result, error)
}
)
Expand Down
4 changes: 4 additions & 0 deletions src/go/plugin/go.d/modules/smartctl/smartctl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 7aeb251

Please sign in to comment.