From 093f1125d9f51e167694e95b1d2824d29d39eaad Mon Sep 17 00:00:00 2001
From: Peter Senna Tschudin <peter.senna@collabora.com>
Date: Wed, 26 Jun 2019 15:55:55 +0200
Subject: [PATCH] psdk: Code cleanup

This patch removes code that was not used, simplify the Progressbar
class, and add comments to document corner cases.

Signed-off-by: Peter Senna Tschudin <peter.senna@collabora.com>
---
 tools/psdk | 133 +++++++----------------------------------------------
 1 file changed, 16 insertions(+), 117 deletions(-)

diff --git a/tools/psdk b/tools/psdk
index 177810f..d99009e 100755
--- a/tools/psdk
+++ b/tools/psdk
@@ -46,11 +46,6 @@ class ConfigurationAlreadyPersistent(Exception):
     """Exception for configuration files that are already stored on persistent storage"""
     pass
 
-class ProcessStillRunning(Exception):
-    """Exception for signaling that the process is still running and that requested information is
-    not yet available."""
-    pass
-
 class Disk:
     """Persistent a persistent disk with a single partition for /home filesystem. There are two
        special directories:
@@ -223,7 +218,9 @@ class Disk:
         self.progressbar = Progressbar("Disk initialization", "Starting...")
 
 class Disks:
-    """Get a list of disks from lsblk and compute some statistics"""
+    """Get a list of disks from lsblk and compute some statistics. The assumption is that there is
+only one level of nesting like disk -> partition. In case of dm-crypt devices and LVM volumes this
+would not be the case and the code would need to recursively look for the mount points"""
 
     def __init__(self, zenity):
         """Initializer"""
@@ -347,81 +344,37 @@ class Disks:
             if self.zenity:
                 self.pdisk.enable_zenity()
 
-    def probe_disks(self):
-        """Clear attributes and call _init() again"""
-
-        self.jdisks = None
-        self.count = 0
-        self.disks = []
-        self.empty_disks = []
-        self.psdk_disks = []
-        self.psdk_mounted_disks = []
-        self.pdisk = None
-
-        self._init()
-
-def pid_running(pid):
-    """Check if pid is running"""
-    try:
-        os.kill(pid, 0)
-    except OSError:
-        return False
-    else:
-        return True
-
 class Progressbar():
     """A convenient way to use Zenity progress bar"""
 
     def __init__(self, title, text):
         """Initializer"""
 
-        self.read, self.write = os.pipe()
-        self.title = title
-        self.text = text
-        self.zenity_pid = None # No, not really. This is the pid of the child which calls zenity
-        self.zenity_result = None
+        self.zproc = None
         self.command = ["zenity", "--progress", "--percentage", "0", "--auto-close", "--text",
                         text, "--title", title, "--width=360", "--no-cancel"]
 
     def create_window(self):
-        """Fork and call Zinity"""
+        """Call Zenity"""
 
-        self.zenity_pid = os.fork()
-        if self.zenity_pid:
-            # parent
-            os.close(self.read)
-            return
-
-        else:
-            # child
-            os.close(self.write)
-            self.zenity_result = subprocess.run(self.command, stdin=self.read)
-            sys.exit(0)
+        self.zproc = subprocess.Popen(self.command, stdin=subprocess.PIPE, bufsize=0)
 
     def set_text(self, text):
         """Update the progress bar description text"""
 
         text = "# " + text + "\n"
-        os.write(self.write, text.encode('utf8'))
+        self.zproc.stdin.write(text.encode('utf8'))
 
     def set_percentage(self, percentage):
-        """Update the progress bar description text"""
+        """Update the progress bar percentage"""
 
         text = str(percentage) + "\n"
-        os.write(self.write, text.encode('utf8'))
-
-    def returncode(self):
-        """Return code of Zenity"""
-
-        if pid_running(self.zenity_pid):
-            raise ProcessStillRunning()
-
-        return self.zenity_result.returncode
+        self.zproc.stdin.write(text.encode('utf8'))
 
     def wait(self):
         """Wait until zenity child returns"""
 
-        os.waitpid(self.zenity_pid, 0)
+        self.zproc.wait()
 
 def fstab_updated():
     """Check if fstab was configured to use persistent workspace disk
@@ -582,9 +535,12 @@ def check_configuration_file(conf_file):
     return conf_file_abspath
 
 def conf_to_persistent(conf_file, alt_mount=None):
-    """Create a backup copy of a configuration file, move it to the persistent
-    folder, and create a symlink from persistent folder to /etc. This function
-    will intentionally fail if receives a directory as parameter"""
+    """Create a backup copy of a configuration file, move it to the persistent folder, and create a
+symlink from persistent folder to /etc. This function will intentionally fail if receives a
+directory as parameter. alt_mount is used during the initialization when the persistent disk mount point
+is different from what it will be when the persistent disk is actually in use. This is relevant for
+the creation of symlinks. This function need to handle moving configuration files to persistent
+storage with the persistent disk mounted in both locations."""
 
     conf_file_abspath = check_configuration_file(conf_file)
 
@@ -748,63 +704,6 @@ def zmenu(disks=None):
 
             return True
 
-def menu(disks=None):
-    """Menu for user interaction"""
-
-    print("Persistent disk setup")
-    print("\nFor help visit:\n\t" + HELP_LINK)
-    print("\nStatus:")
-
-    if disks.psdk_mounted_disks:
-        print("\tPersistent disk in use: {}".format(str(disks.psdk_mounted_disks[0])))
-        print("\nThis SDK is currently using the persistent disk. No further action required.")
-        input("\nPress enter to exit")
-        return False
-
-    if fstab_updated():
-        print("\nThis SDK was already configured to use persistent workspace disk. " + \
-              "You may need to reboot your system.")
-        input("\nPress enter to exit")
-        return False
-
-    if (not disks.psdk_disks) and (not disks.empty_disks):
-        print("\nNo disk found to use as persistent storage.")
-        input("\nPress enter to exit")
-        return False
-
-    if disks.empty_disks:
-        empty_disk_found = disks.empty_disks[0]
-        print("\tEmpty disk found: {}".format(str(empty_disk_found)))
-        answer = input("\nDo you want to initialize the empty disk {} ".format(empty_disk_found) +
-                       "and configure the SDK to use it as persistent disk?\n" +
-                       "(yes and press enter): ")
-
-        if answer == "yes":
-            disks.pdisk.initialize()
-            configure_sdk()
-
-            input("\nDisk initialization completed. Press enter to reboot the SDK.")
-            command = ["sudo", "reboot"]
-            _run(command)
-
-            return True
-
-    if disks.psdk_disks:
-        pdisk_found = disks.psdk_disks[0]
-        print("\tPersistent disk found: {}".format(str(pdisk_found)))
-        answer = input("\nDo you want to configure the SDK to use the persistent " +
-                       "disk {}?\n".format(pdisk_found) +
-                       "(yes and press enter): ")
-
-        if answer == "yes":
-            configure_sdk()
-
-            input("\nSDK configuration completed. Press enter to reboot the SDK.")
-            command = ["sudo", "reboot"]
-            _run(command)
-
-            return True
-
 def is_json(myjson):
     """Check if input is valid json"""
 
-- 
GitLab