HighPerformanceHooks: Defer irqSMPAffinityFile rollback

andreaskaris · andreaskaris · commit 53ea4f4602c1 · 2025-10-01T13:16:08.000+02:00
Signed-off-by: Andreas Karis <ak.karis@gmail.com> (cherry picked from commit 78c966c)
diff --git a/internal/runtimehandlerhooks/high_performance_hooks_linux.go b/internal/runtimehandlerhooks/high_performance_hooks_linux.go
@@ -667,7 +667,7 @@ func (h *HighPerformanceHooks) handleIRQBalanceOneShot(ctx context.Context, cNam
 }
 
 // updateNewIRQSMPAffinityMask updates SMP IRQ affinity and IRQ balance configuration files.
-func (h *HighPerformanceHooks) updateNewIRQSMPAffinityMask(ctx context.Context, cID, cName, cpus string, enable bool) (string, error) {
+func (h *HighPerformanceHooks) updateNewIRQSMPAffinityMask(ctx context.Context, cID, cName, cpus string, enable bool) (newIRQBalanceSetting string, err error) {
 	content, err := os.ReadFile(h.irqSMPAffinityFile)
 	if err != nil {
 		return "", err
@@ -690,17 +690,22 @@ func (h *HighPerformanceHooks) updateNewIRQSMPAffinityMask(ctx context.Context,
 		return "", err
 	}
 
+	// Rollback IRQ SMP affinity file to maintain consistency if something goes wrong.
+	defer func() {
+		if err != nil {
+			if rollbackErr := os.WriteFile(h.irqSMPAffinityFile, []byte(originalIRQSMPSetting), 0o644); rollbackErr != nil {
+				log.Errorf(ctx, "Failed to rollback IRQ SMP affinity file after config update failure: err: %q, rollback err: %q",
+					err, rollbackErr)
+			}
+		}
+	}()
+
 	// Nothing else to do if irq balance config file does not exist.
 	if !fileExists(h.irqBalanceConfigFile) {
 		return newIRQBalanceSetting, nil
 	}
 
 	if err := updateIrqBalanceConfigFile(h.irqBalanceConfigFile, newIRQBalanceSetting); err != nil {
-		// Rollback IRQ SMP affinity file to maintain consistency
-		if rollbackErr := os.WriteFile(h.irqSMPAffinityFile, []byte(originalIRQSMPSetting), 0o644); rollbackErr != nil {
-			log.Errorf(ctx, "Failed to rollback IRQ SMP affinity file after config update failure: %v", rollbackErr)
-		}
-
 		return "", err
 	}
 
diff --git a/internal/runtimehandlerhooks/high_performance_hooks_test.go b/internal/runtimehandlerhooks/high_performance_hooks_test.go
@@ -813,6 +813,60 @@ var _ = Describe("high_performance_hooks", func() {
 		)
 	})
 
+	Describe("updateNewIRQSMPAffinityMask rollback", func() {
+		irqBalanceConfigFile := filepath.Join(fixturesDir, "irqbalance")
+		irqSMPAffinityFile := filepath.Join(fixturesDir, "irqsmpaffinity")
+
+		h := &HighPerformanceHooks{
+			irqSMPAffinityFile:   irqSMPAffinityFile,
+			irqBalanceConfigFile: irqBalanceConfigFile,
+		}
+
+		type parameters struct {
+			irqBalanceFileRO           bool
+			originalIRQSMPAffinityMask string
+			expectedIRQSMPAffinityMask string
+		}
+
+		DescribeTable("test rollback",
+			func(p parameters) {
+				err := os.WriteFile(irqSMPAffinityFile, []byte(p.originalIRQSMPAffinityMask), 0o644)
+				Expect(err).ToNot(HaveOccurred())
+
+				if p.irqBalanceFileRO {
+					err = os.Symlink("/proc/version", irqBalanceConfigFile)
+					Expect(err).ToNot(HaveOccurred())
+				} else {
+					err = os.WriteFile(irqBalanceConfigFile, []byte(""), 0o644)
+					Expect(err).ToNot(HaveOccurred())
+				}
+
+				_, err = h.updateNewIRQSMPAffinityMask(context.TODO(), "cID", "CName", "2-3", false)
+				if p.irqBalanceFileRO {
+					Expect(err).To(HaveOccurred())
+				} else {
+					Expect(err).NotTo(HaveOccurred())
+				}
+
+				writtenMask, err := os.ReadFile(irqSMPAffinityFile)
+				Expect(err).NotTo(HaveOccurred())
+				Expect(string(writtenMask)).To(Equal(p.expectedIRQSMPAffinityMask))
+			},
+			Entry("writing IRQ balance file fails",
+				parameters{
+					irqBalanceFileRO:           true,
+					originalIRQSMPAffinityMask: "ffffffff",
+					expectedIRQSMPAffinityMask: "ffffffff",
+				}),
+			Entry("writing IRQ balance file succeeds",
+				parameters{
+					irqBalanceFileRO:           false,
+					originalIRQSMPAffinityMask: "ffffffff",
+					expectedIRQSMPAffinityMask: "fffffff3",
+				}),
+		)
+	})
+
 	Describe("convertAnnotationToLatency", func() {
 		verifyConvertAnnotationToLatency := func(annotation string, expected string, expect_error bool) {
 			latency, err := convertAnnotationToLatency(annotation)
