Merge branch 'ran-3.2.3-8.3-mypy' into 3.2.3-8.3

.github/workflows/main.yml

@@ -3,6 +3,25 @@ name: Test SM
 on: [push, pull_request]
 
 jobs:
+  static-analysis:
+    runs-on: ubuntu-latest
+
+    steps:
+      - uses: actions/checkout@v3
+      - name: Set up Python 3
+        uses: actions/setup-python@v4
+        with:
+          python-version: '3.x'
+
+      - name: Install dependencies
+        run: |
+          python -m pip install --upgrade pip
+          pip install -r dev_requirements_static_analysis.txt
+
+      - name: mypy
+        run: |
+          mypy .
+
   build:
     runs-on: ubuntu-20.04
 

.mypy.ini

@@ -0,0 +1,3 @@
+[mypy]
+enable_error_code = explicit-override
+

Makefile

@@ -127,7 +127,7 @@ precheck: build
 	echo "Precheck succeeded with no outstanding issues found."
 
 .PHONY: install
-install: precheck
+install: build
 	mkdir -p $(SM_STAGING)
 	$(call mkdir_clean,$(SM_STAGING))
 	mkdir -p $(SM_STAGING)$(SM_DEST)
@@ -231,6 +231,7 @@ install: precheck
 	install -m 755 scripts/set-iscsi-initiator $(SM_STAGING)$(LIBEXEC)
 	$(MAKE) -C dcopy install DESTDIR=$(SM_STAGING)
 	$(MAKE) -C linstor install DESTDIR=$(SM_STAGING)
+	$(MAKE) -C sm_typing install DESTDIR=$(SM_STAGING)
 	ln -sf $(SM_DEST)blktap2.py $(SM_STAGING)$(BIN_DEST)/blktap2
 	ln -sf $(SM_DEST)lcache.py $(SM_STAGING)$(BIN_DEST)tapdisk-cache-stats
 	ln -sf /dev/null $(SM_STAGING)$(UDEV_RULES_DIR)/69-dm-lvm-metad.rules

dev_requirements_static_analysis.txt

@@ -0,0 +1,3 @@
+bitarray
+mypy
+python-linstor

drivers/BaseISCSI.py

@@ -18,7 +18,10 @@
 # ISCSISR: ISCSI software initiator SR driver
 #
 
+from sm_typing import override
+
 import SR
+import VDI
 import util
 import time
 import LUNperVDI
@@ -100,11 +103,12 @@ def address(self):
             self._initPaths()
         return self._address
 
-    def handles(type):
+    @override
+    @staticmethod
+    def handles(type) -> bool:
         return False
-    handles = staticmethod(handles)
 
-    def _synchroniseAddrList(self, addrlist):
+    def _synchroniseAddrList(self, addrlist) -> None:
         if not self.multihomed:
             return
         change = False
@@ -133,7 +137,8 @@ def _synchroniseAddrList(self, addrlist):
             except:
                 pass
 
-    def load(self, sr_uuid):
+    @override
+    def load(self, sr_uuid) -> None:
         if self.force_tapdisk:
             self.sr_vditype = 'aio'
         else:
@@ -175,23 +180,23 @@ def load(self, sr_uuid):
                 and ('chappassword' in self.dconf or 'chappassword_secret' in self.dconf):
             self.chapuser = self.dconf['chapuser'].encode('utf-8')
             if 'chappassword_secret' in self.dconf:
-                self.chappassword = util.get_secret(self.session, self.dconf['chappassword_secret'])
+                chappassword = util.get_secret(self.session, self.dconf['chappassword_secret'])
             else:
-                self.chappassword = self.dconf['chappassword']
+                chappassword = self.dconf['chappassword']
 
-            self.chappassword = self.chappassword.encode('utf-8')
+            self.chappassword = chappassword.encode('utf-8')
 
         self.incoming_chapuser = ""
         self.incoming_chappassword = ""
         if 'incoming_chapuser' in self.dconf \
                 and ('incoming_chappassword' in self.dconf or 'incoming_chappassword_secret' in self.dconf):
             self.incoming_chapuser = self.dconf['incoming_chapuser'].encode('utf-8')
             if 'incoming_chappassword_secret' in self.dconf:
-                self.incoming_chappassword = util.get_secret(self.session, self.dconf['incoming_chappassword_secret'])
+                incoming_chappassword = util.get_secret(self.session, self.dconf['incoming_chappassword_secret'])
             else:
-                self.incoming_chappassword = self.dconf['incoming_chappassword']
+                incoming_chappassword = self.dconf['incoming_chappassword']
 
-            self.incoming_chappassword = self.incoming_chappassword.encode('utf-8')
+            self.incoming_chappassword = incoming_chappassword.encode('utf-8')
 
         self.port = DEFAULT_PORT
         if 'port' in self.dconf and self.dconf['port']:
@@ -276,7 +281,7 @@ def _initPaths(self):
             self._address = self.tgtidx
         self._synchroniseAddrList(addrlist)
 
-    def _init_adapters(self):
+    def _init_adapters(self) -> None:
         # Generate a list of active adapters
         ids = scsiutil._genHostList(ISCSI_PROCNAME)
         util.SMlog(ids)
@@ -293,7 +298,8 @@ def _init_adapters(self):
                 pass
         self._devs = scsiutil.cacheSCSIidentifiers()
 
-    def attach(self, sr_uuid):
+    @override
+    def attach(self, sr_uuid) -> None:
         self._mpathHandle()
 
         multiTargets = False
@@ -391,10 +397,9 @@ def attach(self, sr_uuid):
         util._incr_iscsiSR_refcount(self.targetIQN, sr_uuid)
         IQNs = []
         if "multiSession" in self.dconf:
-            IQNs = ""
             for iqn in self.dconf['multiSession'].split("|"):
                 if len(iqn):
-                    IQNs += iqn.split(',')[2]
+                    IQNs.append(iqn.split(',')[2])
         else:
             IQNs.append(self.targetIQN)
 
@@ -429,7 +434,11 @@ def attach(self, sr_uuid):
                 realdev = os.path.realpath(os.path.join(dev_path, dev))
                 util.set_scheduler(os.path.basename(realdev))
 
-    def detach(self, sr_uuid, delete=False):
+    @override
+    def detach(self, sr_uuid) -> None:
+        self.detach_and_delete(sr_uuid, delete=False)
+
+    def detach_and_delete(self, sr_uuid, delete=True) -> None:
         keys = []
         pbdref = None
         try:
@@ -470,7 +479,8 @@ def detach(self, sr_uuid, delete=False):
 
         self.attached = False
 
-    def create(self, sr_uuid, size):
+    @override
+    def create(self, sr_uuid, size) -> None:
         # Check whether an SR already exists
         SRs = self.session.xenapi.SR.get_all_records()
         for sr in SRs:
@@ -499,11 +509,13 @@ def create(self, sr_uuid, size):
         self.session.xenapi.SR.set_sm_config(self.sr_ref, self.sm_config)
         return
 
-    def delete(self, sr_uuid):
+    @override
+    def delete(self, sr_uuid) -> None:
         self.detach(sr_uuid)
         return
 
-    def probe(self):
+    @override
+    def probe(self) -> str:
         SRs = self.session.xenapi.SR.get_all_records()
         Recs = {}
         for sr in SRs:
@@ -513,8 +525,9 @@ def probe(self):
                sm_config['targetIQN'] == self.targetIQN:
                 Recs[record["uuid"]] = sm_config
         return self.srlist_toxml(Recs)
-
-    def scan(self, sr_uuid):
+
+    @override
+    def scan(self, sr_uuid) -> None:
         if not self.passthrough:
             if not self.attached:
                 raise xs_errors.XenError('SRUnavailable')
@@ -526,9 +539,10 @@ def scan(self, sr_uuid):
                 if vdi.managed:
                     self.physical_utilisation += vdi.size
             self.virtual_allocation = self.physical_utilisation
-        return super(BaseISCSISR, self).scan(sr_uuid)
+        super(BaseISCSISR, self).scan(sr_uuid)
 
-    def vdi(self, uuid):
+    @override
+    def vdi(self, uuid) -> VDI.VDI:
         return LUNperVDI.RAWVDI(self, uuid)
 
     def _scan_IQNs(self):

drivers/CephFSSR.py

@@ -18,6 +18,8 @@
 #
 # CEPHFSSR: Based on FileSR, mounts ceph fs share
 
+from sm_typing import override
+
 import errno
 import os
 import socket
@@ -33,6 +35,7 @@
 import SRCommand
 import FileSR
 # end of careful
+import VDI
 import cleanup
 import util
 import vhdutil
@@ -83,13 +86,14 @@ class CephFSSR(FileSR.FileSR):
 
     DRIVER_TYPE = 'cephfs'
 
-    def handles(sr_type):
+    @override
+    @staticmethod
+    def handles(sr_type) -> bool:
         # fudge, because the parent class (FileSR) checks for smb to alter its behavior
         return sr_type == CephFSSR.DRIVER_TYPE or sr_type == 'smb'
 
-    handles = staticmethod(handles)
-
-    def load(self, sr_uuid):
+    @override
+    def load(self, sr_uuid) -> None:
         if not self._is_ceph_available():
             raise xs_errors.XenError(
                 'SRUnavailable',
@@ -180,7 +184,8 @@ def unmount(self, mountpoint, rmmountpoint):
             except OSError as inst:
                 raise CephFSException("rmdir failed with error '%s'" % inst.strerror)
 
-    def attach(self, sr_uuid):
+    @override
+    def attach(self, sr_uuid) -> None:
         if not self.checkmount():
             try:
                 self.mount()
@@ -189,18 +194,19 @@ def attach(self, sr_uuid):
                 raise xs_errors.SROSError(12, exc.errstr)
         self.attached = True
 
-    def probe(self):
+    @override
+    def probe(self) -> str:
         try:
             self.mount(PROBE_MOUNTPOINT)
             sr_list = filter(util.match_uuid, util.listdir(PROBE_MOUNTPOINT))
             self.unmount(PROBE_MOUNTPOINT, True)
         except (util.CommandException, xs_errors.XenError):
             raise
         # Create a dictionary from the SR uuids to feed SRtoXML()
-        sr_dict = {sr_uuid: {} for sr_uuid in sr_list}
-        return util.SRtoXML(sr_dict)
+        return util.SRtoXML({sr_uuid: {} for sr_uuid in sr_list})
 
-    def detach(self, sr_uuid):
+    @override
+    def detach(self, sr_uuid) -> None:
         if not self.checkmount():
             return
         util.SMlog("Aborting GC/coalesce")
@@ -211,7 +217,8 @@ def detach(self, sr_uuid):
         os.unlink(self.path)
         self.attached = False
 
-    def create(self, sr_uuid, size):
+    @override
+    def create(self, sr_uuid, size) -> None:
         if self.checkmount():
             raise xs_errors.SROSError(113, 'CephFS mount point already attached')
 
@@ -245,7 +252,8 @@ def create(self, sr_uuid, size):
                                            os.strerror(inst.code)))
         self.detach(sr_uuid)
 
-    def delete(self, sr_uuid):
+    @override
+    def delete(self, sr_uuid) -> None:
         # try to remove/delete non VDI contents first
         super(CephFSSR, self).delete(sr_uuid)
         try:
@@ -260,24 +268,26 @@ def delete(self, sr_uuid):
             if inst.code != errno.ENOENT:
                 raise xs_errors.SROSError(114, "Failed to remove CephFS mount point")
 
-    def vdi(self, uuid, loadLocked=False):
+    @override
+    def vdi(self, uuid, loadLocked=False) -> VDI.VDI:
         return CephFSFileVDI(self, uuid)
 
     @staticmethod
     def _is_ceph_available():
-        import distutils.spawn
-        return distutils.spawn.find_executable('ceph')
+        return util.find_executable('ceph')
 
 class CephFSFileVDI(FileSR.FileVDI):
-    def attach(self, sr_uuid, vdi_uuid):
+    @override
+    def attach(self, sr_uuid, vdi_uuid) -> str:
         if not hasattr(self, 'xenstore_data'):
             self.xenstore_data = {}
 
         self.xenstore_data['storage-type'] = CephFSSR.DRIVER_TYPE
 
         return super(CephFSFileVDI, self).attach(sr_uuid, vdi_uuid)
 
-    def generate_config(self, sr_uuid, vdi_uuid):
+    @override
+    def generate_config(self, sr_uuid, vdi_uuid) -> str:
         util.SMlog("SMBFileVDI.generate_config")
         if not util.pathexists(self.path):
             raise xs_errors.XenError('VDIUnavailable')
@@ -291,15 +301,16 @@ def generate_config(self, sr_uuid, vdi_uuid):
         config = xmlrpc.client.dumps(tuple([resp]), "vdi_attach_from_config")
         return xmlrpc.client.dumps((config,), "", True)
 
-    def attach_from_config(self, sr_uuid, vdi_uuid):
+    @override
+    def attach_from_config(self, sr_uuid, vdi_uuid) -> str:
         try:
             if not util.pathexists(self.sr.path):
-                self.sr.attach(sr_uuid)
+                return self.sr.attach(sr_uuid)
         except:
             util.logException("SMBFileVDI.attach_from_config")
             raise xs_errors.XenError('SRUnavailable',
                                      opterr='Unable to attach from config')
-
+        return ''
 
 if __name__ == '__main__':
     SRCommand.run(CephFSSR, DRIVER_INFO)