From 478c4447be20380c5b173f8e150d2373cf6d021b Mon Sep 17 00:00:00 2001
From: Alejandro Sirgo Rica <asirgo@soleta.eu>
Date: Wed, 14 Feb 2024 12:20:46 +0100
Subject: src: improve error check in image_create and image_restore

cover more error cases where exceptions need to be raised.
check return code in the invoked subprocess.

restoreImageCustom has been intentionally left behind, it
is unclear what this custom script returns on success and
error.
---
 src/live/ogOperations.py |  8 ++++++--
 src/utils/disk.py        |  2 +-
 src/utils/fs.py          | 20 +++++++++++++++++---
 src/utils/legacy.py      | 10 +++++-----
 4 files changed, 29 insertions(+), 11 deletions(-)

diff --git a/src/live/ogOperations.py b/src/live/ogOperations.py
index e19a788..b24a23c 100644
--- a/src/live/ogOperations.py
+++ b/src/live/ogOperations.py
@@ -441,7 +441,9 @@ class OgLiveOperations:
                 logging.error('No filesystem detected. Aborting image creation.')
                 raise ValueError('Target partition has no filesystem present')
 
-            cambiar_acceso(user=self._smb_user, pwd=self._smb_pass)
+            if change_access(user=self._smb_user, pwd=self._smb_pass) == -1:
+                logging.error('remount of /opt/opengnsys/images has failed')
+                raise AssertionError('remount of /opt/opengnsys/images has failed')
 
             if os.access(f'/opt/opengnsys/images', os.R_OK | os.W_OK) == False:
                 logging.error('Cannot access /opt/opengnsys/images in read and write mode, check permissions')
@@ -451,7 +453,9 @@ class OgLiveOperations:
                 logging.info(f'image file {image_path} already exists, updating.')
 
             ogCopyEfiBootLoader(disk, partition)
-            ogReduceFs(disk, partition)
+            if ogReduceFs(disk, partition) == -1:
+                logging.error('Failed to shrink filesystem')
+                raise ValueError('Failed to shrink filesystem')
 
             cmd1 = shlex.split(f'partclone.{fstype} -I -C --clone -s {padev} -O -')
             cmd2 = shlex.split(f'lzop -1 -fo {image_path}')
diff --git a/src/utils/disk.py b/src/utils/disk.py
index e26983c..1f1fdab 100644
--- a/src/utils/disk.py
+++ b/src/utils/disk.py
@@ -33,4 +33,4 @@ def get_partition_device(disknum, partnum):
         if pa.partno == partnum - 1:
             return cxt.partition_to_string(pa, fdisk.FDISK_FIELD_DEVICE)
 
-    raise ValueError('No such partition')
+    raise ValueError(f'No such partition with disk index {disknum} and partition index {partnum}')
diff --git a/src/utils/fs.py b/src/utils/fs.py
index c030e68..fbbbd3c 100644
--- a/src/utils/fs.py
+++ b/src/utils/fs.py
@@ -100,13 +100,14 @@ def ogReduceFs(disk, part):
 
     umount(partdev)
     if fstype == 'ext4':
-        _reduce_resize2fs(partdev)
+        ret = _reduce_resize2fs(partdev)
     elif fstype == 'ntfs':
-        _reduce_ntfsresize(partdev)
+        ret = _reduce_ntfsresize(partdev)
     else:
         logging.warn(f'Unable to shrink filesystem at {partdev}. '
                      f'Unsupported filesystem "{fstype}".')
 
+    return ret
 
 def ogExtendFs(disk, part):
     """
@@ -202,9 +203,13 @@ def get_filesystem_type(partdev):
 
 
 def _reduce_resize2fs(partdev):
+    ret = -1
     cmd = shlex.split(f'resize2fs -fpM {partdev}')
     with open('/tmp/command.log', 'ab', 0) as logfile:
-        subprocess.run(cmd, stdout=logfile, stderr=STDOUT)
+        proc = subprocess.run(cmd, stdout=logfile, stderr=STDOUT)
+        ret = proc.returncode
+
+    return 0 if ret == 0 else -1
 
 
 def _reduce_ntfsresize(partdev):
@@ -212,6 +217,14 @@ def _reduce_ntfsresize(partdev):
     proc_info = subprocess.run(cmd_info, stdout=subprocess.PIPE, encoding='utf-8')
     out_info = proc_info.stdout.strip()
 
+    if out_info.find('ERROR: NTFS is inconsistent. Run chkdsk') != -1:
+        logging.error('NTFS is inconsistent. Run chkdsk /f on Windows then reboot TWICE!')
+        return -1
+
+    if proc_info.returncode != 0:
+        logging.error(f'nfsresize {partdev} has failed with return code {proc_info.returncode}')
+        return -1
+
     # Process ntfsresize output directly.
     # The first split operation leaves the wanted data at the second element of
     # the split ([1]). Finally do a second split with ' ' to get the data but
@@ -241,6 +254,7 @@ def _reduce_ntfsresize(partdev):
         with open('/tmp/command.log', 'ab', 0) as logfile:
             subprocess.run(cmd_resize, input='y', stderr=STDOUT, encoding='utf-8')
 
+    return 0
 
 def _extend_resize2fs(partdev):
     cmd = shlex.split(f'resize2fs -f {partdev}')
diff --git a/src/utils/legacy.py b/src/utils/legacy.py
index 0f6c53c..a272e4c 100644
--- a/src/utils/legacy.py
+++ b/src/utils/legacy.py
@@ -130,7 +130,7 @@ def ogGetImageInfo(image_path):
     return image_info
 
 
-def cambiar_acceso(mode='rw', user='opengnsys', pwd='og'):
+def change_access(mode='rw', user='opengnsys', pwd='og'):
     """
     'CambiarAcceso' (admin/Interface/CambiarAcceso) rewrite into native Python.
 
@@ -138,14 +138,14 @@ def cambiar_acceso(mode='rw', user='opengnsys', pwd='og'):
     Specify access mode ('rw', or 'ro') with mode parameter (default 'rw').
     Specify samba credentials with user and pwd parameter.
 
-    Return True if exit-code was 0. Return False otherwise.
+    Return 0 if exit-code was 0. Return -1 otherwise.
     """
-    if mode not in ['rw', 'ro']:
-        raise ValueError('Invalid remount mode option')
+    assert mode in ['rw', 'ro'], 'Invalid remount mode option'
 
     cmd = shlex.split(f'mount -o remount,{mode},username={user},password={pwd} /opt/opengnsys/images')
     p = subprocess.run(cmd, stdout=DEVNULL, stderr=DEVNULL)
-    return p.returncode == 0
+
+    return 0 if p.returncode == 0 else -1
 
 def ogChangeRepo(ip, smb_user='opengnsys', smb_pass='og'):
     """
-- 
cgit v1.2.3-18-g5258