Multiple changes regarding linstorvhdutil: refactor + fixes

drivers/LinstorSR.py

@@ -150,9 +150,7 @@ def attach_thin(session, journaler, linstor, sr_uuid, vdi_uuid):
     # If the virtual VHD size is lower than the LINSTOR volume size,
     # there is nothing to do.
     vhd_size = LinstorVhdUtil.compute_volume_size(
-        # TODO: Replace pylint comment with this feature when possible:
-        # https://github.com/PyCQA/pylint/pull/2926
-        LinstorVhdUtil(session, linstor).get_size_virt(vdi_uuid),  # pylint: disable = E1120
+        LinstorVhdUtil(session, linstor).get_size_virt(vdi_uuid),
         image_type
     )
 
@@ -195,9 +193,7 @@ def check_vbd_count():
     device_path = linstor.get_device_path(vdi_uuid)
     vhdutil_inst = LinstorVhdUtil(session, linstor)
     new_volume_size = LinstorVolumeManager.round_up_volume_size(
-        # TODO: Replace pylint comment with this feature when possible:
-        # https://github.com/PyCQA/pylint/pull/2926
-        vhdutil_inst.get_size_phys(vdi_uuid)  # pylint: disable = E1120
+        vhdutil_inst.get_size_phys(vdi_uuid)
     )
 
     volume_info = linstor.get_volume_info(vdi_uuid)
@@ -1210,9 +1206,7 @@ def _load_vdis_ex(self):
             self.vdis[vdi_uuid] = vdi
 
             if USE_KEY_HASH and vdi.vdi_type == vhdutil.VDI_TYPE_VHD:
-                # TODO: Replace pylint comment with this feature when possible:
-                # https://github.com/PyCQA/pylint/pull/2926
-                vdi.sm_config_override['key_hash'] = self._vhdutil.get_key_hash(vdi_uuid)  # pylint: disable = E1120
+                vdi.sm_config_override['key_hash'] = self._vhdutil.get_key_hash(vdi_uuid)
 
             # 4.c. Update CBT status of disks either just added
             # or already in XAPI.

drivers/linstorvhdutil.py

@@ -30,7 +30,7 @@
 MANAGER_PLUGIN = 'linstor-manager'
 
 
-def call_remote_method(session, host_ref, method, device_path, args):
+def call_remote_method(session, host_ref, method, args):
     try:
         response = session.xenapi.host.call_plugin(
             host_ref, MANAGER_PLUGIN, method, args
@@ -75,23 +75,14 @@ class NoPathLinstorCallException(LinstorCallException):
     pass
 
 def log_successful_call(target_host, device_path, vdi_uuid, remote_method, response):
-    util.SMlog(
-        'Successful access on {} for device {} ({}): `{}` => {}'.format(target_host, device_path, vdi_uuid, remote_method, str(response)),
-        priority=util.LOG_DEBUG
-    )
+    util.SMlog('Successful access on {} for device {} ({}): `{}` => {}'.format(
+        target_host, device_path, vdi_uuid, remote_method, str(response)
+    ), priority=util.LOG_DEBUG)
 
 def log_failed_call(target_host, next_target, device_path, vdi_uuid, remote_method, e):
-    util.SMlog(
-        'Failed to call method on {} for device {} ({}): {}. Trying accessing on {}... (cause: {})'.format(
-            target_host,
-            device_path,
-            vdi_uuid,
-            remote_method,
-            next_target,
-            e
-        ),
-        priority=util.LOG_DEBUG
-    )
+    util.SMlog('Failed to call method on {} for device {} ({}): {}. Trying accessing on {}... (cause: {})'.format(
+        target_host, device_path, vdi_uuid, remote_method, next_target, e
+    ), priority=util.LOG_DEBUG)
 
 def linstorhostcall(local_method, remote_method):
     def decorated(response_parser):
@@ -110,52 +101,47 @@ def wrapper(*args, **kwargs):
             remote_args.update(**kwargs)
             remote_args = {str(key): str(value) for key, value in remote_args.items()}
 
+            this_host_ref = util.get_this_host_ref(self._session)
+            def call_method(host_label, host_ref):
+                if host_ref == this_host_ref:
+                    return self._call_local_method(local_method, device_path, *args[2:], **kwargs)
+                response = call_remote_method(self._session, host_ref, remote_method, remote_args)
+                log_successful_call(host_label, device_path, vdi_uuid, remote_method, response)
+                return response_parser(self, vdi_uuid, response)
+
+            # 1. Try on attached host.
             try:
-                host_ref_attached = next(iter(util.get_hosts_attached_on(self._session, [vdi_uuid])))
+                host_ref_attached = next(iter(util.get_hosts_attached_on(self._session, [vdi_uuid])), None)
                 if host_ref_attached:
-                    response = call_remote_method(
-                        self._session, host_ref_attached, remote_method, device_path, remote_args
-                    )
-                    log_successful_call('attached node', device_path, vdi_uuid, remote_method, response)
-                    return response_parser(self, vdi_uuid, response)
+                    return call_method('attached host', host_ref_attached)
             except Exception as e:
-                log_failed_call('attached node', 'master', device_path, vdi_uuid, remote_method, e)
+                log_failed_call('attached host', 'master', device_path, vdi_uuid, remote_method, e)
 
+            # 2. Try on master host.
             try:
-                master_ref = util.get_master_ref(self._session)
-                response = call_remote_method(self._session, master_ref, remote_method, device_path, remote_args)
-                log_successful_call('master', device_path, vdi_uuid, remote_method, response)
-                return response_parser(self, vdi_uuid, response)
+                return call_method('master', util.get_master_ref(self._session))
             except Exception as e:
                 log_failed_call('master', 'primary', device_path, vdi_uuid, remote_method, e)
 
+            # 3. Try on a primary.
+            hosts = self._get_hosts(remote_method, device_path)
 
             nodes, primary_hostname = self._linstor.find_up_to_date_diskful_nodes(vdi_uuid)
             if primary_hostname:
                 try:
-                    host_ref = self._get_readonly_host(vdi_uuid, device_path, {primary_hostname})
-                    response = call_remote_method(self._session, host_ref, remote_method, device_path, remote_args)
-                    log_successful_call('primary', device_path, vdi_uuid, remote_method, response)
-                    return response_parser(self, vdi_uuid, response)
+                    return call_method('primary', self._find_host_ref_from_hostname(hosts, primary_hostname))
                 except Exception as remote_e:
                     self._raise_openers_exception(device_path, remote_e)
-            else:
-                log_failed_call(
-                    'primary',
-                    'another node',
-                    device_path,
-                    vdi_uuid,
-                    remote_method,
-                    'no primary'
-                )
 
-                try:
-                    host = self._get_readonly_host(vdi_uuid, device_path, nodes)
-                    response = call_remote_method(self._session, host, remote_method, device_path, remote_args)
-                    log_successful_call('another node', device_path, vdi_uuid, remote_method, response)
-                    return response_parser(self, vdi_uuid, response)
-                except Exception as remote_e:
-                    self._raise_openers_exception(device_path, remote_e)
+            log_failed_call('primary', 'another node', device_path, vdi_uuid, remote_method, 'no primary')
+
+            # 4. Try on any host with local data.
+            try:
+                return call_method('another node', next(filter(None,
+                    (self._find_host_ref_from_hostname(hosts, hostname) for hostname in nodes)
+                ), None))
+            except Exception as remote_e:
+                self._raise_openers_exception(device_path, remote_e)
 
         return wrapper
     return decorated
@@ -228,7 +214,7 @@ def check(self, vdi_uuid, ignore_missing_footer=False, fast=False):
             'fast': fast
         }
         try:
-            self._check(vdi_uuid, **kwargs)  # pylint: disable = E1123
+            self._check(vdi_uuid, **kwargs)
             return True
         except Exception as e:
             util.SMlog('Call to `check` failed: {}'.format(e))
@@ -243,9 +229,7 @@ def get_vhd_info(self, vdi_uuid, include_parent=True):
             'includeParent': include_parent,
             'resolveParent': False
         }
-        # TODO: Replace pylint comment with this feature when possible:
-        # https://github.com/PyCQA/pylint/pull/2926
-        return self._get_vhd_info(vdi_uuid, self._extract_uuid, **kwargs)  # pylint: disable = E1123
+        return self._get_vhd_info(vdi_uuid, self._extract_uuid, **kwargs)
 
     @linstorhostcall(vhdutil.getVHDInfo, 'getVHDInfo')
     def _get_vhd_info(self, vdi_uuid, response):
@@ -359,9 +343,7 @@ def inflate(self, journaler, vdi_uuid, vdi_path, new_size, old_size):
         )
         self._linstor.resize_volume(vdi_uuid, new_size)
 
-        # TODO: Replace pylint comment with this feature when possible:
-        # https://github.com/PyCQA/pylint/pull/2926
-        result_size = self.get_drbd_size(vdi_uuid)  # pylint: disable = E1120
+        result_size = self.get_drbd_size(vdi_uuid)
         if result_size < new_size:
             util.SMlog(
                 'WARNING: Cannot inflate volume to {}B, result size: {}B'
@@ -463,35 +445,22 @@ def _extract_uuid(self, device_path):
             device_path.rstrip('\n')
         )
 
-    def _get_readonly_host(self, vdi_uuid, device_path, node_names):
-        """
-        When vhd-util is called to fetch VDI info we must find a
-        diskful DRBD disk to read the data. It's the goal of this function.
-        Why? Because when a VHD is open in RO mode, the LVM layer is used
-        directly to bypass DRBD verifications (we can have only one process
-        that reads/writes to disk with DRBD devices).
-        """
-
-        if not node_names:
+    def _get_hosts(self, remote_method, device_path):
+        try:
+            return self._session.xenapi.host.get_all_records()
+        except Exception as e:
             raise xs_errors.XenError(
                 'VDIUnavailable',
-                opterr='Unable to find diskful node: {} (path={})'
-                .format(vdi_uuid, device_path)
+                opterr='Unable to get host list to run vhdutil command `{}` (path={}): {}'
+                .format(remote_method, device_path, e)
             )
 
-        hosts = self._session.xenapi.host.get_all_records()
-        for host_ref, host_record in hosts.items():
-            if host_record['hostname'] in node_names:
-                return host_ref
-
-        raise xs_errors.XenError(
-            'VDIUnavailable',
-            opterr='Unable to find a valid host from VDI: {} (path={})'
-            .format(vdi_uuid, device_path)
-        )
-
     # --------------------------------------------------------------------------
 
+    @staticmethod
+    def _find_host_ref_from_hostname(hosts, hostname):
+        return next((ref for ref, rec in hosts.items() if rec['hostname'] == hostname), None)
+
     def _raise_openers_exception(self, device_path, e):
         if isinstance(e, util.CommandException):
             e_str = 'cmd: `{}`, code: `{}`, reason: `{}`'.format(e.cmd, e.code, e.reason)
@@ -560,14 +529,7 @@ def _call_method(self, local_method, remote_method, device_path, use_parent, *ar
 
         # B. Execute the command on another host.
         # B.1. Get host list.
-        try:
-            hosts = self._session.xenapi.host.get_all_records()
-        except Exception as e:
-            raise xs_errors.XenError(
-                'VDIUnavailable',
-                opterr='Unable to get host list to run vhd-util command `{}` (path={}): {}'
-                .format(remote_method, device_path, e)
-            )
+        hosts = self._get_hosts(remote_method, device_path)
 
         # B.2. Prepare remote args.
         remote_args = {
@@ -602,14 +564,13 @@ def remote_call():
                 if not openers:
                     continue
 
-                try:
-                    host_ref = next(ref for ref, rec in hosts.items() if rec['hostname'] == hostname)
-                except StopIteration:
+                host_ref = self._find_host_ref_from_hostname(hosts, hostname)
+                if not host_ref:
                     continue
 
                 no_host_found = False
                 try:
-                    return call_remote_method(self._session, host_ref, remote_method, device_path, remote_args)
+                    return call_remote_method(self._session, host_ref, remote_method, remote_args)
                 except Exception:
                     pass
 

tests/pylintrc

@@ -92,6 +92,8 @@ generated-members=REQUEST,acl_users,aq_parent,linstor.*
 # # supports qualified module names, as well as Unix pattern matching.
 ignored-modules=bitarray
 
+# List of decorators that change the signature of a decorated function.
+signature-mutators=linstorvhdutil.linstorhostcall
 
 [SIMILARITIES]