From 30ea702efa54b6fe9901bc468d229db3692382bd Mon Sep 17 00:00:00 2001
From: Mathieu Labourier <mathieu.labourier@vates.tech>
Date: Thu, 21 Aug 2025 16:09:58 +0200
Subject: [PATCH] fix: stop SMGC service on SR detach to prevent orphaned
 systemd units

Signed-off-by: Mathieu Labourier <mathieu.labourier@vates.tech>
---
 libs/sm/cleanup.py    | 30 ++++++++++++++++++++++++------
 tests/test_cleanup.py |  2 +-
 2 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/libs/sm/cleanup.py b/libs/sm/cleanup.py
index 850086acd..afae903bf 100644
--- a/libs/sm/cleanup.py
+++ b/libs/sm/cleanup.py
@@ -3205,6 +3205,7 @@ def abort(srUuid, soft=False):
     """Abort GC/coalesce if we are currently GC'ing or coalescing a VDI pair.
     """
     if _abort(srUuid, soft):
+        stop_gc_service(srUuid)
         Util.log("abort: releasing the process lock")
         lockGCActive.release()
         return True
@@ -3278,6 +3279,18 @@ def start_gc(session, sr_uuid):
     subprocess.run([__file__, '-b', '-u', sr_uuid, '-g'],
                    stdout=subprocess.PIPE, stderr=subprocess.PIPE, close_fds=True)
 
+def _gc_service_cmd(sr_uuid, action, extra_args=None):
+    """
+    Build and run the systemctl command for the GC service using util.doexec.
+    """
+    sr_uuid_esc = sr_uuid.replace("-", "\\x2d")
+    cmd=["/usr/bin/systemctl", "--quiet"]
+    if extra_args:
+        cmd.extend(extra_args)
+    cmd += [action, f"SMGC@{sr_uuid_esc}"]
+    return util.doexec(cmd)
+
+
 def start_gc_service(sr_uuid, wait=False):
     """
     This starts the templated systemd service which runs GC on the given SR UUID.
@@ -3288,13 +3301,18 @@ def start_gc_service(sr_uuid, wait=False):
     run has finished. This is used to force a run of the GC instead of just kicking it
     in the background.
     """
-    sr_uuid_esc = sr_uuid.replace("-", "\\x2d")
     util.SMlog(f"Kicking SMGC@{sr_uuid}...")
-    cmd=[ "/usr/bin/systemctl", "--quiet" ]
-    if not wait:
-        cmd.append("--no-block")
-    cmd += ["start", f"SMGC@{sr_uuid_esc}"]
-    subprocess.run(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, close_fds=True)
+    _gc_service_cmd(sr_uuid, "start", extra_args=None if wait else ["--no-block"])
+
+
+def stop_gc_service(sr_uuid):
+    """
+    Stops the templated systemd service which runs GC on the given SR UUID.
+    """
+    util.SMlog(f"Stopping SMGC@{sr_uuid}...")
+    (rc, _stdout, stderr) = _gc_service_cmd(sr_uuid, "stop")
+    if rc != 0:
+        util.SMlog(f"Failed to stop gc service `SMGC@{sr_uuid}`: `{stderr}`")
 
 
 def gc_force(session, srUuid, force=False, dryRun=False, lockSR=False):
diff --git a/tests/test_cleanup.py b/tests/test_cleanup.py
index e66f7ed9d..e6952a8e1 100644
--- a/tests/test_cleanup.py
+++ b/tests/test_cleanup.py
@@ -398,7 +398,7 @@ def test_lock_released_by_abort_when_held(
         cleanup.lockGCActive = TestRelease()
         cleanup.lockGCActive.release = mock.Mock(return_value=None)
 
-        ret = cleanup.abort(mock_sr, False)
+        ret = cleanup.abort(str(mock_sr.uuid), False)
 
         # Pass on the return from _abort.
         self.assertEqual(True, ret)