From e19437290d9b442789164d72bbfd55e35fef6e0b Mon Sep 17 00:00:00 2001
From: Alejandro Sirgo Rica <asirgo@soleta.eu>
Date: Wed, 3 Apr 2024 11:09:23 +0200
Subject: live: improve exception handling in image_create

Reduce the scope of the try except block that controls the case
of deleting the image backup in case of error. Now it only covers
the section of code after backup creation and up to image
verification. Check when the Exception is an OgError to raise
with added context.

Prevent the deletion of the target image in case of error before
the backup creation.

Bundle the backup creation on its own try except block to give
more feedback on a failed backup creation.

Enables a better error management allowing unhandled
exceptions to be reported properly.
---
 src/live/ogOperations.py | 61 +++++++++++++++++++++++++++---------------------
 1 file changed, 34 insertions(+), 27 deletions(-)

diff --git a/src/live/ogOperations.py b/src/live/ogOperations.py
index 02af011..41cf4b0 100644
--- a/src/live/ogOperations.py
+++ b/src/live/ogOperations.py
@@ -421,45 +421,48 @@ class OgLiveOperations:
         if ogRest.terminated:
             return
 
-        try:
-            diskname = get_disks()[disk-1]
-            cxt = fdisk.Context(f'/dev/{diskname}', details=True)
-            pa = None
+        diskname = get_disks()[disk-1]
+        cxt = fdisk.Context(f'/dev/{diskname}', details=True)
+        pa = None
 
-            for i, p in enumerate(cxt.partitions):
-                if (p.partno + 1) == partition:
-                    pa = cxt.partitions[i]
+        for i, p in enumerate(cxt.partitions):
+            if (p.partno + 1) == partition:
+                pa = cxt.partitions[i]
 
-            if pa is None:
-                self._restartBrowser(self._url)
-                raise OgError(f'Target partition /dev/{diskname} not found')
+        if pa is None:
+            self._restartBrowser(self._url)
+            raise OgError(f'Target partition /dev/{diskname} not found')
 
-            padev = cxt.partition_to_string(pa, fdisk.FDISK_FIELD_DEVICE)
-            fstype = cxt.partition_to_string(pa, fdisk.FDISK_FIELD_FSTYPE)
-            if not fstype:
-                raise OgError(f'No filesystem detected in {padev}. Aborting image creation')
+        padev = cxt.partition_to_string(pa, fdisk.FDISK_FIELD_DEVICE)
+        fstype = cxt.partition_to_string(pa, fdisk.FDISK_FIELD_FSTYPE)
+        if not fstype:
+            raise OgError(f'No filesystem detected in {padev}. Aborting image creation')
 
-            if change_access(user=self._smb_user, pwd=self._smb_pass) == -1:
-                raise OgError('remount of /opt/opengnsys/images has failed')
+        if change_access(user=self._smb_user, pwd=self._smb_pass) == -1:
+            raise OgError('remount of /opt/opengnsys/images has failed')
 
-            if os.access(f'/opt/opengnsys/images', os.R_OK | os.W_OK) == False:
-                raise OgError('Cannot access /opt/opengnsys/images in read and write mode, check permissions')
+        if os.access(f'/opt/opengnsys/images', os.R_OK | os.W_OK) == False:
+            raise OgError('Cannot access /opt/opengnsys/images in read and write mode, check permissions')
 
-            if os.access(f'{image_path}', os.R_OK) == True:
-                logging.info(f'image file {image_path} already exists, updating.')
+        if os.access(f'{image_path}', os.R_OK) == True:
+            logging.info(f'image file {image_path} already exists, updating.')
 
-            copy_windows_efi_bootloader(disk, partition)
-            if ogReduceFs(disk, partition) == -1:
-                raise OgError(f'Failed to shrink {fstype} filesystem in {padev}')
+        copy_windows_efi_bootloader(disk, partition)
+        if ogReduceFs(disk, partition) == -1:
+            raise OgError(f'Failed to shrink {fstype} filesystem in {padev}')
 
-            cmd1 = shlex.split(f'partclone.{fstype} -I -C --clone -s {padev} -O -')
-            cmd2 = shlex.split(f'lzop -1 -fo {image_path}')
+        cmd1 = shlex.split(f'partclone.{fstype} -I -C --clone -s {padev} -O -')
+        cmd2 = shlex.split(f'lzop -1 -fo {image_path}')
 
-            logfile = open('/tmp/command.log', 'wb', 0)
+        logfile = open('/tmp/command.log', 'wb', 0)
 
+        try:
             if os.path.exists(image_path) and backup:
                 shutil.move(image_path, f'{image_path}.ant')
+        except OSError as e:
+            raise OgError(f'Cannot create backup for {image_path}: {e}') from e
 
+        try:
             p1 = Popen(cmd1, stdout=PIPE, stderr=logfile)
             p2 = Popen(cmd2, stdin=p1.stdout)
             p1.stdout.close()
@@ -493,7 +496,11 @@ class OgLiveOperations:
                 shutil.move(f'{image_path}.ant', image_path)
 
             self._restartBrowser(self._url)
-            raise OgError(f'Failed to create image for {fstype} filesystem in device {padev}: {e}') from e
+
+            if isinstance(e, OgError):
+                raise OgError(f'Failed to create image for {fstype} filesystem in device {padev}: {e}') from e
+            else:
+                raise
 
         try:
             st = os.stat(image_path)
-- 
cgit v1.2.3-18-g5258