CA-392823: ensure no device mapper conflicts in LVHDSR detach

drivers/LVHDSR.py

@@ -51,6 +51,7 @@
 from xmlrpc.client import DateTime
 import glob
 from constants import CBTLOG_TAG
+from fairlock import Fairlock
 DEV_MAPPER_ROOT = os.path.join('/dev/mapper', lvhdutil.VG_PREFIX)
 
 geneology = {}
@@ -601,13 +602,15 @@ def detach(self, uuid):
             if util.extractSRFromDevMapper(fileName) != self.uuid:
                 continue
 
-            #   check if any file has open handles
-            if util.doesFileHaveOpenHandles(fileName):
-                #   if yes, log this and signal failure
-                util.SMlog("LVHDSR.detach: The dev mapper entry %s has open " \
-                           "handles" % fileName)
-                success = False
-                continue
+            with Fairlock('devicemapper'):
+                # check if any file has open handles
+                if util.doesFileHaveOpenHandles(fileName):
+                    # if yes, log this and signal failure
+                    util.SMlog(
+                        f"LVHDSR.detach: The dev mapper entry {fileName} has "
+                        "open handles")
+                    success = False
+                    continue
 
             # Now attempt to remove the dev mapper entry
             if not lvutil.removeDevMapperEntry(fileName, False):

drivers/util.py

@@ -1496,7 +1496,7 @@ def findRunningProcessOrOpenFile(name, process=True):
         # There is no specific guarantee of when a PID's /proc directory will disappear
         # when it exits, particularly relative to filedescriptor cleanup, so we want to
         # make sure we're not reporting a false positive.
-        processandpids = [ x for x in processandpids if pid_is_alive(x[1]) ]
+        processandpids = [x for x in processandpids if pid_is_alive(int(x[1]))]
         for pp in processandpids:
             SMlog(f"File {name} has an open handle with process {pp[0]} with pid {pp[1]}")
 

tests/test_LVHDSR.py

@@ -104,7 +104,7 @@ def test_undoAllInflateJournals(
     @mock.patch('LVHDSR.Lock', autospec=True)
     @mock.patch('SR.XenAPI')
     @testlib.with_context
-    def test_attach_success(self,
+    def test_srlifecycle_success(self,
                             context,
                             mock_xenapi,
                             mock_lock,
@@ -196,7 +196,35 @@ def get_vdi_by_uuid(vdi_uuid):
         # Arrange (2)
         sr = self.create_LVHDSR(master=True, command='sr_detach',
                                 sr_uuid=sr_uuid)
+
+        # Arrange for detach
+        self.stubout('LVHDSR.Fairlock')
+        mock_remove_device = self.stubout(
+            'LVHDSR.lvutil.removeDevMapperEntry')
+        mock_glob = self.stubout('glob.glob')
+        mock_vdi_uuid = "72101dbd-bd62-4a14-a03c-afca8cceec86"
+        mock_filepath = os.path.join(
+            '/dev/mapper/', 'VG_XenStorage'
+            f'--{sr_uuid.replace("-", "--")}-'
+            f'{mock_vdi_uuid.replace("-", "--")}')
+        mock_glob.return_value = [mock_filepath]
+        mock_open_handles = self.stubout(
+            'LVHDSR.util.doesFileHaveOpenHandles')
+
+        # Act (Detach)
+        with self.assertRaises(Exception):
+            # Fail the first one with busy handles
+            mock_open_handles.return_value = True
+            sr.detach(sr.uuid)
+
+        # Now succeed
+        mock_open_handles.return_value = False
         sr.detach(sr.uuid)
+
+        # Assert for detach
+        mock_remove_device.assert_called_once_with(mock_filepath, False)
+
+        # Create new SR
         mock_lvm_cache.return_value.checkLV.return_value = True
         sr = self.create_LVHDSR(master=True, command='sr_attach',
                                 sr_uuid=sr_uuid)