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

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):