Stop SMGC service on SR detach to prevent orphaned systemd units (#92)

This is not done on every and each implementation of SR but only on ones that calls cleanup.start_gc_service (like FileSR)
and on the classes that inherits from them and don't call super on detach.

This is to prevent useless errors logs like Failed to stop xxx.service: Unit xxx.service not loaded.

Signed-off-by: Mathieu Labourier <[email protected]>

Author: Millefeuille42

drivers/cleanup.py

@@ -3948,6 +3948,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
@@ -4021,6 +4022,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.
@@ -4031,14 +4044,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):
     """Garbage collect all deleted VDIs in SR "srUuid". The caller must ensure

tests/test_cleanup.py

@@ -407,8 +407,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)