fix(blktap2): retry host key tag removal on XAPI HTTP failure (#149)

- Add a decorator to retry a method when a XAPI call fails with HTTPStatus.INTERNAL_SERVER_ERROR.
- Decorate blktap2:_get_pool_config(): this method gets data, therefore it can be retried.
- Decorate blktap2:_check_tag().
- Retry plugin call of cancel_coalesce_master in blktap2:_check_journal_coalesce_chain().
- Remove superfluous calls to XAPI in blktap2:activate().

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

Author: Ythogtha

drivers/blktap2.py

@@ -30,7 +30,6 @@
 from lock import Lock
 import util
 import xmlrpc.client
-import http.client
 import errno
 import signal
 import subprocess
@@ -1556,6 +1555,13 @@ def call_pluginhandler(cls, session, host_ref, sr_uuid, vdi_uuid, action,
             util.logException("BLKTAP2:call_pluginhandler %s" % e)
             return False
 
+    @util.xapi_safe_call
+    def _call_xapi_plugin(self, *args):
+        """
+        wrapper to retry xenapi.host.call_plugin on XAPI HTTP failure.
+        """
+        return self._session.xenapi.host.call_plugin(*args)
+
     def _add_tag(self, vdi_uuid, writable):
         util.SMlog("Adding tag to: %s" % vdi_uuid)
         attach_mode = "RO"
@@ -1601,6 +1607,7 @@ def _add_tag(self, vdi_uuid, writable):
         util.SMlog("Activate lock succeeded")
         return True
 
+    @util.xapi_safe_call
     def _check_tag(self, vdi_uuid):
         vdi_ref = self._session.xenapi.VDI.get_by_uuid(vdi_uuid)
         sm_config = self._session.xenapi.VDI.get_sm_config(vdi_ref)
@@ -1609,6 +1616,7 @@ def _check_tag(self, vdi_uuid):
             return False
         return True
 
+    @util.xapi_safe_call
     def _remove_tag(self, vdi_uuid):
         vdi_ref = self._session.xenapi.VDI.get_by_uuid(vdi_uuid)
         host_ref = self._session.xenapi.host.get_by_uuid(util.get_this_host())
@@ -1620,8 +1628,9 @@ def _remove_tag(self, vdi_uuid):
         else:
             util.SMlog("_remove_tag: host key %s not found, ignore" % host_key)
 
+    @util.xapi_safe_call
     def _get_pool_config(self, pool_name):
-        pool_info = dict()
+        pool_info = {}
         vdi_ref = self.target.vdi.sr.srcmd.params.get('vdi_ref')
         if not vdi_ref:
             # attach_from_config context: HA disks don't need to be in any
@@ -1706,9 +1715,7 @@ def activate(self, sr_uuid, vdi_uuid, writable, caching_params):
         options = {"rdonly": not writable}
         options.update(caching_params)
 
-        sr_ref = self.target.vdi.sr.srcmd.params.get('sr_ref')
-        sr_other_config = self._session.xenapi.SR.get_other_config(sr_ref)
-        for i in range(self.ATTACH_DETACH_RETRY_SECS):
+        for _ in range(self.ATTACH_DETACH_RETRY_SECS):
             try:
                 if self._activate_locked(sr_uuid, vdi_uuid, options):
                     return
@@ -1795,8 +1802,8 @@ def _check_journal_coalesce_chain(self, sr_uuid: str, vdi_uuid: str) -> bool:
         host_ref = self._get_sr_master_host_ref()
         for vdi in vdi_to_cancel:
             args = {"sr_uuid": sr_uuid, "vdi_uuid": vdi}
-            util.SMlog("Calling cancel_coalesce_master with args: {}".format(args))
-            self._session.xenapi.host.call_plugin(\
+            util.SMlog(f"Calling cancel_coalesce_master with args: {args}")
+            self._call_xapi_plugin(
                 host_ref, PLUGIN_ON_SLAVE, "cancel_coalesce_master", args)
 
         return True
@@ -1871,22 +1878,9 @@ def _activate_locked(self, sr_uuid, vdi_uuid, options):
             util.SMlog("Exception in activate/attach")
             if self.tap_wanted():
                 util.fistpoint.activate_custom_fn(
-                        "blktap_activate_error_handling",
-                        lambda: time.sleep(30))
-                while True:
-                    try:
-                        self._remove_tag(vdi_uuid)
-                        break
-                    except xmlrpc.client.ProtocolError as e:
-                        # If there's a connection error, keep trying forever.
-                        if e.errcode == http.HTTPStatus.INTERNAL_SERVER_ERROR.value:
-                            continue
-                        else:
-                            util.SMlog('failed to remove tag: %s' % e)
-                            break
-                    except Exception as e:
-                        util.SMlog('failed to remove tag: %s' % e)
-                        break
+                    "blktap_activate_error_handling",
+                    lambda: time.sleep(30))
+                self._remove_tag(vdi_uuid)
             raise
         finally:
             vdi_ref = self._session.xenapi.VDI.get_by_uuid(vdi_uuid)

drivers/util.py

@@ -39,6 +39,7 @@
 import xs_errors
 import XenAPI # pylint: disable=import-error
 import xmlrpc.client
+import http.client
 import base64
 import syslog
 import resource
@@ -783,6 +784,28 @@ def get_localAPI_session():
     return session
 
 
+def xapi_safe_call(function):
+    """
+    Decorator to catch classic XAPI connection problems and retry forever.
+    The method should be only called for XAPI calls.
+    eg: blktap2.py#VDI:_remove_tag()
+    """
+    def wrapper(*args, **kwargs):
+        call_str = f"{function.__name__}(args={args}, kwargs={kwargs})"
+        while True:
+            try:
+                return function(*args, **kwargs)
+            except xmlrpc.client.ProtocolError as e:
+                # If there's a connection error, keep trying forever.
+                if e.errcode == http.HTTPStatus.INTERNAL_SERVER_ERROR.value:
+                    continue
+                raise
+            except Exception as e:
+                SMlog(f"Failed XAPI call `{call_str}`: `{e}`")
+                raise
+    return wrapper
+
+
 def get_this_host():
     uuid = None
     f = open("/etc/xensource-inventory", 'r')