diff --git a/libs/sm/cleanup.py b/libs/sm/cleanup.py
index 850086acd..c248598f3 100644
--- a/libs/sm/cleanup.py
+++ b/libs/sm/cleanup.py
@@ -2316,7 +2316,6 @@ def _doCoalesceLeaf(self, vdi):
# garbage
# update the VDI record
- vdi.parent.delConfig(VDI.DB_VHD_PARENT)
if vdi.parent.raw:
vdi.parent.setConfig(VDI.DB_VDI_TYPE, vhdutil.VDI_TYPE_RAW)
vdi.parent.delConfig(VDI.DB_VHD_BLOCKS)
@@ -2331,6 +2330,9 @@ def _doCoalesceLeaf(self, vdi):
vdi.parent.children = []
vdi.parent = None
+ if parent.parent is None:
+ parent.delConfig(VDI.DB_VHD_PARENT)
+
extraSpace = self._calcExtraSpaceNeeded(vdi, parent)
freeSpace = self.getFreeSpace()
if freeSpace < extraSpace:
@@ -2633,10 +2635,12 @@ def _finishInterruptedCoalesceLeaf(self, childUuid, parentUuid):
Util.log("*** FINISH LEAF-COALESCE")
vdi = self.getVDI(childUuid)
if not vdi:
+ Util.log(f"_finishInterruptedCoalesceLeaf, vdi {childUuid} not found, aborting")
raise util.SMException("VDI %s not found" % childUuid)
try:
self.forgetVDI(parentUuid)
except XenAPI.Failure:
+ Util.logException('_finishInterruptedCoalesceLeaf')
pass
self._updateSlavesOnResize(vdi)
util.fistpoint.activate("LVHDRT_coaleaf_finish_end", self.uuid)
diff --git a/libs/sm/core/XE_SR_ERRORCODES.xml b/libs/sm/core/XE_SR_ERRORCODES.xml
index 47fefd830..792fe17fc 100644
--- a/libs/sm/core/XE_SR_ERRORCODES.xml
+++ b/libs/sm/core/XE_SR_ERRORCODES.xml
@@ -513,6 +513,12 @@
117
+
+ PVMultiIDs
+ PVs found with multiple SCSI IDs
+ 119
+
+
APISession
diff --git a/libs/sm/drivers/LVHDoISCSISR.py b/libs/sm/drivers/LVHDoISCSISR.py
index eb13f8cbe..0823cf28b 100644
--- a/libs/sm/drivers/LVHDoISCSISR.py
+++ b/libs/sm/drivers/LVHDoISCSISR.py
@@ -471,11 +471,21 @@ def attach(self, sr_uuid):
scsiutil.rescan([self.iscsi.adapter[a]])
self._pathrefresh(LVHDoISCSISR)
+
+ # Check that we only have PVs for the volume group with the expected SCSI ID
+ lvutil.checkPVScsiIds(self.vgname, self.SCSIid)
+
LVHDSR.LVHDSR.attach(self, sr_uuid)
except Exception as inst:
for i in self.iscsiSRs:
i.detach(sr_uuid)
+
+ # If we already have a proper error just raise it
+ if isinstance(inst, xs_errors.SROSError):
+ raise
+
raise xs_errors.XenError("SRUnavailable", opterr=inst)
+
self._setMultipathableFlag(SCSIid=self.SCSIid)
def detach(self, sr_uuid):
diff --git a/libs/sm/lvutil.py b/libs/sm/lvutil.py
index d1e7eb976..2566f9397 100644
--- a/libs/sm/lvutil.py
+++ b/libs/sm/lvutil.py
@@ -23,6 +23,7 @@
import time
from fairlock import Fairlock
+from sm.core import scsiutil
from sm.core import util
from sm.core import xs_errors
import xml.dom.minidom
@@ -613,6 +614,19 @@ def setActiveVG(path, active):
text = cmd_lvm([CMD_VGCHANGE, "-a" + val, path])
+def checkPVScsiIds(vgname, SCSIid):
+ # Get all the PVs for the specified vgName even if not active
+ cmd = [CMD_PVS, '-a', '--select', f'vgname={vgname}', '--no-headings']
+ text = cmd_lvm(cmd)
+ pv_paths = [x.split()[0] for x in text.splitlines()]
+ for pv_path in pv_paths:
+ pv_scsi_id = scsiutil.getSCSIid(pv_path)
+ if pv_scsi_id != SCSIid:
+ raise xs_errors.XenError(
+ 'PVMultiIDs',
+ opterr=f'Found PVs {",".join(pv_paths)} and unexpected '
+ f'SCSI ID {pv_scsi_id}, expected {SCSIid}')
+
@lvmretry
def create(name, size, vgname, tag=None, size_in_percentage=None):
if size_in_percentage:
@@ -728,13 +742,6 @@ def deactivateNoRefcount(path):
@lvmretry
def _deactivate(path):
- # Records what is using the LVM path in case there is an issue.
- # In most cases this should be nothing.
- try:
- (rc, stdout, stderr) = util.doexec(['/usr/sbin/fuser', "-v", path])
- util.SMlog(f"fuser {path} => {rc} / '{stdout}' / '{stderr}'")
- except:
- pass
text = cmd_lvm([CMD_LVCHANGE, "-an", path])
diff --git a/tests/test_LVHDoISCSISR.py b/tests/test_LVHDoISCSISR.py
index 8fa7f8647..4082317cb 100644
--- a/tests/test_LVHDoISCSISR.py
+++ b/tests/test_LVHDoISCSISR.py
@@ -159,6 +159,9 @@ def deepcopy(to_copy):
lvlock_patcher = mock.patch('sm.drivers.LVHDSR.lvutil.Fairlock')
self.mock_lvlock = lvlock_patcher.start()
+ pv_check_patcher = mock.patch('sm.drivers.LVHDoISCSISR.lvutil.checkPVScsiIds', autospec=True)
+ self.mock_pv_check = pv_check_patcher.start()
+
self.addCleanup(mock.patch.stopall)
super().setUp()
diff --git a/tests/test_cleanup.py b/tests/test_cleanup.py
index e66f7ed9d..4fde15e6b 100644
--- a/tests/test_cleanup.py
+++ b/tests/test_cleanup.py
@@ -7,7 +7,9 @@
from uuid import uuid4
from sm import cleanup
+from sm import fjournaler
from sm.core import lock
+from sm.core.util import SMException
from sm.core import util
from sm import vhdutil
@@ -15,6 +17,9 @@
from sm import ipc
import XenAPI
+from XenAPI import Failure
+
+MEGA = 1024 * 1024
class FakeFile(object):
@@ -1815,6 +1820,46 @@ def test_not_plugged_retry(self):
# Assert
self.assertIsNotNone(sr)
+ def test_doCoalesceLeaf_no_parent_after(self):
+ sr = create_cleanup_sr(self.xapi_mock)
+ sr.journaler = mock.create_autospec(fjournaler.Journaler)
+ parent_uuid = str(uuid.uuid4())
+ mock_parent = mock.MagicMock()
+ mock_parent.uuid = parent_uuid
+ mock_parent.parent = None
+ mock_parent.raw = False
+ mock_parent.getSizeVHD.return_value = 10 * MEGA
+
+ vdi_uuid = str(uuid.uuid4())
+ mock_vdi = mock.MagicMock()
+ mock_vdi.uuid = vdi_uuid
+ mock_vdi.getSizeVHD.return_value = 10 * MEGA
+ mock_vdi.parent = mock_parent
+
+ sr._doCoalesceLeaf(mock_vdi)
+
+ mock_parent.delConfig.assert_called_with("vhd-parent")
+
+ def test_doCoalesceLeaf_parent_remains_after(self):
+ sr = create_cleanup_sr(self.xapi_mock)
+ sr.journaler = mock.create_autospec(fjournaler.Journaler)
+ parent_uuid = str(uuid.uuid4())
+ mock_parent = mock.MagicMock()
+ mock_parent.uuid = parent_uuid
+ mock_parent.parent = mock.MagicMock()
+ mock_parent.raw = False
+ mock_parent.getSizeVHD.return_value = 10 * MEGA
+
+ vdi_uuid = str(uuid.uuid4())
+ mock_vdi = mock.MagicMock()
+ mock_vdi.uuid = vdi_uuid
+ mock_vdi.getSizeVHD.return_value = 10 * MEGA
+ mock_vdi.parent = mock_parent
+
+ sr._doCoalesceLeaf(mock_vdi)
+
+ self.assertNotIn('vhd-parent', mock_parent.delConfig.call_args)
+
class TestLockGCActive(unittest.TestCase):
@@ -1919,3 +1964,46 @@ def test_cannot_acquire_if_other_process_holds_sr_lock(self):
# When
with self.assertRaises(BlockingIOError):
gcLock.acquireNoblock()
+
+
+class TestFileSR(TestSR):
+ """
+ Exercise the specfic bits of the FileSR support
+ """
+
+ def setUp(self):
+ super().setUp()
+
+ self.sr_uuid = str(uuid.uuid4())
+
+ def _make_test_sr(self):
+ self.mock_sr = cleanup.FileSR(self.sr_uuid, self.xapi_mock,
+ createLock=False, force=False)
+
+ def test_finishInterruptedCoalesceLeaf_no_vdi(self):
+ self._make_test_sr()
+ self.mock_sr.vdis = {}
+
+ child_vdi_uuid = str(uuid.uuid4())
+ parent_vdi_uuid = str(uuid.uuid4())
+
+ with self.assertRaises(SMException) as sme:
+ self.mock_sr._finishInterruptedCoalesceLeaf(child_vdi_uuid, parent_vdi_uuid)
+
+ self.assertRegex(str(sme.exception), r"VDI \S+ not found")
+
+ def test_finishInterruptedCoalesceLeaf_forget_fail_and_complete(self):
+ child_vdi_uuid = str(uuid.uuid4())
+ parent_vdi_uuid = str(uuid.uuid4())
+
+ self._make_test_sr()
+ self.mock_sr.vdis = {
+ child_vdi_uuid: mock.create_autospec(cleanup.FileVDI)
+ }
+
+ def forgetVdi(self, uuid):
+ raise Failure(f"ForgetVDI {uuid} failed")
+
+ self.xapi_mock.forgetVDI.side_effect = forgetVdi
+
+ self.mock_sr._finishInterruptedCoalesceLeaf(child_vdi_uuid, parent_vdi_uuid)
diff --git a/tests/test_lvutil.py b/tests/test_lvutil.py
index 43b3b7127..176dd3c93 100644
--- a/tests/test_lvutil.py
+++ b/tests/test_lvutil.py
@@ -6,6 +6,7 @@
import testlib
import lvmlib
from sm.core import util
+from sm.core import xs_errors
from sm import lvutil
@@ -394,6 +395,27 @@ def test_command_error(self, mock_smlog, mock_cmd_lvm):
])
mock_smlog.assert_called_with("PVs in VG vg1: []")
+ @mock.patch('sm.core.xs_errors.XML_DEFS', 'libs/sm/core/XE_SR_ERRORCODES.xml')
+ @mock.patch('sm.lvutil.scsiutil.getSCSIid', autospec=True)
+ def test_check_PV_SCSI_IDs_failure(self, mock_get_scsi_id, mock_smlog, mock_cmd_lvm):
+ mock_cmd_lvm.return_value = """ /dev/disk/by-id/scsi-360014057b7f26e2962843fa8a0645fbd VG_XenStorage-401d198b-60ab-1f21-1359-bd4f127b8f38 lvm2 d-- 0 0
+ /dev/disk/by-id/scsi-36001405fdd436fa7f854cd685fc3b1fd VG_XenStorage-401d198b-60ab-1f21-1359-bd4f127b8f38 lvm2 a-- <49.99g 49.98g
+""" # noqa: E501
+ mock_get_scsi_id.side_effect = ['36001405fdd436fa7f854cd685fc3b1fd',
+ '360014057b7f26e2962843fa8a0645fbd']
+ with self.assertRaises(xs_errors.SROSError) as srose:
+ lvutil.checkPVScsiIds('VG_XenStorage-401d198b-60ab-1f21-1359-bd4f127b8f38',
+ '36001405fdd436fa7f854cd685fc3b1fd')
+ self.assertEqual(119, srose.exception.errno)
+
+ @mock.patch('sm.lvutil.scsiutil.getSCSIid', autospec=True)
+ def test_check_PV_SCSI_IDs_success(self, mock_get_scsi_id, mock_smlog, mock_cmd_lvm):
+ mock_cmd_lvm.return_value = """ /dev/disk/by-id/scsi-360014057b7f26e2962843fa8a0645fbd VG_XenStorage-401d198b-60ab-1f21-1359-bd4f127b8f38 lvm2 d-- 0 0""" # noqa: E501
+ mock_get_scsi_id.side_effect = ['36001405fdd436fa7f854cd685fc3b1fd']
+ lvutil.checkPVScsiIds('VG_XenStorage-401d198b-60ab-1f21-1359-bd4f127b8f38',
+ '36001405fdd436fa7f854cd685fc3b1fd')
+
+
@mock.patch('sm.lvutil.cmd_lvm')
@mock.patch('sm.core.util.SMlog', autospec=True)
class TestGetPVsWithUUID(unittest.TestCase):
@@ -431,3 +453,4 @@ def test_command_error(self, mock_smlog, mock_cmd_lvm):
mock.call("Warning: Invalid or empty line in pvs output: Invalid return value."),
mock.call("PVs with uuid uuid1: []")
])
+