diff --git a/pyanaconda/modules/storage/devicetree/fsset.py b/pyanaconda/modules/storage/devicetree/fsset.py index 83feaaac588..cace6f733da 100644 --- a/pyanaconda/modules/storage/devicetree/fsset.py +++ b/pyanaconda/modules/storage/devicetree/fsset.py @@ -20,6 +20,7 @@ import time from blivet import blockdev +from blivet.fstab import FSTabManager from blivet.devices import NoDevice, DirectoryDevice, NFSDevice, FileDevice, MDRaidArrayDevice, \ NetworkStorageDevice, OpticalDevice from blivet.errors import UnrecognizedFSTabEntryError, FSTabTypeMismatchError, SwapSpaceError @@ -307,7 +308,9 @@ def __init__(self, devicetree): self.blkid_tab = None self._fstab_swaps = set() self._system_filesystems = [] - self.preserve_lines = [] # lines we just ignore and preserve + + # unrecognized or ignored entries in fstab (blivet.fstab.FSTabEntry) + self.preserve_entries = [] @property def system_filesystems(self): @@ -326,134 +329,17 @@ def devices(self): def mountpoints(self): return self.devicetree.mountpoints - def _parse_one_line(self, devspec, mountpoint, fstype, options, _dump="0", _passno="0"): - """Parse an fstab entry for a device, return the corresponding device. - - The parameters correspond to the items in a single entry in the - order in which they occur in the entry. - - :return: the device corresponding to the entry - :rtype: :class:`blivet.devices.Device` - """ - - # no sense in doing any legwork for a noauto entry - if "noauto" in options.split(","): - log.info("ignoring noauto entry") - raise UnrecognizedFSTabEntryError() - - # find device in the tree - device = self.devicetree.resolve_device(devspec, - crypt_tab=self.crypt_tab, - blkid_tab=self.blkid_tab, - options=options) - - 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 and fstype.startswith("nfs"): - # NFS -- preserve but otherwise ignore - device = NFSDevice(devspec, - fmt=get_format(fstype, - exists=True, - device=devspec)) - elif devspec.startswith("/") and fstype == "swap": - # swap file - device = FileDevice(devspec, - parents=get_containing_device(devspec, self.devicetree), - fmt=get_format(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" - - # This is probably not going to do anything useful, so we'll - # make sure to try again from FSSet.mount_filesystems. The bind - # mount targets should be accessible by the time we try to do - # the bind mount from there. - parents = get_containing_device(devspec, self.devicetree) - device = DirectoryDevice(devspec, parents=parents, exists=True) - device.format = get_format("bind", - device=device.path, - exists=True) - elif mountpoint in ("/proc", "/sys", "/dev/shm", "/dev/pts", - "/sys/fs/selinux", "/proc/bus/usb", "/sys/firmware/efi/efivars"): - # drop these now -- we'll recreate later - return None - else: - # nodev filesystem -- preserve or drop completely? - fmt = get_format(fstype) - fmt_class = get_device_format_class("nodev") - if devspec == "none" or \ - (fmt_class and isinstance(fmt, fmt_class)): - device = NoDevice(fmt=fmt) - - if device is None: - log.error("failed to resolve %s (%s) from fstab", devspec, - fstype) - raise UnrecognizedFSTabEntryError() - - device.setup() - fmt = get_format(fstype, device=device.path, exists=True) - if fstype != "auto" and None in (device.format.type, fmt.type): - log.info("Unrecognized filesystem type for %s (%s)", - device.name, fstype) - device.teardown() - raise UnrecognizedFSTabEntryError() - - # make sure, if we're using a device from the tree, that - # the device's format we found matches what's in the fstab - ftype = getattr(fmt, "mount_type", fmt.type) - dtype = getattr(device.format, "mount_type", device.format.type) - if hasattr(fmt, "test_mount") and fstype != "auto" and ftype != dtype: - log.info("fstab says %s at %s is %s", dtype, mountpoint, ftype) - if fmt.test_mount(): # pylint: disable=no-member - device.format = fmt - else: - device.teardown() - raise FSTabTypeMismatchError(_( - "There is an entry in your /etc/fstab file that contains " - "an invalid or incorrect file system type. The file says that " - "{detected_type} at {mount_point} is {fstab_type}.").format( - detected_type=dtype, - mount_point=mountpoint, - fstab_type=ftype - )) - - del ftype - del dtype - - if hasattr(device.format, "mountpoint"): - device.format.mountpoint = mountpoint - - device.format.options = options - - return device - def parse_fstab(self, chroot=None): - """Parse /etc/fstab. - - preconditions: - all storage devices have been scanned, including filesystems - - FIXME: control which exceptions we raise - - XXX do we care about bind mounts? - how about nodev mounts? - loop mounts? + """ Process fstab. Devices in there but not in devicetree are added into it. + Unrecognized fstab entries are stored so they can be preserved """ + if not chroot or not os.path.isdir(chroot): chroot = conf.target.system_root - path = "%s/etc/fstab" % chroot - if not os.access(path, os.R_OK): - # XXX should we raise an exception instead? - log.info("cannot open %s for read", path) + fstab_path = "%s/etc/fstab" % chroot + if not os.access(fstab_path, os.R_OK): + log.info("cannot open %s for read", fstab_path) return blkid_tab = BlkidTab(chroot=chroot) @@ -475,35 +361,22 @@ def parse_fstab(self, chroot=None): self.blkid_tab = blkid_tab self.crypt_tab = crypt_tab - with open(path) as f: - log.debug("parsing %s", path) - - lines = f.readlines() - - for line in lines: - - (line, _pound, _comment) = line.partition("#") - fields = line.split() + fstab = FSTabManager(src_file=fstab_path) + fstab.read() - if not 4 <= len(fields) <= 6: - continue + for entry in fstab: + try: + device = fstab.get_device(self.devicetree, entry=entry) + except UnrecognizedFSTabEntryError: + self.preserve_entries.append(entry) + continue + if device not in self.devicetree.devices: try: - device = self._parse_one_line(*fields) - except UnrecognizedFSTabEntryError: - # just write the line back out as-is after upgrade - self.preserve_lines.append(line) - continue + self.devicetree._add_device(device) + except ValueError: + self.preserve_entries.append(entry) - if not device: - continue - - if device not in self.devicetree.devices: - try: - self.devicetree._add_device(device) - except ValueError: - # just write duplicates back out post-install - self.preserve_lines.append(line) def turn_on_swap(self, root_path=""): """Activate the system's swap space.""" @@ -643,9 +516,8 @@ def write(self): """Write out all config files based on the set of filesystems.""" sysroot = conf.target.system_root # /etc/fstab - fstab_path = os.path.normpath("%s/etc/fstab" % sysroot) fstab = self.fstab() - open(fstab_path, "w").write(fstab) + fstab.write(dest_file=os.path.normpath("%s/etc/fstab" % sysroot)) # /etc/crypttab crypttab_path = os.path.normpath("%s/etc/crypttab" % sysroot) @@ -718,8 +590,10 @@ def mdadm_conf(self): return content def fstab(self): - fmt_str = "%-23s %-23s %-7s %-15s %d %d\n" - fstab = """ + """ Create a FSTabManager object, fill it with current devices + and return it as a result to be written + """ + intro_comment = """ # # /etc/fstab # Created by anaconda on %s @@ -743,6 +617,10 @@ def fstab(self): rootdev = devices[0] root_on_netdev = any(rootdev.depends_on(netdev) for netdev in netdevs) + fstab = FSTabManager(src_file=None, dest_file=None) + + fstab._table.intro_comment = intro_comment + for device in devices: # why the hell do we put swap in the fstab, anyway? if not device.format.mountable and device.format.type != "swap": @@ -752,43 +630,31 @@ def fstab(self): if isinstance(device, OpticalDevice): continue - fstype = getattr(device.format, "mount_type", device.format.type) - if fstype == "swap": - mountpoint = "none" - options = device.format.options - else: - mountpoint = device.format.mountpoint - options = device.format.options - if not mountpoint: - log.warning("%s filesystem on %s has no mount point", - fstype, - device.path) - continue + try: + new_entry = fstab.entry_from_device(device) + except ValueError: + fstype = getattr(device.format, "mount_type", device.format.type) + log.warning("%s filesystem on %s has no mount point", + fstype, + device.path) # legacy warning message # TODO change to e.msg? + continue - options = options or "defaults" for netdev in netdevs: if device.depends_on(netdev): - if root_on_netdev and mountpoint == "/var": - options = options + ",x-initrd.mount" + if root_on_netdev and new_entry.file == "/var": + new_entry.mntops_add("x-initrd.mount") break + if device.encrypted: - options += ",x-systemd.device-timeout=0" - devspec = device.fstab_spec - dump = device.format.dump - if device.format.check and mountpoint == "/": - passno = 1 - elif device.format.check: - passno = 2 - else: - passno = 0 - fstab = fstab + device.fstab_comment - fstab = fstab + fmt_str % (devspec, mountpoint, fstype, - options, dump, passno) - - # now, write out any lines we were unable to process because of + new_entry.mntops_add("x-systemd.device-timeout=0") + + new_entry.comment = device.fstab_comment + fstab.add_entry(entry=new_entry) + + # now, write out any entries we were unable to process because of # unrecognized filesystems or unresolvable device specifications - for line in self.preserve_lines: - fstab += line + for preserved_entry in self.preserve_entries: + fstab.add_entry(entry=preserved_entry) return fstab diff --git a/pyanaconda/modules/storage/devicetree/root.py b/pyanaconda/modules/storage/devicetree/root.py index da97baed016..63da232ac0c 100644 --- a/pyanaconda/modules/storage/devicetree/root.py +++ b/pyanaconda/modules/storage/devicetree/root.py @@ -20,6 +20,7 @@ import shlex from blivet import util as blivet_util +from blivet.fstab import FSTabManager from blivet.errors import StorageError from blivet.storage_log import log_exception_info @@ -251,13 +252,13 @@ def _parse_fstab(devicetree, chroot): :param chroot: a path to the target OS installation :return: a tuple of a mount dict and a device list """ - mounts = {} - devices = [] - path = "%s/etc/fstab" % chroot - if not os.access(path, os.R_OK): - # XXX should we raise an exception instead? - log.info("cannot open %s for read", path) + mounts = dict() + devices = list() + + fstab_path = "%s/etc/fstab" % chroot + if not os.access(fstab_path, os.R_OK): + log.info("cannot open %s for read", fstab_path) return mounts, devices blkid_tab = BlkidTab(chroot=chroot) @@ -276,38 +277,24 @@ def _parse_fstab(devicetree, chroot): log_exception_info(log.info, "error parsing crypttab") crypt_tab = None - with open(path) as f: - log.debug("parsing %s", path) - for line in f.readlines(): - - (line, _pound, _comment) = line.partition("#") - fields = line.split(None, 4) - - if len(fields) < 5: - continue + fstab = FSTabManager(src_file=fstab_path) + fstab.read() - (devspec, mountpoint, fstype, options, _rest) = fields + for entry in fstab: - # find device in the tree - device = devicetree.resolve_device( - devspec, - crypt_tab=crypt_tab, - blkid_tab=blkid_tab, - options=options - ) - - if device is None: - continue + device = fstab.find_device(devicetree, entry=entry) + if device is None: + continue - # If a btrfs volume is found but a subvolume is expected, ignore the volume. - if device.type == "btrfs volume" and "subvol=" in options: - log.debug("subvolume from %s for %s not found", options, devspec) - continue + # If a btrfs volume is found but a subvolume is expected, ignore the volume. + if device.type == "btrfs volume" and "subvol=" in options: + log.debug("subvolume from %s for %s not found", options, devspec) + continue - if fstype != "swap": - mounts[mountpoint] = device + if entry.vfstype != "swap": + mounts[entry.file] = device - devices.append(device) + devices.append(device) return mounts, devices diff --git a/tests/unit_tests/pyanaconda_tests/modules/storage/test_fsset.py b/tests/unit_tests/pyanaconda_tests/modules/storage/test_fsset.py index 481fef2ba32..25794bbbd1e 100644 --- a/tests/unit_tests/pyanaconda_tests/modules/storage/test_fsset.py +++ b/tests/unit_tests/pyanaconda_tests/modules/storage/test_fsset.py @@ -15,6 +15,8 @@ # License and may only be used or replicated with the express permission of # Red Hat, Inc. # +import os +import tempfile import unittest from unittest.mock import patch @@ -24,6 +26,7 @@ from pyanaconda.modules.storage.platform import EFI, X86 from pyanaconda.modules.storage.devicetree.fsset import FSSet +from pyanaconda.modules.storage.devicetree.root import _parse_fstab class FSSetTestCase(unittest.TestCase): @@ -200,3 +203,50 @@ def test_collect_filesystems_extra(self): 'selinuxfs', 'tmpfs', ] + + def test_parse_fstab(self): + """ test the fsset.py parse_fstab method: unrecognized devices + from fstab are supposed to be stored in preserve entries + the rest of devices should stay in devicetree + """ + + UNRECOGNIZED_ENTRY = "UUID=111111 /mnt/fakemount ext3 defaults 0 0\n" + DEVICE_ENTRY = "/dev/dev2 /mnt ext4 defaults 0 0\n" + + self._add_device(StorageDevice("dev2", fmt=get_format("ext4", mountpoint="/"))) + + with tempfile.TemporaryDirectory() as tmpdirname: + fstab_path = os.path.join(tmpdirname, 'etc/fstab') + os.makedirs(os.path.dirname(fstab_path), exist_ok=True) + with open(fstab_path, "w") as f: + f.write(UNRECOGNIZED_ENTRY) + f.write(DEVICE_ENTRY) + + self.fsset.parse_fstab(chroot=tmpdirname) + + self.assertEqual(self.fsset.preserve_entries[0].file, "/mnt/fakemount") + self.assertIsNotNone(self.devicetree.get_device_by_path('/dev/dev2')) + + def test_root_parse_fstab(self): + """ test the root.py _parse_fstab function: return mounts and devices obtained from fstab + """ + + test_dev = StorageDevice("dev2", fmt=get_format("ext4", mountpoint="/mnt/testmount")) + + devicetree = DeviceTree() + devicetree._add_device(test_dev) + DEVICE_ENTRY = "/dev/dev2 /mnt/testmount ext4 defaults 0 0\n" + + with tempfile.TemporaryDirectory() as tmpdirname: + fstab_path = os.path.join(tmpdirname, 'etc/fstab') + os.makedirs(os.path.dirname(fstab_path), exist_ok=True) + with open(fstab_path, "w") as f: + f.write(DEVICE_ENTRY) + + mounts, devices = _parse_fstab(devicetree, chroot=tmpdirname) + + self.assertEqual(mounts["/mnt/testmount"], test_dev) + self.assertTrue(test_dev in devices) + + print("MYDEBUG: %s" % mounts) + print("MYDEBUG: %s" % devices)