From 434bb4220668e7918719dedeabbee2ce34a9f038 Mon Sep 17 00:00:00 2001 From: Jan Pokorny Date: Mon, 26 Jun 2023 13:32:17 +0200 Subject: [PATCH] Added support for user defined values in fstab - Allows to set prefered spec type in fstab by either setting blivet.fstab.spec_type for 'global' effect or device.format.fstab.spec_type for individual device. Allowed values are "UUID", "PARTUUID", "LABEL", "PARTLABEL" or "PATH". When selected value is not available, it uses other one (defaults to UUID). - Allows to set freq, passno and mntops for individual devices to be written in fstab. Done by setting device.format.fstab.freq/passno/mntops values respectively. --- blivet/actionlist.py | 9 +- blivet/formats/fs.py | 3 + blivet/fstab.py | 149 ++++++++++++++++++++++-------- tests/storage_tests/fstab_test.py | 20 ++-- tests/unit_tests/fstab_test.py | 13 +-- 5 files changed, 139 insertions(+), 55 deletions(-) diff --git a/blivet/actionlist.py b/blivet/actionlist.py index 0bda1f1a0..d3664cd04 100644 --- a/blivet/actionlist.py +++ b/blivet/actionlist.py @@ -287,8 +287,13 @@ def process(self, callbacks=None, devices=None, fstab=None, dry_run=None): # get (b)efore (a)ction.(e)xecute fstab entry # (device may not exist afterwards) if fstab is not None: - entry = fstab.entry_from_device(action.device) - bae_entry = fstab.find_entry(entry=entry) + try: + entry = fstab.entry_from_device(action.device) + except ValueError: + # this device should not be in fstab + bae_entry = None + else: + bae_entry = fstab.find_entry(entry=entry) with blivet_lock: diff --git a/blivet/formats/fs.py b/blivet/formats/fs.py index 63d1043dd..937e71803 100644 --- a/blivet/formats/fs.py +++ b/blivet/formats/fs.py @@ -49,6 +49,7 @@ from . import DeviceFormat, register_device_format from .. import util from ..flags import flags +from ..fstab import FSTabOptions from ..storage_log import log_exception_info, log_method_call from .. import arch from ..size import Size, ROUND_UP @@ -121,6 +122,8 @@ def __init__(self, **kwargs): DeviceFormat.__init__(self, **kwargs) + self.fstab = FSTabOptions() + # Create task objects self._fsck = self._fsck_class(self) self._mkfs = self._mkfs_class(self) diff --git a/blivet/fstab.py b/blivet/fstab.py index 59832cf70..6c15570d4 100644 --- a/blivet/fstab.py +++ b/blivet/fstab.py @@ -29,6 +29,23 @@ log = logging.getLogger("blivet") +class FSTabOptions(object): + """ User prefered fstab settings object intended to be attached to device.format. + Set variables override otherwise automatically obtained values put into fstab. + """ + + def __init__(self): + self.freq = None + self.passno = None + + # prefered spec identification type; default "UUID" + # possible values: None, "UUID", "LABEL", "PARTLABEL", "PARTUUID", "PATH" + self.spec_type = None + + # list of fstab options to be used + self.mntops = [] + + class FSTabEntry(object): """ One processed line of fstab """ @@ -205,6 +222,8 @@ def passno(self): @passno.setter def passno(self, value): + if value not in [None, 0, 1, 2]: + raise ValueError("fstab field passno must be 0, 1 or 2 (got '%s')" % value) self._entry.passno = value @property @@ -277,8 +296,12 @@ def __init__(self, src_file=None, dest_file=None): self.src_file = src_file self.dest_file = dest_file + # prefered spec identification type; default "UUID" + # possible values: None, "UUID", "LABEL", "PARTLABEL", "PARTUUID", "PATH" + self.spec_type = None + if self.src_file is not None: - self.read(self.src_file) + self.read() def __deepcopy__(self, memo): clone = FSTabManager(src_file=self.src_file, dest_file=self.dest_file) @@ -304,7 +327,7 @@ def __deepcopy__(self, memo): return clone - def __repr__(self): + def __str__(self): entry = self._table.next_fs() entries_str = "" while entry: @@ -347,27 +370,25 @@ def entry_from_device(self, device): entry = FSTabEntry() - if device.format.uuid: - entry.spec = "UUID=%s" % device.format.uuid - else: - entry.spec = getattr(device, "fstab_spec", None) - entry.file = None if device.format.mountable: entry.file = device.format.mountpoint - if device.format.type == "swap": + elif device.format.type == "swap": entry.file = "swap" + else: + raise ValueError("""cannot generate fstab entry from device '%s' because + it is neither mountable nor swap type""" % device.format.name) + + entry.spec = self._get_spec(device) + if entry.spec is None: + entry.spec = getattr(device, "fstab_spec", None) entry.vfstype = device.format.type return entry - def read(self, src_file=""): - """ Read the fstab file from path stored in self.src_file. Setting src_file parameter overrides - it with new value. - - :keyword src_file: When set, reads fstab from path specified in it - :type src_file: str + def read(self): + """ Read the fstab file from path stored in self.src_file. Resets currently loaded table contents. """ # Reset table @@ -375,24 +396,16 @@ def read(self, src_file=""): self._table.enable_comments(True) # resolve which file to read - if src_file == "": - if self.src_file is None: - # No parameter given, no internal value - return - elif src_file is None: + if self.src_file is None: return - else: - self.src_file = src_file self._table.errcb = self._parser_errcb - try: - self._table.parse_fstab(self.src_file) - except Exception as e: # pylint: disable=broad-except - # libmount throws general Exception. Okay... - if str(e) == "No such file or directory": - log.warning("Fstab file '%s' does not exist", self.src_file) - else: - raise + + if not os.path.isfile(self.src_file): + raise FileNotFoundError("Fstab file '%s' does not exist" % self.src_file) + + self._table.parse_fstab(self.src_file) + def find_device(self, blivet, spec=None, mntops=None, blkid_tab=None, crypt_tab=None, *, entry=None): """ Find a blivet device, based on spec or entry. Mount options can be used to refine the search. @@ -535,7 +548,7 @@ def add_entry(self, spec=None, file=None, vfstype=None, mntops=None, _entry.freq = 0 if passno is not None: - _entry.passno = freq + _entry.passno = passno elif _entry.passno is None: # 'passno' represents order of fsck run at the boot time (default: 0, i.e. disabled). # '/' should have 1, other checked should have 2 @@ -569,6 +582,8 @@ def remove_entry(self, spec=None, file=None, *, entry=None): fs = self.find_entry(spec, file, entry=entry) if fs: self._table.remove_fs(fs.entry) + else: + raise ValueError("Cannot remove entry (%s) from fstab, because it is not there" % entry) def write(self, dest_file=None): """ Commit the self._table into the self._dest_file. Setting dest_file parameter overrides @@ -664,6 +679,34 @@ def find_entry(self, spec=None, file=None, *, entry=None): return None + def _get_spec(self, device): + """ Resolve which device spec should be used and return it in a form accepted by fstab. + Returns None if desired spec was not found + """ + + # Use device specific spec type if it is set + # Use "globally" set (on FSTabManager level) spec type otherwise + + spec = None + spec_type = device.format.fstab.spec_type or self.spec_type + + if spec_type == "LABEL" and device.format.label: + spec = "LABEL=%s" % device.format.label + elif spec_type == "PARTUUID" and device.uuid: + spec = "PARTUUID=%s" % device.uuid + elif spec_type == "PARTLABEL" and device.format.name: + spec = "PARTLABEL=%s" % device.format.name + elif spec_type == "PATH": + spec = device.path + elif device.format.uuid: + # default choice + spec = "UUID=%s" % device.format.uuid + else: + # if everything else failed, let blivet decide + return None + + return spec + def update(self, action, bae_entry): """ Update fstab based on action type and device. Does not commit changes to a file. @@ -689,22 +732,56 @@ def update(self, action, bae_entry): if action.is_create and action.device.format.mountable: # add the device to the fstab # make sure it is not already present there - entry = self.entry_from_device(action.device) - found = self.find_entry(entry=entry) + try: + entry = self.entry_from_device(action.device) + except ValueError: + # this device should not be at fstab + found = None + else: + found = self.find_entry(entry=entry) + + # get correct spec type to use (if None, the one already present in entry is used) + spec = self._get_spec(action.device) + if found is None and action.device.format.mountpoint is not None: # device is not present in fstab and has a defined mountpoint => add it - self.add_entry(entry=entry) + self.add_entry(spec=spec, + mntops=action.device.format.fstab.mntops, + freq=action.device.format.fstab.freq, + passno=action.device.format.fstab.passno, + entry=entry) + elif found and found.spec != spec and action.device.format.mountpoint is not None: + # allow change of spec of existing devices + self.remove_entry(entry=found) + self.add_entry(spec=spec, + mntops=action.device.format.fstab.mntops, + freq=action.device.format.fstab.freq, + passno=action.device.format.fstab.passno, + entry=found) elif found and found.file != action.device.format.mountpoint and action.device.format.mountpoint is not None: # device already exists in fstab but with a different mountpoint => add it - self.add_entry(file=action.device.format.mountpoint, entry=found) + self.add_entry(spec=spec, + file=action.device.format.mountpoint, + mntops=action.device.format.fstab.mntops, + freq=action.device.format.fstab.freq, + passno=action.device.format.fstab.passno, + entry=found) return if action.is_configure and action.is_format and bae_entry is not None: # Handle change of the mountpoint: # Change its value if it is defined, remove the fstab entry if it is None - entry = self.entry_from_device(action.device) + + # get correct spec type to use (if None, the one already present in entry is used) + spec = self._get_spec(action.device) + if action.device.format.mountpoint is not None and bae_entry.file != action.device.format.mountpoint: self.remove_entry(entry=bae_entry) - self.add_entry(file=action.device.format.mountpoint, entry=bae_entry) + self.add_entry(spec=spec, + file=action.device.format.mountpoint, + mntops=action.device.format.fstab.mntops, + freq=action.device.format.fstab.freq, + passno=action.device.format.fstab.passno, + entry=bae_entry) elif action.device.format.mountpoint is None: self.remove_entry(entry=bae_entry) diff --git a/tests/storage_tests/fstab_test.py b/tests/storage_tests/fstab_test.py index 05fb4e520..87a4dc48c 100644 --- a/tests/storage_tests/fstab_test.py +++ b/tests/storage_tests/fstab_test.py @@ -64,6 +64,12 @@ def test_fstab(self): parents=[vg], name="blivetTestLVMine") self.storage.create_device(lv) + # specify device spec representation in fstab + lv.format.fstab.spec_type = "PATH" + lv.format.fstab.freq = 54321 + lv.format.fstab.passno = 2 + lv.format.fstab.mntops = ['optionA', 'optionB'] + # Change the mountpoint, make sure the change will make it into the fstab ac = blivet.deviceaction.ActionConfigureFormat(device=lv, attr="mountpoint", new_value="/mnt/test2") self.storage.devicetree.actions.add(ac) @@ -75,7 +81,9 @@ def test_fstab(self): with open(fstab_path, "r") as f: contents = f.read() self.assertTrue("blivetTestLVMine" in contents) - self.assertTrue("/mnt/test2" in contents) + self.assertTrue("54321" in contents) + self.assertTrue("54321 2" in contents) + self.assertTrue("optionA,optionB" in contents) dev = self.storage.devicetree.get_device_by_name("blivetTestVG-blivetTestLVMine") self.storage.recursive_remove(dev) @@ -88,13 +96,3 @@ def test_fstab(self): contents = f.read() self.assertFalse("blivetTestLVMine" in contents) self.assertFalse("/mnt/test2" in contents) - - def test_get_device(self): - disk = self.storage.devicetree.get_device_by_path(self.vdevs[0]) - self.assertIsNotNone(disk) - - with tempfile.TemporaryDirectory() as tmpdirname: - fstab_path = os.path.join(tmpdirname, 'fstab') - - # change write path of blivet.fstab - self.storage.fstab.dest_file = fstab_path diff --git a/tests/unit_tests/fstab_test.py b/tests/unit_tests/fstab_test.py index c8353eb0e..8e8f388aa 100644 --- a/tests/unit_tests/fstab_test.py +++ b/tests/unit_tests/fstab_test.py @@ -98,7 +98,8 @@ def test_entry_from_device(self): def test_update(self): # Reset table - self.fstab.read(None) + self.fstab.src_file = None + self.fstab.read() # Device is not in the table and should be added dev1 = DiskDevice("device1", fmt=get_format("ext4", exists=True)) @@ -148,14 +149,13 @@ def test_update(self): action.device = dev2 action.is_create = True - entry = self.fstab.entry_from_device(dev2) - self.fstab.update(action, None) - self.assertIsNone(self.fstab.find_entry(entry=entry)) + self.assertIsNone(self.fstab.find_entry(file="/media/absent_in_fstab")) def test_find_device(self): # Reset table - self.fstab.read(None) + self.fstab.src_file = None + self.fstab.read() b = Blivet() @@ -170,7 +170,8 @@ def test_find_device(self): def test_get_device(self): # Reset table - self.fstab.read(None) + self.fstab.src_file = None + self.fstab.read() b = Blivet()