Skip to content

Commit

Permalink
CA-392823: ensure no device mapper conflicts in LVHDSR detach
Browse files Browse the repository at this point in the history
Within the detach operation checks are made that none of the device
mapper devices for the SR volume group have opern handles. The falls
false if operations on other LVM srs happen to call a device mapper
operation such as `lvs` or `vgs` at the point the check is made as
these operations can result in all of the devices managed by the
device mapper subsystem being accessed as part of the scan. this then
gives a false response for the open file handles check.

Signed-off-by: Mark Syms <mark.syms@cloud.com>
  • Loading branch information
MarkSymsCtx committed May 13, 2024
1 parent 0ac8e84 commit 54e17e9
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 8 deletions.
17 changes: 10 additions & 7 deletions drivers/LVHDSR.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {}
Expand Down Expand Up @@ -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):
Expand Down
30 changes: 29 additions & 1 deletion tests/test_LVHDSR.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 54e17e9

Please sign in to comment.