Better names, comments and a small but elegant optimization.

database_backup_path instead of database_backup_file because it is a str|Path.
Throttling delay converted to Constant.

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

Author: Ythogtha

drivers/LinstorSR.py

@@ -16,7 +16,7 @@
 
 from sm_typing import Any, Optional, override, Literal
 
-from constants import CBTLOG_TAG
+from constants import CBTLOG_TAG, LINSTOR_AUTO_BACKUP_DELAY
 
 try:
     from linstorcowutil import LinstorCowUtil, MultiLinstorCowUtil
@@ -824,15 +824,15 @@ def check_sr(self, sr_uuid) -> None:
         # Applied only on the Linstor Controller, for reasons -> listed below.
         if not LinstorVolumeManager.is_controller():
             return
-        # Start database invalidation.
+        # Validate and clean previous backups if necessary.
         # -> Needs access to backup files, available only on the Controller.
         LinstorVolumeManager.database_invalidation()
         # check_sr is launched on *all* hosts, but it turns out that
         # we do not want all of them to blindly generate concurrencing backups.
         # Hence we must choose one, either one is good, but there must be only one.
         # Apply throttling: only backup if last one is >1h old.
         # -> Needs access to backup files, available only on the Controller.
-        if LinstorVolumeManager.database_backup_age() > 3600:
+        if LinstorVolumeManager.get_database_backup_age() > LINSTOR_AUTO_BACKUP_DELAY:
             self.database_backup("auto")
 
     @override

drivers/constants.py

@@ -11,3 +11,5 @@
 # Ref counting for VDI's: we need a ref count for LV activation/deactivation
 # on the master.
 NS_PREFIX_LVM: Final = "lvm-"
+
+LINSTOR_AUTO_BACKUP_DELAY = 3600

drivers/linstorvolumemanager.py

@@ -1789,7 +1789,7 @@ def is_controller(cls):
         return cls._is_mounted(DATABASE_PATH)
 
     @classmethod
-    def database_backup_age(cls):
+    def get_database_backup_age(cls):
         """
         Return the latest backup age in seconds.
         If not called on the Controller, since backups are not available,
@@ -1817,17 +1817,17 @@ def database_invalidation(cls):
         for directory in (DATABASE_BACKUP_DIR_MAIN, DATABASE_BACKUP_DIR_SPARE):
             valid_backup_count = 0
             # Validate file and apply retention
-            for database_backup_file, _ in cls._get_sorted_database_backup(directory):
+            for database_backup_path, _ in cls._get_sorted_database_backup(directory):
                 try:
-                    cls._check_database_backup(database_backup_file)
+                    cls._check_database_backup(database_backup_path)
                     valid_backup_count += 1
                     if valid_backup_count < DATABASE_BACKUP_RETENTION:
                         continue
                 except LinstorDatabaseBackupError as error:
-                    util.SMlog(f"[database_backup] Check failed `{error}` [{database_backup_file}]",
+                    util.SMlog(f"[database_backup] Check failed `{error}` [{database_backup_path}]",
                                priority=util.LOG_ERR)
                 with contextlib.suppress(OSError):
-                    os.unlink(database_backup_file)
+                    os.unlink(database_backup_path)
 
     @classmethod
     def get_all_group_names(cls, base_name):
@@ -2707,7 +2707,7 @@ def _get_volume_properties(self, volume_uuid):
         return properties
 
     @classmethod
-    def _list_database_backup(cls, database_backup_dir):
+    def _list_database_backups(cls, database_backup_dir):
         """
         List all visible backup files in database_backup_dir.
         DATABASE_BACKUP_DIR_MAIN is only available on the Linstor Controller.
@@ -2727,7 +2727,7 @@ def _get_sorted_database_backup(cls, database_backup_dir):
         Return list of backups in database_backup_dir, alongside their creation date.
         Sorted by date from the more recent to the older one.
         """
-        return sorted(cls._list_database_backup(database_backup_dir),
+        return sorted(cls._list_database_backups(database_backup_dir),
                       reverse=True,
                       key=lambda p: p[1])
 
@@ -2737,28 +2737,31 @@ def _get_latest_database_backup(cls):
         Return the latest backup in DATABASE_BACKUP_DIR_MAIN, and its creation date.
         Returns (None, timestamp(0)) when none are found.
         None will be found if it is not called on the Linstor Controller.
-        (cf _list_database_backup)
+        (cf _list_database_backups)
         """
-        return max(cls._list_database_backup(DATABASE_BACKUP_DIR_MAIN),
+        return max(cls._list_database_backups(DATABASE_BACKUP_DIR_MAIN),
                    default=(None, datetime.fromtimestamp(0)),
                    key=lambda p: p[1])
 
     @classmethod
-    def _check_database_backup(cls, database_backup_file):
+    def _check_database_backup(cls, database_backup_path):
         """
         Make some validation of a database backup zip-file.
         Check its a valid zipfile, and CRC-test its content.
         Check it contains a non-empty linstordb.mv.db file.
         Always raises a LinstorDatabaseBackupError if checks failed.
         """
         try:
-            with zipfile.ZipFile(database_backup_file, mode="r") as archive:
+            with zipfile.ZipFile(database_backup_path, mode="r") as archive:
                 if archive.testzip() is not None:
                     raise LinstorDatabaseBackupError("zip archive CRC failed")
-                linstordb = [f for f in archive.filelist if f.filename == "linstordb.mv.db"]
+                linstordb = next((
+                    f
+                    for f in archive.filelist
+                    if f.filename == "linstordb.mv.db"
+                ), None)
                 if not linstordb:
                     raise LinstorDatabaseBackupError("cannot find linstordb.mv.db")
-                linstordb = linstordb[0]
                 if linstordb.file_size == 0:
                     raise LinstorDatabaseBackupError("linstordb.mv.db is empty")
         except (FileNotFoundError, zipfile.BadZipFile, zipfile.LargeZipFile) as e: