diff --git a/drivers/LVHDSR.py b/drivers/LVHDSR.py index 21441e365..f15c92d8f 100755 --- a/drivers/LVHDSR.py +++ b/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): diff --git a/drivers/util.py b/drivers/util.py index f6b9ba8d8..8fa80da4a 100755 --- a/drivers/util.py +++ b/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]}") diff --git a/tests/test_LVHDSR.py b/tests/test_LVHDSR.py index da786b2d1..94a390d3f 100644 --- a/tests/test_LVHDSR.py +++ b/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)