Name consistency, less intermediate variables, some more explicit logout calls, privatise some APISession methods.

Signed-off-by: Arnaud Garcia-Fernandez <arnaud.garcia-fernandez@vates.tech>

Author: Ythogtha

drivers/LinstorSR.py

@@ -2587,9 +2587,8 @@ def _start_persistent_http_server(volume_name):
                 # Use a timeout call because XAPI may be unusable on startup
                 # or if the host has been ejected. So in this case the call can
                 # block indefinitely.
-                apisession = util.timeout_call(
-                    5, util.APISession, "SM-LinstorSR-http")
-                session = apisession.session
+                api_session = util.timeout_call(5, util.APISession, "SM-ha-linstor-http-server")
+                session = api_session.session
                 host_ip = util.get_this_host_address(session)
             except:
                 # Fallback using the XHA file if session not available.
@@ -2598,6 +2597,8 @@ def _start_persistent_http_server(volume_name):
                     raise Exception(
                         'Cannot start persistent HTTP server: no XAPI session, nor XHA config file'
                     )
+            finally:
+                api_session.logout()
 
             arguments = [
                 'http-disk-server',
@@ -2673,9 +2674,8 @@ def _start_persistent_nbd_server(self, volume_name):
                 device_size = 256 * 1024 * 1024
 
             try:
-                apisession = util.timeout_call(
-                    5, util.APISession, "SM-LinstorSR-nbd")
-                session = apisession.session
+                api_session = util.timeout_call(5, util.APISession, "SM-ha-linstor-nbd-server")
+                session = api_session.session
                 ips = util.get_host_addresses(session)
             except Exception as e:
                 _, ips = get_ips_from_xha_config_file()
@@ -2684,6 +2684,8 @@ def _start_persistent_nbd_server(self, volume_name):
                         'Cannot start persistent NBD server: no XAPI session, nor XHA config file ({})'.format(e)
                     )
                 ips = ips.values()
+            finally:
+                api_session.logout()
 
             arguments = [
                 'nbd-http-server',

drivers/cleanup.py

@@ -297,10 +297,10 @@ class LookupError(util.SMException):
 
     def __init__(self, session, srUuid):
         self.session = session
-        self.apisession = None
+        self._api_session = None
         if self.session is None:
-            self.apisession = util.APISession("SM-cleanup-XAPI")
-            self.session = self.apisession.session
+            self._api_session = util.APISession("SM-cleanup-XAPI")
+            self.session = self._api_session.session
         self._srRef = self.session.xenapi.SR.get_by_uuid(srUuid)
         self.srRecord = self.session.xenapi.SR.get_record(self._srRef)
         self.hostUuid = util.get_this_host()
@@ -309,8 +309,8 @@ def __init__(self, session, srUuid):
         self.task_progress = {"coalescable": 0, "done": 0}
 
     def __del__(self):
-        if self.apisession:
-            self.apisession.logout()
+        if self._api_session:
+            self._api_session.logout()
 
     @property
     def srRef(self):
@@ -1993,7 +1993,7 @@ def msg_cleared(self, xapi_session, msg_ref):
         return msg is None
 
     def check_no_space_candidates(self):
-        with util.APISession("SM-cleanup-SR-check_no_space_candidates") as xapi_session:
+        with util.APISession("GC-check_no_space") as xapi_session:
             msg_id = self.xapi.srRecord["sm_config"].get(VDI.DB_GC_NO_SPACE)
             if self.no_space_candidates:
                 if msg_id is None or self.msg_cleared(xapi_session, msg_id):

drivers/coalesce-leaf

@@ -6,9 +6,9 @@
 # it under the terms of the GNU Lesser General Public License as published
 # by the Free Software Foundation; version 2.1 only.
 #
-# This program is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# This program is distributed in the hope that it will be useful, 
+# but WITHOUT ANY WARRANTY; without even the implied warranty of 
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the 
 # GNU Lesser General Public License for more details.
 #
 # You should have received a copy of the GNU Lesser General Public License

drivers/lcache.py

@@ -219,7 +219,7 @@ def from_session(cls, session):
 
     @classmethod
     def from_cli(cls):
-        with util.APISession("SM-lcache-CacheFileSR") as session:
+        with util.APISession("SM-local-cache") as session:
             return cls.from_session(session)
 
     def statvfs(self):

drivers/linstorvhdutil.py

@@ -634,8 +634,8 @@ def local_vhdutil(self):
         return self._executor_data.vhdutil
 
     def _init_executor_thread(self):
-        apisession = util.APISession("SM-linstorvhdutil")
-        session = apisession.session
+        api_session = util.APISession("SM-linstorvhdutil")
+        session = api_session.session
         try:
             linstor = LinstorVolumeManager(
                 self._uri,
@@ -649,7 +649,7 @@ def _init_executor_thread(self):
         except:
             self._executor_data.clear()
             raise
-        self._loads.append(apisession)
+        self._loads.append(api_session)
 
     def _cleanup(self):
         for load in self._loads:

drivers/linstorvolumemanager.py

@@ -98,7 +98,7 @@ def get_all_volume_openers(resource_name, volume):
     volume = str(volume)
     openers = {}
 
-    with util.APISession("SM-linstorvolumemanager-get_all_volume_openers") as session:
+    with util.APISession("SM-get-linstor-volume-openers") as session:
         hosts = session.xenapi.host.get_all_records()
         for host_ref, host_record in hosts.items():
             node_name = host_record['hostname']
@@ -187,8 +187,7 @@ def get_controller_node_name():
         if res:
             return res.groups()[0]
 
-    apisession = util.timeout_call(5, util.APISession, "SM-linstorvolumemanager-get_controller_node_name")
-    session = apisession.session
+    session = util.timeout_call(5, util.APISession, "SM-get-linstor-controller-node-name").session
 
     for host_ref, host_record in session.xenapi.host.get_all_records().items():
         node_name = host_record['hostname']
@@ -211,8 +210,7 @@ def get_controller_node_name():
 def demote_drbd_resource(node_name, resource_name):
     PLUGIN_CMD = 'demoteDrbdResource'
 
-    apisession = util.timeout_call(5, util.APISession, "SM-linstorvolumemanager-demote_drbd_resource")
-    session = apisession.session
+    session = util.timeout_call(5, util.APISession, "SM-demote-drbd-resource").session
 
     for host_ref, host_record in session.xenapi.host.get_all_records().items():
         if host_record['hostname'] != node_name:
@@ -1404,8 +1402,7 @@ def destroy(self):
             # It needs to be done locally by each host so we go through the linstor-manager plugin.
             # If we don't do this sometimes, the destroy will fail when trying to destroy the resource groups with:
             # "linstor-manager:destroy error: Failed to destroy SP `xcp-sr-linstor_group_thin_device` on node `r620-s2`: The specified storage pool 'xcp-sr-linstor_group_thin_device' on node 'r620-s2' can not be deleted as volumes / snapshot-volumes are still using it."
-            apisession = util.timeout_call(5, util.APISession, "SM-linstorvolumemanager-destroy")
-            session = apisession.session
+            session = util.timeout_call(5, util.APISession, "SM-linstor-destroy").session
             for host_ref in session.xenapi.host.get_all():
                 try:
                     response = session.xenapi.host.call_plugin(

drivers/mpathcount.py

@@ -222,8 +222,7 @@ def _run_command(command, timeout):
 
 if __name__ == '__main__':
     try:
-        apisession = util.APISession("SM-mpathcount")
-        session = apisession.session
+        session = util.APISession("SM-mpathcount").session
     except xs_errors.XenError:
         sys.exit(-1)
 

drivers/sr_health_check.py

@@ -33,7 +33,7 @@ def main():
     """
     For all locally plugged SRs check that they are healthy
     """
-    with util.APISession("SM-sr_health_check") as session:
+    with util.APISession("SM-sr-health-check") as session:
         localhost = util.get_localhost_ref(session)
         if not check_xapi_is_enabled(session, localhost):
             # Xapi not enabled, skip and let the next timer trigger this

drivers/util.py

@@ -401,9 +401,10 @@ def ioretry_stat(path, maxretry=IORETRY_MAX):
 
 def sr_get_capability(sr_uuid, session=None):
     result = []
+    api_session = None
     if session is None:
-        apisession = APISession("SM-util-sr_get_capability")
-        session = apisession.session
+        api_session = APISession("SM-sr-get-capability")
+        session = api_session.session
     sr_ref = session.xenapi.SR.get_by_uuid(sr_uuid)
     sm_type = session.xenapi.SR.get_record(sr_ref)['type']
     sm_rec = session.xenapi.SM.get_all_records_where(
@@ -412,7 +413,8 @@ def sr_get_capability(sr_uuid, session=None):
     # SM expects at least one entry of any SR type
     if len(sm_rec) > 0:
         result = list(sm_rec.values())[0]['capabilities']
-
+    if api_session:
+        session.logout()
     return result
 
 def sr_get_driver_info(driver_info):
@@ -705,19 +707,17 @@ def getrootdevID():
 class APISession(contextlib.AbstractContextManager):
     def __init__(self, originator="SM"):
         self.originator = originator
-        self.session = self.login()
+        # First acquire a valid session
+        self.session = self._login()
         SMlog("APISession [{}] login".format(self.originator), priority=LOG_DEBUG)
-        atexit.register(self.atexit)
+        atexit.register(self._atexit)
 
-    def login(self):
-        # First acquire a valid session
+    def _login(self):
         session = XenAPI.xapi_local()
         try:
             session.xenapi.login_with_password('root', '', '', self.originator)
         except Exception as exc:
-            msg = f"APISession [{self.originator}] Unable to open local XAPI session"
-            SMlog(msg, priority=LOG_ERR)
-            raise xs_errors.XenError(msg) from exc
+            raise xs_errors.XenError(f"APISession [{self.originator}] Unable to open local XAPI session") from exc
         return session
 
     def _logout(self, log):
@@ -730,14 +730,14 @@ def _logout(self, log):
         self.session = None
 
     def logout(self, log="logout"):
-        atexit.unregister(self.atexit)
+        atexit.unregister(self._atexit)
         self._logout(log=log)
 
     def __del__(self):
         if self.session:
             self.logout(log="logout del")
 
-    def atexit(self):
+    def _atexit(self):
         self._logout(log="logout atexit")
 
     @override

tests/test_cleanup.py

@@ -59,6 +59,7 @@ def acquireNoblock(self):
 class IrrelevantLock(object):
     pass
 
+
 def create_cleanup_sr(xapi, uuid=None):
     return cleanup.SR(uuid=uuid, xapi=xapi, createLock=False, force=False)
 
@@ -84,9 +85,9 @@ def setUp(self) -> None:
         self.xapi_mock.isMaster.return_value = True
         self.mock_xapi_session = mock.MagicMock(name="MockSession")
 
-        util_apisession_patcher = mock.patch('cleanup.util.APISession.login')
-        self.mock_util_apisession_patcher = util_apisession_patcher.start()
-        self.mock_util_apisession_patcher.return_value = self.mock_xapi_session
+        util_api_session_patcher = mock.patch('cleanup.util.APISession._login')
+        self.mock_util_api_session_patcher = util_api_session_patcher.start()
+        self.mock_util_api_session_patcher.return_value = self.mock_xapi_session
 
         self.addCleanup(mock.patch.stopall)