From bc277474487b97053c3d9cc4d898d8ac1f914759 Mon Sep 17 00:00:00 2001 From: Mark Syms Date: Mon, 7 Oct 2024 15:56:47 +0100 Subject: [PATCH] CA-400106: disable leaf coalesce with VDI snapshot secondary When the secondary/mirror option is provided to the vdi_snasphot operation we do not want any leaf coalesce garbage collection taking place. Unfortunately xapi will delete the snapshot once it's finished copying the new parent to the destination SR in SXM and whilst the mirroring operation is still live. This will kick the GC process and if the GC gets to the point of performing the leaf coalesce before the migrate completes will result in a failure of the mirror operation. So block the leaf-coalesce operation in this case. The block will be removed if another snapshot were to be taken but in the case of SXM the next expected operation is a vdi_delete of the leaf which will clear the entire chain. Signed-off-by: Mark Syms --- drivers/FileSR.py | 3 ++- drivers/LVHDSR.py | 1 + drivers/VDI.py | 13 +++++++++++++ tests/test_LVHDSR.py | 44 ++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 60 insertions(+), 1 deletion(-) diff --git a/drivers/FileSR.py b/drivers/FileSR.py index ba9b18630..b84d9eae3 100755 --- a/drivers/FileSR.py +++ b/drivers/FileSR.py @@ -707,7 +707,7 @@ def reset_leaf(self, sr_uuid, vdi_uuid): vhdutil.killData(self.path) def _do_snapshot(self, sr_uuid, vdi_uuid, snap_type, - secondary=None, cbtlog=None): + _=False, secondary=None, cbtlog=None): # If cbt enabled, save file consistency state if cbtlog is not None: if blktap2.VDI.tap_status(self.session, vdi_uuid): @@ -727,6 +727,7 @@ def _do_snapshot(self, sr_uuid, vdi_uuid, snap_type, try: return self._snapshot(snap_type, cbtlog, consistency_state) finally: + self.disable_leaf_on_secondary(vdi_uuid, secondary=secondary) blktap2.VDI.tap_unpause(self.session, sr_uuid, vdi_uuid, secondary) def _rename(self, src, dst): diff --git a/drivers/LVHDSR.py b/drivers/LVHDSR.py index dedaef957..19175a7eb 100755 --- a/drivers/LVHDSR.py +++ b/drivers/LVHDSR.py @@ -1656,6 +1656,7 @@ def _do_snapshot(self, sr_uuid, vdi_uuid, snapType, util.SMlog('WARNING: failed to clean up failed snapshot: ' '%s (error ignored)' % e2) raise + self.disable_leaf_on_secondary(vdi_uuid, secondary=secondary) blktap2.VDI.tap_unpause(self.session, sr_uuid, vdi_uuid, secondary) unpause_time = time.time() if (unpause_time - pause_time) > LONG_SNAPTIME: diff --git a/drivers/VDI.py b/drivers/VDI.py index bc2ae5f2a..ac83ea175 100755 --- a/drivers/VDI.py +++ b/drivers/VDI.py @@ -16,6 +16,7 @@ # VDI: Base class for virtual disk instances # +import cleanup import SR import xmlrpc.client import xs_errors @@ -28,6 +29,7 @@ from bitarray import bitarray import uuid + SM_CONFIG_PASS_THROUGH_FIELDS = ["base_mirror", "key_hash"] SNAPSHOT_SINGLE = 1 # true snapshot: 1 leaf, 1 read-only parent @@ -902,3 +904,14 @@ def _disable_cbt_on_error(self, alert_name, alert_str): alert_prio_warning, alert_obj, alert_uuid, alert_str) + + def disable_leaf_on_secondary(self, vdi_uuid, secondary=None): + vdi_ref = self.session.xenapi.VDI.get_by_uuid(vdi_uuid) + self.session.xenapi.VDI.remove_from_other_config( + vdi_ref, cleanup.VDI.DB_LEAFCLSC) + if secondary is not None: + util.SMlog(f"We have secondary for {vdi_uuid}, " + "blocking leaf coalesce") + self.session.xenapi.VDI.add_to_other_config( + vdi_ref, cleanup.VDI.DB_LEAFCLSC, + cleanup.VDI.LEAFCLSC_DISABLED) diff --git a/tests/test_LVHDSR.py b/tests/test_LVHDSR.py index 94a390d3f..3e8b71fc9 100644 --- a/tests/test_LVHDSR.py +++ b/tests/test_LVHDSR.py @@ -4,6 +4,7 @@ import uuid +import cleanup import LVHDSR import lvhdutil import lvutil @@ -423,3 +424,46 @@ def test_snapshot_attached_cbt_success(self, mock_xenapi, mock_lock): # Assert self.assertIsNotNone(snap) self.assertEqual(self.mock_cbtutil.set_cbt_child.call_count, 3) + + @mock.patch('LVHDSR.Lock', autospec=True) + @mock.patch('SR.XenAPI') + def test_snapshot_secondary_success(self, mock_xenapi, mock_lock): + """ + LVHDSR.snapshot, attached on host with secondary mirror + """ + # Arrange + xapi_session = mock_xenapi.xapi_local.return_value + xapi_session.xenapi.VDI.get_sm_config.return_value = {} + + vdi_ref = mock.MagicMock() + xapi_session.xenapi.VDI.get_by_uuid.return_value = vdi_ref + vdi_uuid = 'some VDI UUID' + self.get_dummy_vdi(vdi_uuid) + self.get_dummy_vhd(vdi_uuid, False) + + sr = self.create_LVHDSR() + sr.isMaster = True + sr.legacyMode = False + sr.srcmd.params = { + 'vdi_ref': 'test ref', + 'driver_params': { + 'type': 'double', + 'mirror': 'nbd:mirror_vbd/5/xvda'} + } + sr.cmd = "vdi_snapshot" + + vdi = sr.vdi('some VDI UUID') + vdi.vdi_type = vhdutil.VDI_TYPE_VHD + self.mock_sr_util.pathexists.return_value = True + self.mock_sr_util.get_hosts_attached_on.return_value = ["hostref2"] + self.mock_sr_util.get_this_host_ref.return_value = ["hostref1"] + self.mock_vhdutil.getDepth.return_value = 1 + + # Act + with mock.patch('lock.Lock'): + snap = vdi.snapshot(sr.uuid, "Dummy UUID") + + # Assert + self.assertIsNotNone(snap) + xapi_session.xenapi.VDI.add_to_other_config.assert_called_once_with( + vdi_ref, cleanup.VDI.DB_LEAFCLSC, cleanup.VDI.LEAFCLSC_DISABLED)