From 59c753533d54fd38ab1bf8d1a81e581b171c5419 Mon Sep 17 00:00:00 2001 From: Jan Pokorny Date: Wed, 10 Jul 2024 11:37:26 +0200 Subject: [PATCH] Fstab handling moves to blivet Anaconda handling fstab is something that should be done by blivet. This moves the fstab logic into the blivet. Crypttab and blkidtab handling is not supported by blivet yet so it remains mostly unchanged (for now) Added unit_tests --- .../modules/storage/devicetree/fsset.py | 234 ++++-------------- pyanaconda/modules/storage/devicetree/root.py | 53 ++-- .../modules/storage/test_fsset.py | 50 ++++ 3 files changed, 120 insertions(+), 217 deletions(-) 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)