Code cleanups and gofmt run.

I refactored the 3 different /sys/class entries in class_sas_device, because they're almost exactly the same except for the directory name and which devices are included in the directory.

Signed-off-by: Scott Laird <scott@sigkill.org>

sysfs/class_sas_device.go

@@ -1,4 +1,4 @@
-// Copyright 2021 The Prometheus Authors
+// Copyright 2022 The Prometheus Authors
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
 // You may obtain a copy of the License at
@@ -28,18 +28,21 @@ const sasEndDeviceClassPath = "class/sas_end_device"
 const sasExpanderClassPath = "class/sas_expander"
 
 type SASDevice struct {
-	Name       string   // /sys/class/sas_device/<Name>
-	SASAddress string   // /sys/class/sas_device/<Name>/sas_address
-	SASPhys    []string // /sys/class/sas_device/<Name>/device/phy-*
-	SASPorts   []string // /sys/class/sas_device/<Name>/device/ports-*
-	BlockDevices []string  // /sys/class/sas_device/<Name>/device/target*/*/block/*
+	Name         string   // /sys/class/sas_device/<Name>
+	SASAddress   string   // /sys/class/sas_device/<Name>/sas_address
+	SASPhys      []string // /sys/class/sas_device/<Name>/device/phy-*
+	SASPorts     []string // /sys/class/sas_device/<Name>/device/ports-*
+	BlockDevices []string // /sys/class/sas_device/<Name>/device/target*/*/block/*
 }
 
 type SASDeviceClass map[string]SASDevice
 
-// SASDeviceClass parses devices in /sys/class/sas_device.
-func (fs FS) SASDeviceClass() (SASDeviceClass, error) {
-	path := fs.sys.Path(sasDeviceClassPath)
+// sasDeviceClasses reads all of the SAS devices from a specific set
+// of /sys/class/sas*/ entries.  The sas_device, sas_end_device, and
+// sas_expander classes are all nearly identical and can be handled by the same basic code.
+
+func (fs FS) parseSASDeviceClass(dir string) (SASDeviceClass, error) {
+	path := fs.sys.Path(dir)
 
 	dirs, err := ioutil.ReadDir(path)
 	if err != nil {
@@ -60,57 +63,26 @@ func (fs FS) SASDeviceClass() (SASDeviceClass, error) {
 	return sdc, nil
 }
 
+// SASDeviceClass parses devices in /sys/class/sas_device.
+func (fs FS) SASDeviceClass() (SASDeviceClass, error) {
+	return fs.parseSASDeviceClass(sasDeviceClassPath)
+}
+
 // SASEndDeviceClass parses devices in /sys/class/sas_end_device.
-// This is *almost* identical to sas_device, just with a different
-// base directory.  The major difference is that end_devices don't
-// include expanders and other infrastructure devices.
+// This is a subset of sas_device, and excludes expanders.
 func (fs FS) SASEndDeviceClass() (SASDeviceClass, error) {
-	path := fs.sys.Path(sasEndDeviceClassPath)
-
-	dirs, err := ioutil.ReadDir(path)
-	if err != nil {
-		return nil, err
-	}
-
-	sdc := make(SASDeviceClass, len(dirs))
-
-	for _, d := range dirs {
-		device, err := fs.parseSASDevice(d.Name())
-		if err != nil {
-			return nil, err
-		}
-
-		sdc[device.Name] = *device
-	}
-
-	return sdc, nil
+	return fs.parseSASDeviceClass(sasEndDeviceClassPath)
 }
 
 // SASExpanderClass parses devices in /sys/class/sas_expander.
-// This is *almost* identical to sas_device, but only includes expanders.
+// This is a subset of sas_device, but only includes expanders.
 func (fs FS) SASExpanderClass() (SASDeviceClass, error) {
-	path := fs.sys.Path(sasExpanderClassPath)
-
-	dirs, err := ioutil.ReadDir(path)
-	if err != nil {
-		return nil, err
-	}
-
-	sdc := make(SASDeviceClass, len(dirs))
-
-	for _, d := range dirs {
-		device, err := fs.parseSASDevice(d.Name())
-		if err != nil {
-			return nil, err
-		}
-
-		sdc[device.Name] = *device
-	}
-
-	return sdc, nil
+	return fs.parseSASDeviceClass(sasExpanderClassPath)
 }
 
-// Parse a single sas_device.
+// Parse a single sas_device.  This uses /sys/class/sas_device, as
+// it's a superset of the other two directories so there's no reason
+// to plumb the path through to here.
 func (fs FS) parseSASDevice(name string) (*SASDevice, error) {
 	device := SASDevice{Name: name}
 
@@ -125,37 +97,37 @@ func (fs FS) parseSASDevice(name string) (*SASDevice, error) {
 	portDevice := regexp.MustCompile(`^port-[0-9:]+$`)
 
 	for _, d := range dirs {
-		if phyDevice.Match([]byte(d.Name())) {
+		if phyDevice.MatchString(d.Name()) {
 			device.SASPhys = append(device.SASPhys, d.Name())
 		}
-		if portDevice.Match([]byte(d.Name())) {
+		if portDevice.MatchString(d.Name()) {
 			device.SASPorts = append(device.SASPorts, d.Name())
 		}
 	}
 
 	address := fs.sys.Path(sasDeviceClassPath, name, "sas_address")
 	value, err := util.SysReadFile(address)
-
 	if err != nil {
-		return &device, err
-	} else {
-		device.SASAddress = value
+		return nil, err
 	}
+	device.SASAddress = value
 
 	device.BlockDevices, err = fs.blockSASDeviceBlockDevices(name)
 	if err != nil {
-		return &device, err
+		return nil, err
 	}
 
 	return &device, nil
 }
 
 // Identify block devices that map to a specific SAS Device
-// This info comes from (for example) /sys/class/sas_device/end_device-11:2/device/target11:0:0/11:0:0:0/block/sdp
+// This info comes from (for example)
+// /sys/class/sas_device/end_device-11:2/device/target11:0:0/11:0:0:0/block/sdp
 //
 // To find that, we have to look in the device directory for target$X
-// subdirs, then a subdir of $X, then read from directory names in the
-// 'block/' subdirectory under that.
+// subdirs, then specific subdirs of $X, then read from directory
+// names in the 'block/' subdirectory under that.  This really
+// shouldn't be this hard.
 func (fs FS) blockSASDeviceBlockDevices(name string) ([]string, error) {
 	var devices []string
 
@@ -177,28 +149,26 @@ func (fs FS) blockSASDeviceBlockDevices(name string) ([]string, error) {
 			if err != nil {
 				return nil, err
 			}
-			
+
 			for _, targetsubdir := range subtargets {
 
 				if !targetSubDevice.MatchString(targetsubdir.Name()) {
 					// need to skip 'power', 'subsys', etc.
 					continue
 				}
-				
+
 				blocks, err := ioutil.ReadDir(filepath.Join(devicepath, targetdir, targetsubdir.Name(), "block"))
-				
+
 				if err != nil {
 					return nil, err
 				}
-				
+
 				for _, blockdevice := range blocks {
 					devices = append(devices, blockdevice.Name())
 				}
 			}
 		}
 	}
 
-
-
 	return devices, nil
 }

sysfs/class_sas_device_test.go

@@ -1,4 +1,4 @@
-// Copyright 2021 The Prometheus Authors
+// Copyright 2022 The Prometheus Authors
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
 // You may obtain a copy of the License at
@@ -17,9 +17,8 @@
 package sysfs
 
 import (
-	"testing"
-
 	"github.com/google/go-cmp/cmp"
+	"testing"
 )
 
 func TestSASDeviceClass(t *testing.T) {

sysfs/class_sas_host.go

@@ -18,16 +18,16 @@ package sysfs
 
 import (
 	"io/ioutil"
-        "path/filepath"
+	"path/filepath"
 	"regexp"
 )
 
 const sasHostClassPath = "class/sas_host"
 
 type SASHost struct {
-	Name     string           // /sys/class/sas_host/<Name>
-	SASPhys []string         // /sys/class/sas_host/<Name>/device/phy-*
-	SASPorts []string       // /sys/class/sas_host/<Name>/device/ports-*
+	Name     string   // /sys/class/sas_host/<Name>
+	SASPhys  []string // /sys/class/sas_host/<Name>/device/phy-*
+	SASPorts []string // /sys/class/sas_host/<Name>/device/ports-*
 }
 
 type SASHostClass map[string]SASHost
@@ -39,7 +39,8 @@ type SASHostClass map[string]SASHost
 // The sas_host class doesn't collect any obvious statistics.  Each
 // sas_host contains a scsi_host, which seems to collect a couple
 // minor stats (ioc_reset_count and reply_queue_count), but they're
-// not worth collecting at this time.
+// not worth collecting at this time.  There are more useful SAS stats
+// in the sas_phy class.
 func (fs FS) SASHostClass() (SASHostClass, error) {
 	path := fs.sys.Path(sasHostClassPath)
 
@@ -67,7 +68,7 @@ func (fs FS) parseSASHost(name string) (*SASHost, error) {
 	//path := fs.sys.Path(sasHostClassPath, name)
 	host := SASHost{Name: name}
 
-	devicepath := fs.sys.Path(filepath.Join(sasHostClassPath,name,"device"))
+	devicepath := fs.sys.Path(filepath.Join(sasHostClassPath, name, "device"))
 
 	dirs, err := ioutil.ReadDir(devicepath)
 	if err != nil {
@@ -78,10 +79,10 @@ func (fs FS) parseSASHost(name string) (*SASHost, error) {
 	portDevice := regexp.MustCompile(`^port-[0-9:]+$`)
 
 	for _, d := range dirs {
-		if phyDevice.Match([]byte(d.Name())) {
+		if phyDevice.MatchString(d.Name()) {
 			host.SASPhys = append(host.SASPhys, d.Name())
 		}
-		if portDevice.Match([]byte(d.Name())) {
+		if portDevice.MatchString(d.Name()) {
 			host.SASPorts = append(host.SASPorts, d.Name())
 		}
 	}

sysfs/class_sas_host_test.go

@@ -17,9 +17,8 @@
 package sysfs
 
 import (
-	"testing"
-
 	"github.com/google/go-cmp/cmp"
+	"testing"
 )
 
 func TestSASHostClass(t *testing.T) {
@@ -36,13 +35,14 @@ func TestSASHostClass(t *testing.T) {
 	want := SASHostClass{
 		"host11": SASHost{
 			Name: "host11",
-        		SASPhys: []string{
-        			"phy-11:10", "phy-11:11", "phy-11:12", "phy-11:13",
-        			"phy-11:14", "phy-11:15", "phy-11:7", "phy-11:8", "phy-11:9",
-        		},
+			SASPhys: []string{
+				"phy-11:10", "phy-11:11", "phy-11:12", "phy-11:13",
+				"phy-11:14", "phy-11:15", "phy-11:7", "phy-11:8",
+				"phy-11:9",
+			},
 			SASPorts: []string{
-        			"port-11:0", "port-11:1", "port-11:2",
-        		},
+				"port-11:0", "port-11:1", "port-11:2",
+			},
 		},
 	}
 

sysfs/class_sas_phy.go

@@ -23,8 +23,8 @@ import (
 	"os"
 	"path/filepath"
 	"regexp"
-	"strings"
 	"strconv"
+	"strings"
 )
 
 const sasPhyClassPath = "class/sas_phy"
@@ -143,8 +143,13 @@ func (fs FS) parseSASPhy(name string) (*SASPhy, error) {
 	return &phy, nil
 }
 
+// parseLinkRate turns the kernel's SAS linkrate values into floats.
+// The kernel returns values like "12.0 Gbit".  Valid speeds are
+// currently 1.5, 3.0, 6.0, 12.0, and up.  This is a float to cover
+// the 1.5 Gbps case.  A value of 0 is returned if the speed can't be
+// parsed.
 func parseLinkrate(value string) float64 {
-	f := strings.Split(value," ")[0]
+	f := strings.Split(value, " ")[0]
 	gb, err := strconv.ParseFloat(f, 64)
 	if err != nil {
 		return 0

sysfs/class_sas_phy_test.go

@@ -17,9 +17,8 @@
 package sysfs
 
 import (
-	"testing"
-
 	"github.com/google/go-cmp/cmp"
+	"testing"
 )
 
 func TestSASPhyClass(t *testing.T) {

sysfs/class_sas_port.go

@@ -25,14 +25,24 @@ import (
 const sasPortClassPath = "class/sas_port"
 
 type SASPort struct {
-	Name       string   // /sys/class/sas_device/<Name>
-	SASPhys    []string // /sys/class/sas_device/<Name>/device/phy-*
-	Expanders  []string // /sys/class/sas_port/<Name>/device/expander-*
+	Name      string   // /sys/class/sas_device/<Name>
+	SASPhys   []string // /sys/class/sas_device/<Name>/device/phy-*
+	Expanders []string // /sys/class/sas_port/<Name>/device/expander-*
 }
 
 type SASPortClass map[string]SASPort
 
 // SASPortClass parses ports in /sys/class/sas_port.
+//
+// A SAS port in this context is a collection of SAS PHYs operating
+// together.  For example, it's common to have 8-lane SAS cards that
+// have 2 external connectors, each of which carries 4 SAS lanes over
+// a SFF-8088 or SFF-8644 connector.  While it's possible to split
+// those 4 lanes into 4 different cables wired directly into
+// individual drives, it's more common to connect them all to a SAS
+// expander.  This gives you 4x the bandwidth between the expander and
+// the SAS host, and is represented by a sas-port object which
+// contains 4 sas-phy objects.
 func (fs FS) SASPortClass() (SASPortClass, error) {
 	path := fs.sys.Path(sasPortClassPath)
 

sysfs/class_sas_port_test.go

@@ -17,9 +17,8 @@
 package sysfs
 
 import (
-	"testing"
-
 	"github.com/google/go-cmp/cmp"
+	"testing"
 )
 
 func TestSASPortClass(t *testing.T) {