CA-421013: ensure cbt log removed on supporter after disable

When CBT is disabled the CBT log is deleted, on the coordinator, when
using shared LVM SRs it is necessary to inform the supporter of this
change so that it can remove all information relating to the CBT log
volume from the kernel device mapper table. If this is not done a
subsequent CBT enable for this VDI will fail if the device mapper
table still contains information about the previous, and now deleted,
volume.

Signed-off-by: Mark Syms <[email protected]>

Author: MarkSymsCtx

libs/sm/VDI.py

@@ -572,6 +572,10 @@ def in_sync_with_xenapi_record(self, x):
             return False
         return True
 
+    def update_slaves_on_cbt_disable(self, cbtlog):
+        # Override in implementation as required.
+        pass
+
     def configure_blocktracking(self, sr_uuid, vdi_uuid, enable):
         """Function for configuring blocktracking"""
         from sm import blktap2
@@ -626,6 +630,8 @@ def configure_blocktracking(self, sr_uuid, vdi_uuid, enable):
                     if self._cbt_log_exists(parent_path):
                         self._cbt_op(parent, cbtutil.set_cbt_child,
                                      parent_path, uuid.UUID(int=0))
+                    if disk_state:
+                        self.update_slaves_on_cbt_disable(logpath)
                 except Exception as error:
                     raise xs_errors.XenError('CBTDeactivateFailed', str(error))
                 finally:

libs/sm/drivers/LVHDSR.py

@@ -1197,20 +1197,25 @@ def _undoAllVHDJournals(self):
         delattr(self, "vdiInfo")
         delattr(self, "allVDIs")
 
-    def _updateSlavesPreClone(self, hostRefs, origOldLV):
-        masterRef = util.get_this_host_ref(self.session)
-        args = {"vgName": self.vgname,
-                "action1": "deactivateNoRefcount",
-                "lvName1": origOldLV}
-        for hostRef in hostRefs:
-            if hostRef == masterRef:
+    def call_on_slave(self, args, host_refs, message: str):
+        master_ref = util.get_this_host_ref(self.session)
+        for hostRef in host_refs:
+            if hostRef == master_ref:
                 continue
-            util.SMlog("Deactivate VDI on %s" % hostRef)
-            rv = self.session.xenapi.host.call_plugin(hostRef, self.PLUGIN_ON_SLAVE, "multi", args)
+            util.SMlog(f"{message} on slave {hostRef}")
+            rv = self.session.xenapi.host.call_plugin(
+                hostRef, self.PLUGIN_ON_SLAVE, "multi", args)
             util.SMlog("call-plugin returned: %s" % rv)
             if not rv:
                 raise Exception('plugin %s failed' % self.PLUGIN_ON_SLAVE)
 
+    def _updateSlavesPreClone(self, hostRefs, origOldLV):
+        args = {"vgName": self.vgname,
+                "action1": "deactivateNoRefcount",
+                "lvName1": origOldLV}
+        message = "Deactivate VDI"
+        self.call_on_slave(args, hostRefs, message)
+
     def _updateSlavesOnClone(self, hostRefs, origOldLV, origLV,
             baseUuid, baseLV):
         """We need to reactivate the original LV on each slave (note that the
@@ -1224,17 +1229,8 @@ def _updateSlavesOnClone(self, hostRefs, origOldLV, origLV,
                 "lvName2": baseLV,
                 "uuid2": baseUuid}
 
-        masterRef = util.get_this_host_ref(self.session)
-        for hostRef in hostRefs:
-            if hostRef == masterRef:
-                continue
-            util.SMlog("Updating %s, %s, %s on slave %s" % \
-                    (origOldLV, origLV, baseLV, hostRef))
-            rv = self.session.xenapi.host.call_plugin(
-                hostRef, self.PLUGIN_ON_SLAVE, "multi", args)
-            util.SMlog("call-plugin returned: %s" % rv)
-            if not rv:
-                raise Exception('plugin %s failed' % self.PLUGIN_ON_SLAVE)
+        message = f"Updating {origOldLV}, {origLV}, {baseLV}"
+        self.call_on_slave(args, hostRefs, message)
 
     def _updateSlavesOnCBTClone(self, hostRefs, cbtlog):
         """Reactivate and refresh CBT log file on slaves"""
@@ -1244,16 +1240,8 @@ def _updateSlavesOnCBTClone(self, hostRefs, cbtlog):
                 "action2": "refresh",
                 "lvName2": cbtlog}
 
-        masterRef = util.get_this_host_ref(self.session)
-        for hostRef in hostRefs:
-            if hostRef == masterRef:
-                continue
-            util.SMlog("Updating %s on slave %s" % (cbtlog, hostRef))
-            rv = self.session.xenapi.host.call_plugin(
-                hostRef, self.PLUGIN_ON_SLAVE, "multi", args)
-            util.SMlog("call-plugin returned: %s" % rv)
-            if not rv:
-                raise Exception('plugin %s failed' % self.PLUGIN_ON_SLAVE)
+        message = f"Updating {cbtlog}"
+        self.call_on_slave(args, hostRefs, message)
 
     def _updateSlavesOnRemove(self, hostRefs, baseUuid, baseLV):
         """Tell the slave we deleted the base image"""
@@ -1262,16 +1250,8 @@ def _updateSlavesOnRemove(self, hostRefs, baseUuid, baseLV):
                 "uuid1": baseUuid,
                 "ns1": lvhdutil.NS_PREFIX_LVM + self.uuid}
 
-        masterRef = util.get_this_host_ref(self.session)
-        for hostRef in hostRefs:
-            if hostRef == masterRef:
-                continue
-            util.SMlog("Cleaning locks for %s on slave %s" % (baseLV, hostRef))
-            rv = self.session.xenapi.host.call_plugin(
-                hostRef, self.PLUGIN_ON_SLAVE, "multi", args)
-            util.SMlog("call-plugin returned: %s" % rv)
-            if not rv:
-                raise Exception('plugin %s failed' % self.PLUGIN_ON_SLAVE)
+        message = f"Cleaning locks for {baseLV}"
+        self.call_on_slave(args, hostRefs, message)
 
     def _cleanup(self, skipLockCleanup=False):
         """delete stale refcounter, flag, and lock files"""
@@ -2203,6 +2183,18 @@ def _rename(self, oldpath, newpath):
         newname = os.path.basename(newpath)
         self.sr.lvmCache.rename(oldname, newname)
 
+    def update_slaves_on_cbt_disable(self, cbtlog):
+        args = {
+            "vgName": self.sr.vgname,
+            "action1": "deactivateNoRefcount",
+            "lvName1": cbtlog
+        }
+
+        host_refs = util.get_hosts_attached_on(self.session, [self.uuid])
+
+        message = f"Deactivating {cbtlog}"
+        self.sr.call_on_slave(args, host_refs, message)
+
     def _activate_cbt_log(self, lv_name):
         self.sr.lvmCache.refresh()
         if not self.sr.lvmCache.is_active(lv_name):

tests/test_LVHDSR.py

@@ -479,6 +479,38 @@ def test_snapshot_attached_cbt_success(self, mock_xenapi, mock_lock):
         self.assertIsNotNone(snap)
         self.assertEqual(self.mock_cbtutil.set_cbt_child.call_count, 3)
 
+    @mock.patch('sm.drivers.LVHDSR.Lock', autospec=True)
+    @mock.patch('sm.drivers.LVHDSR.SR.XenAPI')
+    def test_update_slaves_on_cbt_disable(self, mock_xenapi, mock_lock):
+        """
+        Ensure we tell the supporter host when we disable CBT for one of its VMs
+        """
+        # Arrange
+        xapi_session = mock_xenapi.xapi_local.return_value
+
+        vdi_uuid = str(uuid.uuid4)
+        mock_lv = self.get_dummy_vdi(vdi_uuid)
+        self.get_dummy_vhd(vdi_uuid, False)
+
+        sr = self.create_LVHDSR()
+        sr.isMaster = True
+
+        vdi = sr.vdi(vdi_uuid)
+        vdi.vdi_type = vhdutil.VDI_TYPE_VHD
+
+        self.mock_sr_util.get_this_host_ref.return_value = 'ref1'
+        self.mock_sr_util.get_hosts_attached_on.return_value = ['ref2']
+
+        # Act
+        log_file_path = "test_log_path"
+        vdi.update_slaves_on_cbt_disable(log_file_path)
+
+        # Assert
+        self.assertEqual(1, xapi_session.xenapi.host.call_plugin.call_count)
+        xapi_session.xenapi.host.call_plugin.assert_has_calls([
+            mock.call('ref2', 'on-slave', 'multi', mock.ANY)
+        ])
+
     @mock.patch('sm.drivers.LVHDSR.Lock', autospec=True)
     @mock.patch('sm.drivers.LVHDSR.SR.XenAPI')
     def test_snapshot_secondary_success(self, mock_xenapi, mock_lock):