From ef65defc14e3cfc849bad2110339deb7924d4b64 Mon Sep 17 00:00:00 2001 From: Jan Pokorny Date: Thu, 8 Feb 2024 15:13:02 +0100 Subject: [PATCH] Fstab cleanup fix Formats with multiple layers (e.g. LUKS) were not handled properly in fstab. This resulted in fstab entries not being properly cleaned from fstab when destroy actions were executed. This fixes the issue by using different variables with correct values based on action type. --- blivet/actionlist.py | 2 +- blivet/blivet.py | 5 ++- blivet/fstab.py | 51 ++++++++++++++++++++++++++++--- tests/storage_tests/fstab_test.py | 49 +++++++++++++++++++++++++++-- 4 files changed, 98 insertions(+), 9 deletions(-) diff --git a/blivet/actionlist.py b/blivet/actionlist.py index 9b3f727e9..75d0d4e5f 100644 --- a/blivet/actionlist.py +++ b/blivet/actionlist.py @@ -292,7 +292,7 @@ def process(self, callbacks=None, devices=None, fstab=None, dry_run=None): # (device may not exist afterwards) if not skip_fstab: try: - entry = fstab.entry_from_device(action.device) + entry = fstab.entry_from_action(action) except ValueError: # this device should not be in fstab bae_entry = None diff --git a/blivet/blivet.py b/blivet/blivet.py index e1c0042d8..3540bfbb2 100644 --- a/blivet/blivet.py +++ b/blivet/blivet.py @@ -56,7 +56,10 @@ log = logging.getLogger("blivet") -FSTAB_PATH = "/etc/fstab" +# Default path to fstab file. Left empty to prevent blivet from using +# fstab functionality by default. +# TODO Change to "/etc/fstab" at next major version +FSTAB_PATH = "" @six.add_metaclass(SynchronizedMeta) diff --git a/blivet/fstab.py b/blivet/fstab.py index 5981a9514..8fa102ffa 100644 --- a/blivet/fstab.py +++ b/blivet/fstab.py @@ -391,11 +391,48 @@ def entry_from_device(self, device): 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 entry_from_action(self, action): + """ Generate FSTabEntry object based on given blivet Action + + :keyword action: action to process + :type action: :class: `blivet.deviceaction.DeviceAction` + :returns: fstab entry object based on device processed by action + :rtype: :class: `FSTabEntry` + """ + + if not action.is_format: + raise ValueError("""cannot generate fstab entry from action '%s' because + its type is not 'format'""" % action) + + fmt = action.format + if action.is_destroy: + fmt = action.orig_format + if action.is_create: + fmt = action.device.format + + entry = FSTabEntry() + + entry.file = None + if fmt.mountable: + entry.file = fmt.mountpoint + elif fmt.type == "swap": + entry.file = "swap" + else: + raise ValueError("""cannot generate fstab entry from action '%s' because + it is neither mountable nor swap type""" % action) + + entry.spec = self._get_spec(action.device) + if entry.spec is None: + entry.spec = getattr(action.device, "fstab_spec", None) + + entry.vfstype = action.device.format.type + + return entry + def read(self): """ Read the fstab file from path stored in self.src_file. Resets currently loaded table contents. """ @@ -546,7 +583,7 @@ def add_entry(self, spec=None, file=None, vfstype=None, mntops=None, if mntops is not None: _entry.mntops = mntops - elif _entry.mntops is None: + if _entry.mntops is None: _entry.mntops = ['defaults'] if freq is not None: @@ -696,7 +733,11 @@ def _get_spec(self, device): # Use "globally" set (on FSTabManager level) spec type otherwise spec = None - spec_type = device.format.fstab.spec_type or self.spec_type + + if hasattr(device.format, 'fstab') and device.format.fstab.spec_type: + spec_type = device.format.fstab.spec_type + else: + spec_type = self.spec_type if spec_type == "LABEL" and device.format.label: spec = "LABEL=%s" % device.format.label @@ -737,7 +778,7 @@ def update(self, action, bae_entry): # does not have UUID assigned yet, so we skip that one return - if action.is_create and action.device.format.mountable: + if action.is_create and action.is_format and action.device.format.mountable: # add the device to the fstab # make sure it is not already present there try: @@ -745,6 +786,7 @@ def update(self, action, bae_entry): except ValueError: # this device should not be at fstab found = None + entry = None else: found = self.find_entry(entry=entry) @@ -754,6 +796,7 @@ def update(self, action, bae_entry): 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(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, diff --git a/tests/storage_tests/fstab_test.py b/tests/storage_tests/fstab_test.py index a6710fb45..bfdfdb44e 100644 --- a/tests/storage_tests/fstab_test.py +++ b/tests/storage_tests/fstab_test.py @@ -1,10 +1,9 @@ import os - -from .storagetestcase import StorageTestCase - import blivet import tempfile +from .storagetestcase import StorageTestCase + class FstabTestCase(StorageTestCase): @@ -97,6 +96,50 @@ def test_fstab(self): self.assertFalse("blivetTestLVMine" in contents) self.assertFalse("/mnt/test2" in contents) + def test_luks_creation(self): + # test creation of a multiple layer device + disk = self.storage.devicetree.get_device_by_path(self.vdevs[0]) + self.assertIsNotNone(disk) + + fstab_path = '/tmp/myfstab' + + 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 + + self.storage.initialize_disk(disk) + + var = self.storage.new_partition(fmt_type="luks", + fmt_args={"passphrase": "opensaysme"}, + size=blivet.size.Size("200MiB"), + parents=[disk], + mountpoint="/mnt/test_fstab_luks_wrong") + + self.storage.create_device(var) + + varenc = blivet.devices.LUKSDevice(name="blivetTest_fstab_luks", + size=var.size, + parents=var) + self.storage.create_device(varenc) + + varfs = blivet.formats.get_format(fmt_type="ext4", + device=varenc.path, + mountpoint="/mnt/test_fstab_luks_correct") + self.storage.format_device(varenc, varfs) + + blivet.partitioning.do_partitioning(self.storage) + + self.storage.do_it() + self.storage.reset() + + # Check fstab contents for added device + with open(fstab_path, "r") as f: + contents = f.read() + self.assertTrue("/mnt/test_fstab_luks_correct" in contents) + self.assertFalse("/mnt/test_fstab_luks_wrong" in contents) + def test_swap_creation(self): # test swap creation for presence of FSTabOptions object disk = self.storage.devicetree.get_device_by_path(self.vdevs[0])