Move most of the parseFSTab logic into its own function.

The purpose of doing this is so we can start to get a better handle on what
line causes an error, so the user has some way to tell what they need to change
in their /etc/fstab to make it work.

2 files changed, 81 insertions, 69 deletions

diff --git a/storage/__init__.py b/storage/__init__.py
index 9532f8830..4f71125f6 100644
--- a/storage/__init__.py
+++ b/storage/__init__.py
@@ -1213,6 +1213,77 @@ class FSSet(object):
                 filesystems[device.format.mountpoint] = device
         return filesystems
 
+    def _parseOneLine(self, (devspec, mountpoint, fstype, options, dump, passno)):
+        # find device in the tree
+        device = resolveDevice(self.devicetree,
+                               devspec,
+                               cryptTab=self.cryptTab,
+                               blkidTab=self.blkidTab)
+        if device:
+            # fall through to the bottom of this block
+            pass
+        elif devspec.startswith("/dev/loop"):
+            # FIXME: create devices.LoopDevice
+            log.warning("completely ignoring your loop mount")
+        elif ":" in devspec:
+            # NFS -- preserve but otherwise ignore
+            device = NFSDevice(devspec,
+                               format=getFormat(fstype,
+                                                device=devspec),
+                               exists=True)
+        elif devspec.startswith("/") and fstype == "swap":
+            # swap file
+            device = FileDevice(devspec,
+                                parents=get_containing_device(devspec),
+                                format=getFormat(fstype,
+                                                 device=devspec,
+                                                 exists=True),
+                                exists=True)
+        elif fstype == "bind" or "bind" in options:
+            # bind mount... set fstype so later comparison won't
+            # turn up false positives
+            fstype = "bind"
+            device = FileDevice(devspec,
+                                parents=get_containing_device(devspec),
+                                exists=True)
+            device.format = getFormat("bind",
+                                      device=device.path,
+                                      exists=True)
+        elif mountpoint in ("/proc", "/sys", "/dev/shm", "/dev/pts"):
+            # drop these now -- we'll recreate later
+            return None
+        else:
+            # nodev filesystem -- preserve or drop completely?
+            format = getFormat(fstype)
+            if isinstance(format, get_device_format_class("nodev")):
+                device = NoDevice(format)
+            else:
+                device = Device(devspec)
+
+        if device is None:
+            log.error("failed to resolve %s (%s) from fstab" % (devspec,
+                                                                fstype))
+            return None
+
+        # make sure, if we're using a device from the tree, that
+        # the device's format we found matches what's in the fstab
+        fmt = getFormat(fstype, device=device.path)
+        if fmt.type != device.format.type:
+            log.warning("scanned format (%s) differs from fstab "
+                        "format (%s)" % (device.format.type, fstype))
+
+        if device.format.mountable:
+            device.format.mountpoint = mountpoint
+            device.format.mountopts = options
+
+        # is this useful?
+        try:
+            device.format.options = options
+        except AttributeError:
+            pass
+
+        return device
+
     def parseFSTab(self, chroot=""):
         """ parse /etc/fstab
 
@@ -1276,74 +1347,14 @@ class FSSet(object):
 
                 (devspec, mountpoint, fstype, options, dump, passno) = fields
 
-                # find device in the tree
-                device = resolveDevice(self.devicetree,
-                                       devspec,
-                                       cryptTab=cryptTab,
-                                       blkidTab=blkidTab)
-                if device:
-                    # fall through to the bottom of this block
-                    pass
-                elif devspec.startswith("/dev/loop"):
-                    # FIXME: create devices.LoopDevice
-                    log.warning("completely ignoring your loop mount")
-                elif ":" in devspec:
-                    # NFS -- preserve but otherwise ignore
-                    device = NFSDevice(devspec,
-                                       format=getFormat(fstype,
-                                                        device=devspec),
-                                       exists=True)
-                elif devspec.startswith("/") and fstype == "swap":
-                    # swap file
-                    device = FileDevice(devspec,
-                                        parents=get_containing_device(devspec),
-                                        format=getFormat(fstype,
-                                                         device=devspec,
-                                                         exists=True),
-                                        exists=True)
-                elif fstype == "bind" or "bind" in options:
-                    # bind mount... set fstype so later comparison won't
-                    # turn up false positives
-                    fstype = "bind"
-                    device = FileDevice(devspec,
-                                        parents=get_containing_device(devspec),
-                                        exists=True)
-                    device.format = getFormat("bind",
-                                              device=device.path,
-                                              exists=True)
-                elif mountpoint in ("/proc", "/sys", "/dev/shm", "/dev/pts"):
-                    # drop these now -- we'll recreate later
-                    continue
-                else:
-                    # nodev filesystem -- preserve or drop completely?
-                    format = getFormat(fstype)
-                    if isinstance(format, get_device_format_class("nodev")):
-                        device = NoDevice(format)
-                    else:
-                        device = Device(devspec)
+                try:
+                    device = self._parseOneLine((devspec, mountpoint, fstype, options, dump, passno))
+                except Exception as e:
+                    raise Exception("fstab entry %s is malformed: %s" % (devspec, e))
 
-                if device is None:
-                    log.error("failed to resolve %s (%s) from fstab" % (devspec,
-                                                                        fstype))
+                if not device:
                     continue
 
-                # make sure, if we're using a device from the tree, that
-                # the device's format we found matches what's in the fstab
-                fmt = getFormat(fstype, device=device.path)
-                if fmt.type != device.format.type:
-                    log.warning("scanned format (%s) differs from fstab "
-                                "format (%s)" % (device.format.type, fstype))
-
-                if device.format.mountable:
-                    device.format.mountpoint = mountpoint
-                    device.format.mountopts = options
-
-                # is this useful?
-                try:
-                    device.format.options = options
-                except AttributeError:
-                    pass
-
                 if device not in self.devicetree.devices.values():
                     self.devicetree._addDevice(device)
 
diff --git a/upgrade.py b/upgrade.py
index c080c8651..be701fb24 100644
--- a/upgrade.py
+++ b/upgrade.py
@@ -234,11 +234,12 @@ def upgradeMountFilesystems(anaconda):
 	    mountExistingSystem(anaconda,
                                 anaconda.id.upgradeRoot[0],
                                 allowDirty = 0)
-        except Exception:
+        except ValueError as e:
+            log.error("Error mounting filesystem: %s" % e)
 	    anaconda.intf.messageWindow(_("Mount failed"),
-		_("One or more of the file systems listed in the "
-		  "/etc/fstab on your Linux system cannot be mounted. "
-		  "Please fix this problem and try to upgrade again."))
+                _("The following error occurred when mounting the file "
+                  "systems listed in /etc/fstab.  Please fix this problem "
+                  "and try to upgrade again.\n%s" % e))
 	    sys.exit(0)
 
 	checkLinks = ( '/etc', '/var', '/var/lib', '/var/lib/rpm',