From 1b2ec5f86133679c7d1111ae3b4f046777426fde Mon Sep 17 00:00:00 2001
From: Nikos Skalkotos <skalkoto@grnet.gr>
Date: Thu, 26 Jun 2014 18:15:27 +0300
Subject: [PATCH] Add Mount context manager in OSBase cls

Remove mount and unmount methods and add a mount context manager for
mounting the media file systems.
---
 image_creator/os_type/__init__.py         | 88 ++++++++++-------------
 image_creator/os_type/freebsd.py          |  2 +-
 image_creator/os_type/unix.py             |  2 +-
 image_creator/os_type/windows/__init__.py | 33 ++-------
 4 files changed, 46 insertions(+), 79 deletions(-)

diff --git a/image_creator/os_type/__init__.py b/image_creator/os_type/__init__.py
index bd3ae62..e38c135 100644
--- a/image_creator/os_type/__init__.py
+++ b/image_creator/os_type/__init__.py
@@ -208,10 +208,10 @@ class OSBase(object):
                                      "`%s'. Reason: %s" % (key, param.error))
 
         self.meta = {}
-        self.mounted = False
 
         # This will host the error if mount fails
         self._mount_error = ""
+        self._mount_warnings = []
 
         # Many guestfs compilations don't support scrub
         self._scrub_support = True
@@ -227,27 +227,19 @@ class OSBase(object):
             return
 
         self.out.output('Running OS inspection:')
-        try:
-            if not self.mount(readonly=True, silent=True):
-                raise FatalError("Unable to mount the media read-only")
+        with self.mount(readonly=True, silent=True):
             self._do_inspect()
-        finally:
-            self.umount(silent=True)
-
         self.out.output()
 
     def collect_metadata(self):
         """Collect metadata about the OS"""
-        try:
-            if not self.mount(readonly=True, silent=True):
-                raise FatalError("Unable to mount the media read-only")
 
-            self.out.output('Collecting image metadata ...', False)
+        self.out.output('Collecting image metadata ...', False)
+
+        with self.mount(readonly=True, silent=True):
             self._do_collect_metadata()
-            self.out.success('done')
-        finally:
-            self.umount(silent=True)
 
+        self.out.success('done')
         self.out.output()
 
     def list_syspreps(self):
@@ -352,12 +344,7 @@ class OSBase(object):
                 "System preparation is disabled for unsupported media")
             return
 
-        try:
-            if not self.mount(readonly=False):
-                msg = "Unable to mount the media read-write. Reason: %s" % \
-                    self._mount_error
-                raise FatalError(msg)
-
+        with self.mount():
             enabled = [task for task in self.list_syspreps() if task.enabled]
 
             size = len(enabled)
@@ -367,39 +354,40 @@ class OSBase(object):
                 self.out.output(('(%d/%d)' % (cnt, size)).ljust(7), False)
                 task()
                 setattr(task.im_func, 'executed', True)
-        finally:
-            self.umount()
 
         self.out.output()
 
-    def mount(self, readonly=False, silent=False):
-        """Mount image."""
-
-        if getattr(self, "mounted", False):
-            return True
-
-        mount_type = 'read-only' if readonly else 'read-write'
-        if not silent:
-            self.out.output("Mounting the media %s ..." % mount_type, False)
-
-        self._mount_error = ""
-        if not self._do_mount(readonly):
-            return False
-
-        self.mounted = True
-        if not silent:
-            self.out.success('done')
-        return True
-
-    def umount(self, silent=False):
-        """Umount all mounted file systems."""
-
-        if not silent:
-            self.out.output("Umounting the media ...", False)
-        self.image.g.umount_all()
-        self.mounted = False
-        if not silent:
-            self.out.success('done')
+    def mount(self, readonly=False, silent=False, fatal=True):
+        """Returns a context manager for mounting an image"""
+
+        parent = self
+        output = lambda msg='', nl=True: None if silent else self.out.output
+        success = lambda msg='', nl=True: None if silent else self.out.success
+        warn = lambda msg='', nl=True: None if silent else self.out.warn
+
+        class Mount:
+            """The Mount context manager"""
+            def __enter__(self):
+                mount_type = 'read-only' if readonly else 'read-write'
+                output("Mounting the media %s ..." % mount_type, False)
+
+                parent._mount_error = ""
+                del parent._mount_warnings[:]
+
+                if not parent._do_mount(readonly) and fatal:
+                    msg = "Unable to mount the media %s. Reason: %s" % \
+                        (mount_type, parent._mount_error)
+                    raise FatalError(msg)
+                for warning in parent._mount_warnings:
+                    warn(warning)
+                success('done')
+
+            def __exit__(self, exc_type, exc_value, traceback):
+                output("Umounting the media ...", False)
+                parent.image.g.umount_all()
+                success('done')
+
+        return Mount()
 
     def check_version(self, major, minor):
         """Checks the OS version against the one specified by the major, minor
diff --git a/image_creator/os_type/freebsd.py b/image_creator/os_type/freebsd.py
index d8df923..426e974 100644
--- a/image_creator/os_type/freebsd.py
+++ b/image_creator/os_type/freebsd.py
@@ -121,7 +121,7 @@ class Freebsd(Unix):
                     self._mount_error = str(msg)
                     return False
                 else:
-                    self.out.warn('%s (ignored)' % msg)
+                    self._mount_warnings.append('%s (ignored)' % msg)
 
         return True
 
diff --git a/image_creator/os_type/unix.py b/image_creator/os_type/unix.py
index 00ede28..ac5e342 100644
--- a/image_creator/os_type/unix.py
+++ b/image_creator/os_type/unix.py
@@ -66,7 +66,7 @@ class Unix(OSBase):
                     self._mount_error = str(msg)
                     return False
                 else:
-                    self.out.warn('%s (ignored)' % msg)
+                    self._mount_warnings.append('%s (ignored)' % msg)
 
         return True
 
diff --git a/image_creator/os_type/windows/__init__.py b/image_creator/os_type/windows/__init__.py
index f93e0b5..d02f33a 100644
--- a/image_creator/os_type/windows/__init__.py
+++ b/image_creator/os_type/windows/__init__.py
@@ -173,14 +173,11 @@ class Windows(OSBase):
         self.registry = Registry(self.image.g, self.root)
 
         # If the image is already sysprepped we cannot further customize it
-        self.mount(readonly=True, silent=True)
-        try:
+        with self.mount(readonly=True, silent=True):
             self.out.output("Checking media state ...", False)
             self.sysprepped = self.registry.get_setup_state() > 0
             self.virtio_state = self._virtio_state()
             self.out.success("done")
-        finally:
-            self.umount(silent=True)
 
         # If the image is sysprepped no driver mappings will be present.
         self.systemdrive = None
@@ -374,12 +371,8 @@ class Windows(OSBase):
         shutdown_timeout = self.sysprep_params['shutdown_timeout'].value
 
         self.out.output("Preparing media for boot ...", False)
-        try:
-            if not self.mount(readonly=False, silent=True):
-                msg = "Unable to mount the media read-write. Reason: %s" % \
-                    self._mount_error
-                raise FatalError(msg)
 
+        with self.mount(readonly=False, silent=True):
             v_val = self.registry.reset_passwd(admin)
             disabled_uac = self.registry.update_uac_remote_setting(1)
             token = self._add_boot_scripts()
@@ -394,8 +387,6 @@ class Windows(OSBase):
             except RuntimeError:
                 pass
 
-        finally:
-            self.umount(silent=True)
         self.out.success('done')
 
         self.image.disable_guestfs()
@@ -433,8 +424,7 @@ class Windows(OSBase):
             self.image.enable_guestfs()
 
             self.out.output("Reverting media boot preparations ...", False)
-            self.mount(readonly=False, silent=True)
-            try:
+            with self.mount(readonly=False, silent=True, fatal=False):
                 if disabled_uac:
                     self.registry.update_uac_remote_setting(0)
 
@@ -444,8 +434,6 @@ class Windows(OSBase):
                     self.registry.reset_passwd(admin, v_val)
 
                 self.registry.update_firewalls(*firewall_states)
-            finally:
-                self.umount(silent=True)
             self.out.success("done")
 
     def _exec_sysprep_tasks(self):
@@ -618,11 +606,7 @@ class Windows(OSBase):
 
         self.out.output('Installing virtio drivers...', False)
 
-        try:
-            if not self.mount(readonly=False, silent=True):
-                msg = "Unable to mount the media read-write. Reason: %s" % \
-                    self._mount_error
-                raise FatalError(msg)
+        with self.mount(readonly=False, silent=True):
 
             admin = self.sysprep_params['admin'].value
             v_val = self.registry.reset_passwd(admin)
@@ -646,20 +630,15 @@ class Windows(OSBase):
 
             self.registry.runonce({'InstallDrivers': cmd})
 
-        finally:
-            self.umount(silent=True)
-
         try:
             self.vm.start()
             self.out.success("started (console on VNC display: %d)" %
                                  self.vm.display)
         finally:
             self.vm.stop(6000)
-            try:
-                self.mount(readonly=True, silent=True)
+
+            with self.mount(readonly=True, silent=True):
                 self.virtio_state = self._virtio_state()
-            finally:
-                self.umount(silent=True)
 
 
     def _install_viostor_driver(self, dirname):
-- 
GitLab