pkg/specgen: refactor FinishThrottleDevices, WeightDevices

Refactor these functions to - avoid repetition of common code (mostly stat of block device path); - perform early return if nothing is to be done; - remove some excessive nesting. It also improves some error messages. This is a preparation for the next commit. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2026-02-05 15:45:08 +01:00 · 2025-06-06 15:48:29 -07:00
parent 6b8bc6f6e6
commit 9b6e548387
1 changed files with 74 additions and 67 deletions
--- a/pkg/specgen/utils_linux.go
+++ b/pkg/specgen/utils_linux.go
@@ -9,94 +9,101 @@ import (
 	"golang.org/x/sys/unix"
 )

+// statBlkDev returns path's major and minor, or an error.
+func statBlkDev(path string) (int64, int64, error) {
+	var stat unix.Stat_t
+
+	if err := unix.Stat(path, &stat); err != nil {
+		return 0, 0, fmt.Errorf("could not parse device %s: %w", path, err)
+	}
+	rdev := uint64(stat.Rdev) //nolint:unconvert // Some architectures have different type.
+	return int64(unix.Major(rdev)), int64(unix.Minor(rdev)), nil
+}
+
+// fillThrottleDev fills in dev.Major and dev.Minor fields based on path to a block device.
+func fillThrottleDev(path string, dev *spec.LinuxThrottleDevice) error {
+	major, minor, err := statBlkDev(path)
+	if err != nil {
+		return err
+	}
+
+	dev.Major, dev.Minor = major, minor
+
+	return nil
+}
+
 // FinishThrottleDevices takes the temporary representation of the throttle
 // devices in the specgen and looks up the major and major minors. it then
 // sets the throttle devices proper in the specgen
 func FinishThrottleDevices(s *SpecGenerator) error {
+	if len(s.ThrottleReadBpsDevice)+len(s.ThrottleWriteBpsDevice)+len(s.ThrottleReadIOPSDevice)+len(s.ThrottleWriteIOPSDevice) == 0 {
+		return nil
+	}
+
 	if s.ResourceLimits == nil {
 		s.ResourceLimits = &spec.LinuxResources{}
 	}
-	if bps := s.ThrottleReadBpsDevice; len(bps) > 0 {
-		if s.ResourceLimits.BlockIO == nil {
-			s.ResourceLimits.BlockIO = &spec.LinuxBlockIO{}
-		}
-		for k, v := range bps {
-			statT := unix.Stat_t{}
-			if err := unix.Stat(k, &statT); err != nil {
-				return fmt.Errorf("could not parse throttle device at %s: %w", k, err)
-			}
-			v.Major = (int64(unix.Major(uint64(statT.Rdev)))) //nolint: unconvert
-			v.Minor = (int64(unix.Minor(uint64(statT.Rdev)))) //nolint: unconvert
-			if s.ResourceLimits.BlockIO == nil {
-				s.ResourceLimits.BlockIO = new(spec.LinuxBlockIO)
-			}
-			s.ResourceLimits.BlockIO.ThrottleReadBpsDevice = append(s.ResourceLimits.BlockIO.ThrottleReadBpsDevice, v)
-		}
+	if s.ResourceLimits.BlockIO == nil {
+		s.ResourceLimits.BlockIO = &spec.LinuxBlockIO{}
 	}
-	if bps := s.ThrottleWriteBpsDevice; len(bps) > 0 {
-		if s.ResourceLimits.BlockIO == nil {
-			s.ResourceLimits.BlockIO = &spec.LinuxBlockIO{}
-		}
-		for k, v := range bps {
-			statT := unix.Stat_t{}
-			if err := unix.Stat(k, &statT); err != nil {
-				return fmt.Errorf("could not parse throttle device at %s: %w", k, err)
-			}
-			v.Major = (int64(unix.Major(uint64(statT.Rdev)))) //nolint: unconvert
-			v.Minor = (int64(unix.Minor(uint64(statT.Rdev)))) //nolint: unconvert
-			s.ResourceLimits.BlockIO.ThrottleWriteBpsDevice = append(s.ResourceLimits.BlockIO.ThrottleWriteBpsDevice, v)
+
+	for k, v := range s.ThrottleReadBpsDevice {
+		if err := fillThrottleDev(k, &v); err != nil {
+			return err
 		}
+		s.ResourceLimits.BlockIO.ThrottleReadBpsDevice = append(s.ResourceLimits.BlockIO.ThrottleReadBpsDevice, v)
 	}
-	if iops := s.ThrottleReadIOPSDevice; len(iops) > 0 {
-		if s.ResourceLimits.BlockIO == nil {
-			s.ResourceLimits.BlockIO = &spec.LinuxBlockIO{}
-		}
-		for k, v := range iops {
-			statT := unix.Stat_t{}
-			if err := unix.Stat(k, &statT); err != nil {
-				return fmt.Errorf("could not parse throttle device at %s: %w", k, err)
-			}
-			v.Major = (int64(unix.Major(uint64(statT.Rdev)))) //nolint: unconvert
-			v.Minor = (int64(unix.Minor(uint64(statT.Rdev)))) //nolint: unconvert
-			s.ResourceLimits.BlockIO.ThrottleReadIOPSDevice = append(s.ResourceLimits.BlockIO.ThrottleReadIOPSDevice, v)
+
+	for k, v := range s.ThrottleWriteBpsDevice {
+		if err := fillThrottleDev(k, &v); err != nil {
+			return err
 		}
+		s.ResourceLimits.BlockIO.ThrottleWriteBpsDevice = append(s.ResourceLimits.BlockIO.ThrottleWriteBpsDevice, v)
 	}
-	if iops := s.ThrottleWriteIOPSDevice; len(iops) > 0 {
-		if s.ResourceLimits.BlockIO == nil {
-			s.ResourceLimits.BlockIO = &spec.LinuxBlockIO{}
-		}
-		for k, v := range iops {
-			statT := unix.Stat_t{}
-			if err := unix.Stat(k, &statT); err != nil {
-				return fmt.Errorf("could not parse throttle device at %s: %w", k, err)
-			}
-			v.Major = (int64(unix.Major(uint64(statT.Rdev)))) //nolint: unconvert
-			v.Minor = (int64(unix.Minor(uint64(statT.Rdev)))) //nolint: unconvert
-			s.ResourceLimits.BlockIO.ThrottleWriteIOPSDevice = append(s.ResourceLimits.BlockIO.ThrottleWriteIOPSDevice, v)
+
+	for k, v := range s.ThrottleReadIOPSDevice {
+		if err := fillThrottleDev(k, &v); err != nil {
+			return err
 		}
+		s.ResourceLimits.BlockIO.ThrottleReadIOPSDevice = append(s.ResourceLimits.BlockIO.ThrottleReadIOPSDevice, v)
 	}
+
+	for k, v := range s.ThrottleWriteIOPSDevice {
+		if err := fillThrottleDev(k, &v); err != nil {
+			return err
+		}
+		s.ResourceLimits.BlockIO.ThrottleWriteIOPSDevice = append(s.ResourceLimits.BlockIO.ThrottleWriteIOPSDevice, v)
+	}
+
 	return nil
 }

 func WeightDevices(specgen *SpecGenerator) error {
-	devs := []spec.LinuxWeightDevice{}
+	if len(specgen.WeightDevice) == 0 {
+		return nil
+	}
+
 	if specgen.ResourceLimits == nil {
 		specgen.ResourceLimits = &spec.LinuxResources{}
 	}
-	for k, v := range specgen.WeightDevice {
-		statT := unix.Stat_t{}
-		if err := unix.Stat(k, &statT); err != nil {
-			return fmt.Errorf("failed to inspect '%s' in --blkio-weight-device: %w", k, err)
-		}
-		dev := new(spec.LinuxWeightDevice)
-		dev.Major = (int64(unix.Major(uint64(statT.Rdev)))) //nolint: unconvert
-		dev.Minor = (int64(unix.Minor(uint64(statT.Rdev)))) //nolint: unconvert
-		dev.Weight = v.Weight
-		devs = append(devs, *dev)
-		if specgen.ResourceLimits.BlockIO == nil {
-			specgen.ResourceLimits.BlockIO = &spec.LinuxBlockIO{}
-		}
-		specgen.ResourceLimits.BlockIO.WeightDevice = devs
+	if specgen.ResourceLimits.BlockIO == nil {
+		specgen.ResourceLimits.BlockIO = &spec.LinuxBlockIO{}
 	}
+
+	for k, v := range specgen.WeightDevice {
+		major, minor, err := statBlkDev(k)
+		if err != nil {
+			return fmt.Errorf("bad --blkio-weight-device: %w", err)
+		}
+		specgen.ResourceLimits.BlockIO.WeightDevice = append(specgen.ResourceLimits.BlockIO.WeightDevice,
+			spec.LinuxWeightDevice{
+				LinuxBlockIODevice: spec.LinuxBlockIODevice{
+					Major: major,
+					Minor: minor,
+				},
+				Weight: v.Weight,
+			})
+	}
+
 	return nil
 }