imgcreate/fs.py: abort on hard-unmount failure

Lazy unmounting was originally introduced  in commit 73bd680 ("Do
lazy umounts to reduce the impact of umount issues.", 2010-07-24) as a
work-around for RHBZ#617844 and RHBZ#509427.

Unfortunately, lazy unmounting and then proceeding as if no error had
occurred is actively harmful: combined with the lazy nature of "losetup
-d", it enables writes pending within the chroot to modify / corrupt the
filesystem while it's being checked & modified by e2fsck, and/or captured
into the ISO image.

We've seen actual examples of this:
- https://bugzilla.redhat.com/show_bug.cgi?id=2007045
- https://bugzilla.redhat.com/show_bug.cgi?id=2038105#c45
- #221

Papering over a hard-unmount failure is never right; in all such cases,
the leaked reference(s) to the chroot filesystem must be tracked down and
fixed -- potentially in packages that underlie livecd-tools, such as
"dnf". Otherwise, corrupt ISO images are built, and worse:
non-deterministically so, which makes people investigate in all the wrong
places.

Throw exceptions upon hard-unmount failures, and encourage users to report
all failures to unmount in the upstream bug tracker. Preserve lazy unmount
only as a last-resort *cleanup* action, so that the user is not left with
a bunch of unwanted mounts *AFTER* the process using imgcreate exits
(i.e., when the leaked references are cleaned up by the kernel
automatically).

Signed-off-by: Laszlo Ersek <lersek@redhat.com>

imgcreate/fs.py

@@ -35,6 +35,12 @@
 from imgcreate.util import *
 from imgcreate.errors import *
 
+umount_fail_fmt = ("Unable to unmount filesystem at %s. The process using the "
+                   "imgcreate module, or one of the libraries used by that "
+                   "process, leaked a reference to the filesystem. Please "
+                   "report a bug at "
+                   "<https://github.com/livecd-tools/livecd-tools/issues>.")
+
 def chrootentitycheck(entity, chrootdir):
     """Check for entity availability in the chroot image.
 
@@ -815,22 +821,10 @@ def unmount(self):
         if self.mounted:
             logging.info("Unmounting directory %s" % self.mountdir)
             rc = call(['umount', self.mountdir])
-            if rc == 0:
-                self.mounted = False
-            else:
-                logging.warning("Unmounting directory %s failed, using lazy "
-                             "umount" % self.mountdir)
-                print("Unmounting directory %s failed, using lazy umount" %
-                      self.mountdir, file=sys.stdout)
-                rc = call(['umount', '-l', self.mountdir])
-                if rc != 0:
-                    raise MountError("Unable to unmount filesystem at %s" %
-                                     self.mountdir)
-                else:
-                    logging.info("lazy umount succeeded on %s" % self.mountdir)
-                    print("lazy umount succeeded on %s" % self.mountdir,
-                          file=sys.stdout)
-                    self.mounted = False
+            if rc != 0:
+                call(['umount', '-l', self.mountdir])
+                raise MountError(umount_fail_fmt % self.mountdir)
+            self.mounted = False
 
         if self.rmdir and not self.mounted:
             try:
@@ -989,15 +983,8 @@ def unmount(self):
 
         rc = call(['umount', self.mountdir])
         if rc != 0:
-            logging.info("Unable to unmount %s normally, using lazy unmount" %
-                         self.mountdir)
-            rc = call(['umount', '-l', self.mountdir])
-            if rc != 0:
-                raise MountError("Unable to unmount fs at %s" % self.mountdir)
-            else:
-                logging.info("lazy umount succeeded on %s" % self.mountdir)
-                print("lazy umount succeeded on '%s'" % (self.mountdir),
-                      sys.stdout)
+            call(['umount', '-l', self.mountdir])
+            raise MountError(umount_fail_fmt % self.mountdir)
         if self.cowmnt:
             self.cowmnt.unmount()
         self.imgmnt.unmount()
@@ -1072,16 +1059,8 @@ def unmount(self):
 
         rc = call(['umount', self.dest])
         if rc != 0:
-            logging.info("Unable to unmount %s normally, using lazy unmount" %
-                         self.dest)
-            rc = call(['umount', '-l', self.dest])
-            if rc != 0:
-                raise MountError("Unable to unmount fs at %s" % self.dest)
-            else:
-                logging.info("lazy umount succeeded on %s" % self.dest)
-                print("lazy umount succeeded on %s" % self.dest,
-                      file=sys.stdout)
-
+            call(['umount', '-l', self.dest])
+            raise MountError(umount_fail_fmt % self.dest)
         self.mounted = False
 
     def cleanup(self):