From ef9502a00922809501a160e29d3475532510188f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=B6rg=20Thalheim?= <joerg@thalheim.io>
Date: Wed, 16 Oct 2024 19:44:43 +0300
Subject: [PATCH] nixos/test-driver: fix resource cleanup of vlan/qmp objects

Using __del__ is somewhat unsound resource cleanup in our clase the
logger already closed its logfile and therefor fails with exception
before the rest of the resources can be cleaned up.
---
 nixos/lib/test-driver/test_driver/driver.py  | 11 ++++++++++-
 nixos/lib/test-driver/test_driver/machine.py |  3 +++
 nixos/lib/test-driver/test_driver/qmp.py     |  2 +-
 nixos/lib/test-driver/test_driver/vlan.py    |  2 +-
 4 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/nixos/lib/test-driver/test_driver/driver.py b/nixos/lib/test-driver/test_driver/driver.py
index 01b64b92e977..0f01bd6d0ab4 100644
--- a/nixos/lib/test-driver/test_driver/driver.py
+++ b/nixos/lib/test-driver/test_driver/driver.py
@@ -99,7 +99,16 @@ class Driver:
         with self.logger.nested("cleanup"):
             self.race_timer.cancel()
             for machine in self.machines:
-                machine.release()
+                try:
+                    machine.release()
+                except Exception as e:
+                    self.logger.error(f"Error during cleanup of {machine.name}: {e}")
+
+            for vlan in self.vlans:
+                try:
+                    vlan.stop()
+                except Exception as e:
+                    self.logger.error(f"Error during cleanup of vlan{vlan.nr}: {e}")
 
     def subtest(self, name: str) -> Iterator[None]:
         """Group logs under a given test name"""
diff --git a/nixos/lib/test-driver/test_driver/machine.py b/nixos/lib/test-driver/test_driver/machine.py
index 3a1d5bc1be76..7a602ce6608f 100644
--- a/nixos/lib/test-driver/test_driver/machine.py
+++ b/nixos/lib/test-driver/test_driver/machine.py
@@ -1234,6 +1234,9 @@ class Machine:
         self.monitor.close()
         self.serial_thread.join()
 
+        if self.qmp_client:
+            self.qmp_client.close()
+
     def run_callbacks(self) -> None:
         for callback in self.callbacks:
             callback()
diff --git a/nixos/lib/test-driver/test_driver/qmp.py b/nixos/lib/test-driver/test_driver/qmp.py
index 62ca6d7d5b80..99c02ca1c120 100644
--- a/nixos/lib/test-driver/test_driver/qmp.py
+++ b/nixos/lib/test-driver/test_driver/qmp.py
@@ -49,7 +49,7 @@ class QMPSession:
         sock.connect(str(path))
         return cls(sock)
 
-    def __del__(self) -> None:
+    def close(self) -> None:
         self.sock.close()
 
     def _wait_for_new_result(self) -> dict[str, str]:
diff --git a/nixos/lib/test-driver/test_driver/vlan.py b/nixos/lib/test-driver/test_driver/vlan.py
index 9340fc92ed4c..03ecbe25e8a2 100644
--- a/nixos/lib/test-driver/test_driver/vlan.py
+++ b/nixos/lib/test-driver/test_driver/vlan.py
@@ -59,7 +59,7 @@ class VLan:
 
         self.logger.info(f"running vlan (pid {self.pid}; ctl {self.socket_dir})")
 
-    def __del__(self) -> None:
+    def stop(self) -> None:
         self.logger.info(f"kill vlan (pid {self.pid})")
         self.fd.close()
         self.process.terminate()