From 32583342976b41e095079cb08956e8f061ff9c28 Mon Sep 17 00:00:00 2001 From: Jan Pokorny Date: Wed, 2 Nov 2022 14:37:45 +0100 Subject: [PATCH 01/32] Fstab support - added support for blivet to directly modify fstab based on blivet actions - added tests and unit tests --- blivet/actionlist.py | 8 +- blivet/blivet.py | 10 +- blivet/formats/fs.py | 8 +- blivet/fstab.py | 364 ++++++++++++++++++++++++++++++ python-blivet.spec | 1 + tests/storage_tests/fstab_test.py | 75 ++++++ tests/unit_tests/fstab_test.py | 124 ++++++++++ 7 files changed, 587 insertions(+), 3 deletions(-) create mode 100644 blivet/fstab.py create mode 100644 tests/storage_tests/fstab_test.py create mode 100644 tests/unit_tests/fstab_test.py diff --git a/blivet/actionlist.py b/blivet/actionlist.py index c7674cc20..d60fdb8ca 100644 --- a/blivet/actionlist.py +++ b/blivet/actionlist.py @@ -268,12 +268,13 @@ def _post_process(self, devices=None): partition.parted_partition = pdisk.getPartitionByPath(partition.path) @with_flag("processing") - def process(self, callbacks=None, devices=None, dry_run=None): + def process(self, callbacks=None, devices=None, fstab=None, dry_run=None): """ Execute all registered actions. :param callbacks: callbacks to be invoked when actions are executed :param devices: a list of all devices current in the devicetree + :param fstab: FSTabManagerObject tied to blivet, if None fstab file will not be modified :type callbacks: :class:`~.callbacks.DoItCallbacks` """ @@ -286,6 +287,7 @@ def process(self, callbacks=None, devices=None, dry_run=None): continue with blivet_lock: + try: action.execute(callbacks) except DiskLabelCommitError: @@ -312,6 +314,10 @@ def process(self, callbacks=None, devices=None, dry_run=None): device.update_name() device.format.device = device.path + if fstab is not None: + fstab.update(devices) + fstab.write() + self._completed_actions.append(self._actions.pop(0)) _callbacks.action_executed(action=action) diff --git a/blivet/blivet.py b/blivet/blivet.py index f2bb32c63..61b0b309f 100644 --- a/blivet/blivet.py +++ b/blivet/blivet.py @@ -40,6 +40,7 @@ from .errors import StorageError, DependencyError from .size import Size from .devicetree import DeviceTree +from .fstab import FSTabManager from .formats import get_default_filesystem_type from .flags import flags from .formats import get_format @@ -71,6 +72,10 @@ def __init__(self): self.size_sets = [] self.set_default_fstype(get_default_filesystem_type()) + # fstab write location purposedly set to None. It has to be overriden + # manually when using blivet. + self.fstab = FSTabManager(src_file="/etc/fstab", dest_file=None) + self._short_product_name = 'blivet' self._next_id = 0 @@ -111,7 +116,8 @@ def do_it(self, callbacks=None): """ - self.devicetree.actions.process(callbacks=callbacks, devices=self.devices) + self.devicetree.actions.process(callbacks=callbacks, devices=self.devices, fstab=self.fstab) + self.fstab.read() @property def next_id(self): @@ -141,6 +147,8 @@ def reset(self, cleanup_only=False): self.edd_dict = get_edd_dict(self.partitioned) self.devicetree.edd_dict = self.edd_dict + self.fstab.read() + if flags.include_nodev: self.devicetree.handle_nodev_filesystems() diff --git a/blivet/formats/fs.py b/blivet/formats/fs.py index acc1f9cb9..63d1043dd 100644 --- a/blivet/formats/fs.py +++ b/blivet/formats/fs.py @@ -91,7 +91,8 @@ class FS(DeviceFormat): # support for resize: grow/shrink, online/offline _resize_support = 0 - config_actions_map = {"label": "write_label"} + config_actions_map = {"label": "write_label", + "mountpoint": "change_mountpoint"} def __init__(self, **kwargs): """ @@ -645,6 +646,11 @@ def _teardown(self, **kwargs): udev.settle() + def change_mountpoint(self, dry_run=False): + # This function is intentionally left blank. Mountpoint change utilizes + # only the generic part of this code branch + pass + def read_label(self): """Read this filesystem's label. diff --git a/blivet/fstab.py b/blivet/fstab.py new file mode 100644 index 000000000..e5d9ce9c8 --- /dev/null +++ b/blivet/fstab.py @@ -0,0 +1,364 @@ +import os + +import gi + +gi.require_version("BlockDev", "2.0") +from gi.repository import BlockDev as blockdev + +from libmount import Table, Fs, MNT_ITER_FORWARD +from libmount import Error as LibmountException + +import logging +log = logging.getLogger("blivet") + + +def parser_errcb(tb, fname, line): # pylint: disable=unused-argument + print("{:s}:{:d}: parse error".format(fname, line)) + return 1 + + +class FSTabManager(): + # Read, write and modify fstab file + # This class is meant to work even without blivet. + # However some of its methods require blivet and will not function without + # it. These methods will import what they need when they are run. + + def __init__(self, src_file=None, dest_file=None): + self._table = Table() # Space for parsed fstab contents + + self.src_file = src_file + self.dest_file = dest_file + + if self.src_file is not None: + self.read(self.src_file) + + def __deepcopy__(self, memo): + clone = FSTabManager(src_file=self.src_file, dest_file=self.dest_file) + clone._table = Table() + clone._table.enable_comments(True) + + entry = self._table.next_fs() + entries = [entry] + while entry: + entries.append(entry) + entry = self._table.next_fs() + + # Libmount does not allow to simply use clone._table.add_fs(entry), so... + for entry in entries: + new_entry = Fs() + new_entry.source = entry.source + new_entry.target = entry.target + new_entry.fstype = entry.fstype + new_entry.append_options(entry.options) + new_entry.freq = entry.freq + new_entry.passno = entry.passno + if entry.comment is not None: + new_entry.comment = entry.comment + clone._table.add_fs(new_entry) + + return clone + + def _get_containing_device(self, path, devicetree): + # Return the device that a path resides on + if not os.path.exists(path): + return None + + st = os.stat(path) + major = os.major(st.st_dev) + minor = os.minor(st.st_dev) + link = "/sys/dev/block/%s:%s" % (major, minor) + if not os.path.exists(link): + return None + + try: + device_name = os.path.basename(os.readlink(link)) + except Exception: # pylint: disable=broad-except + log.error("failed to find device name for path %s", path) + return None + + if device_name.startswith("dm-"): + # have I told you lately that I love you, device-mapper? + # (code and comment copied from anaconda, kept for entertaining purposes) + device_name = blockdev.dm.name_from_node(device_name) + + return devicetree.get_device_by_name(device_name) + + def _from_device(self, device): + # Return the list of fstab options obtained from device + # *(result) of this method is meant to be used as a parameter in other methods + + spec = getattr(device, 'fstab_spec', None) + + file = None + if device.format.mountable: + file = device.format.mountpoint + if device.format.type == 'swap': + file = 'swap' + + vfstype = device.format.type + mntops = None + + return spec, file, vfstype, mntops + + def _from_entry(self, entry): + return entry.source, entry.target, entry.fstype, ','.join(entry.options) + + def read(self, src_file=''): + # Read fstab file + + # Reset table + self._table = Table() + 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: + return + else: + self.src_file = src_file + + self._table.errcb = parser_errcb + self._table.parse_fstab(self.src_file) + + def find_device_by_specs(self, blivet, spec, file_dummy=None, vfstype_dummy=None, mntops=""): # pylint: disable=unused-argument + # Parse an fstab entry for a device, return the corresponding device, + # return None if not found + # dummy arguments allow using result of _from_device/_from_entry as a parameter of this method + return blivet.devicetree.resolve_device(spec, options=mntops) + + def find_device_by_entry(self, blivet, entry): + args = self._from_entry(entry) + return self.find_device_by_specs(blivet, *args) + + def get_device_by_specs(self, blivet, spec, file, vfstype, mntops): + # Parse an fstab entry for a device, return the corresponding device, + # create new one if it does not exist + + from blivet.formats import get_format + from blivet.devices import DirectoryDevice, NFSDevice, FileDevice, NoDevice + from blivet.formats import get_device_format_class + + # no sense in doing any legwork for a noauto entry + if "noauto" in mntops.split(","): + raise ValueError("Unrecognized fstab entry value 'noauto'") + + # find device in the tree + device = blivet.devicetree.resolve_device(spec, options=mntops) + + if device: + # fall through to the bottom of this block + pass + elif ":" in spec and vfstype.startswith("nfs"): + # NFS -- preserve but otherwise ignore + device = NFSDevice(spec, + fmt=get_format(vfstype, + exists=True, + device=spec)) + elif spec.startswith("/") and vfstype == "swap": + # swap file + device = FileDevice(spec, + parents=self._get_containing_device(spec, blivet.devicetree), + fmt=get_format(vfstype, + device=spec, + exists=True), + exists=True) + elif vfstype == "bind" or "bind" in mntops: + # bind mount... set vfstype so later comparison won't + # turn up false positives + vfstype = "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 = self._get_containing_device(spec, blivet.devicetree) + device = DirectoryDevice(spec, parents=parents, exists=True) + device.format = get_format("bind", + device=device.path, + exists=True) + elif file 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(vfstype) + fmt_class = get_device_format_class("nodev") + if spec == "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", spec, + vfstype) + raise ValueError() + + device.setup() + fmt = get_format(vfstype, device=device.path, exists=True) + if vfstype != "auto" and None in (device.format.type, fmt.type): + log.info("Unrecognized filesystem type for %s (%s)", + device.name, vfstype) + device.teardown() + raise ValueError() + + # 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 vfstype != "auto" and ftype != dtype: + log.info("fstab says %s at %s is %s", dtype, file, ftype) + if fmt.test_mount(): # pylint: disable=no-member + device.format = fmt + else: + device.teardown() + log.info("There is an entry in your fstab file that contains " + "unrecognized file system type. The file says that " + "%s at %s is %s.", dtype, file, ftype) + return None + + if hasattr(device.format, "mountpoint"): + device.format.mountpoint = file + + device.format.options = mntops + + return device + + def get_device_by_entry(self, blivet, entry): + args = self._from_entry(entry) + return self.get_device_by_specs(blivet, *args) + + def add_entry_by_specs(self, spec, file, vfstype, mntops, freq=None, passno=None, comment=None): + # Add new entry into the table + + # Default mount options + if mntops is None: + mntops = 'defaults' + + # Whether the fs should be dumped by dump(8), defaults to 0 + if freq is None: + freq = 0 + + # Order of fsck run at boot time. '/' should have 1, other 2, defaults to 0 + if passno is None: + if file is None: + file = '' + if file == '/': + passno = 1 + elif file.startswith('/boot'): + passno = 2 + else: + passno = 0 + + entry = Fs() + + entry.source = spec + entry.target = file + entry.fstype = vfstype + entry.append_options(mntops) + entry.freq = freq + entry.passno = passno + + if comment is not None: + # add '# ' at the start of any comment line and newline to the end of comment + modif_comment = '# ' + comment.replace('\n', '\n# ') + '\n' + entry.comment = modif_comment + + self._table.add_fs(entry) + + def add_entry_by_device(self, device): + args = self._from_device(device) + return self.add_entry_by_specs(*args) + + def remove_entry_by_specs(self, spec, file, vfstype=None, mntops=""): + fs = self.find_entry_by_specs(spec, file, vfstype, mntops) + if fs: + self._table.remove_fs(fs) + + def remove_entry_by_device(self, device): + args = self._from_device(device) + return self.remove_entry_by_specs(*args) + + def write(self, dest_file=None): + # Commit the self._table into the file. + if dest_file is None: + dest_file = self.dest_file + if dest_file is None: + log.info("Fstab path for writing was not specified") + return + + if os.path.exists(dest_file): + self._table.replace_file(dest_file) + else: + # write will fail if underlying directories do not exist + self._table.write_file(dest_file) + + def find_entry_by_specs(self, spec, file, vfstype_dummy=None, mntops_dummy=None): # pylint: disable=unused-argument + # Return line of self._table with given spec or file + # dummy arguments allow using result of _from_device/_from_entry as a parameter of this method + + entry = None + + if spec is not None and file is not None: + try: + entry = self._table.find_pair(spec, file, MNT_ITER_FORWARD) + except LibmountException: + entry = None + + if spec is not None: + try: + entry = self._table.find_source(spec, MNT_ITER_FORWARD) + except LibmountException: + entry = None + + if file is not None: + try: + entry = self._table.find_target(file, MNT_ITER_FORWARD) + except LibmountException: + entry = None + return entry + + def find_entry_by_device(self, device): + args = self._from_device(device) + return self.find_entry_by_specs(*args) + + def update(self, devices): + # Update self._table entries based on 'devices' list + # - keep unaffected devices entries unchanged + # - remove entries no longer tied to any device + # - add new entries for devices not present in self._table + + # remove entries not tied to any device + fstab_entries = [] + entry = self._table.next_fs() + while entry: + fstab_entries.append(entry) + entry = self._table.next_fs() + + for device in devices: + args = self._from_device(device) + entry = self.find_entry_by_specs(*args) + + # remove from the list if found + try: + fstab_entries.remove(entry) + except ValueError: + pass + + # All entries left in the fstab_entries do not have their counterpart + # in devices and are to be removed from self._table + for entry in fstab_entries: + self._table.remove_fs(entry) + + # add new entries based on devices not in the table + for device in devices: + # if mountable the device should be in the fstab + if device.format.mountable: + args = self._from_device(device) + found = self.find_entry_by_specs(*args) + if found is None: + self.add_entry_by_specs(*args) + elif found.target != device.format.mountpoint: + self.add_entry_by_specs(args[0], device.format.mountpoint, args[2], args[3]) diff --git a/python-blivet.spec b/python-blivet.spec index d94874fd0..727f70863 100644 --- a/python-blivet.spec +++ b/python-blivet.spec @@ -56,6 +56,7 @@ Requires: python3-pyudev >= %{pyudevver} Requires: parted >= %{partedver} Requires: python3-pyparted >= %{pypartedver} Requires: libselinux-python3 +Requires: python3-libmount Requires: python3-blockdev >= %{libblockdevver} Recommends: libblockdev-btrfs >= %{libblockdevver} Recommends: libblockdev-crypto >= %{libblockdevver} diff --git a/tests/storage_tests/fstab_test.py b/tests/storage_tests/fstab_test.py new file mode 100644 index 000000000..079a6ad00 --- /dev/null +++ b/tests/storage_tests/fstab_test.py @@ -0,0 +1,75 @@ +import os + +from .storagetestcase import StorageTestCase + +import blivet + +FSTAB_WRITE_FILE = "/tmp/test-blivet-fstab" + + +class FstabTestCase(StorageTestCase): + + def setUp(self): + super().setUp() + + disks = [os.path.basename(vdev) for vdev in self.vdevs] + self.storage = blivet.Blivet() + self.storage.exclusive_disks = disks + self.storage.reset() + + # make sure only the targetcli disks are in the devicetree + for disk in self.storage.disks: + self.assertTrue(disk.path in self.vdevs) + self.assertIsNone(disk.format.type) + self.assertFalse(disk.children) + + def _clean_up(self): + + self.storage.reset() + for disk in self.storage.disks: + if disk.path not in self.vdevs: + raise RuntimeError("Disk %s found in devicetree but not in disks created for tests" % disk.name) + self.storage.recursive_remove(disk) + + self.storage.do_it() + + # restore original fstab target file + self.storage.fstab.dest_file = "/etc/fstab" + + os.remove(FSTAB_WRITE_FILE) + + return super()._clean_up() + + def test_fstab(self): + disk = self.storage.devicetree.get_device_by_path(self.vdevs[0]) + self.assertIsNotNone(disk) + + # change write path of blivet.fstab + self.storage.fstab.dest_file = FSTAB_WRITE_FILE + + self.storage.initialize_disk(disk) + + pv = self.storage.new_partition(size=blivet.size.Size("100 MiB"), fmt_type="lvmpv", + parents=[disk]) + self.storage.create_device(pv) + + blivet.partitioning.do_partitioning(self.storage) + + vg = self.storage.new_vg(name="blivetTestVG", parents=[pv]) + self.storage.create_device(vg) + + lv = self.storage.new_lv(fmt_type="ext4", size=blivet.size.Size("50 MiB"), + parents=[vg], name="blivetTestLVMine") + self.storage.create_device(lv) + + # 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) + + self.storage.do_it() + self.storage.reset() + + with open(FSTAB_WRITE_FILE, "r") as f: + contents = f.read() + self.assertTrue("blivetTestLVMine" in contents) + self.assertTrue("/mnt/test2" in contents) diff --git a/tests/unit_tests/fstab_test.py b/tests/unit_tests/fstab_test.py new file mode 100644 index 000000000..965355eb9 --- /dev/null +++ b/tests/unit_tests/fstab_test.py @@ -0,0 +1,124 @@ +import os +import unittest + +from blivet.fstab import FSTabManager +from blivet.devices import DiskDevice +from blivet.formats import get_format +from blivet import Blivet + +FSTAB_WRITE_FILE = "/tmp/test-blivet-fstab2" + + +class FSTabTestCase(unittest.TestCase): + + def setUp(self): + self.fstab = FSTabManager() + self.addCleanup(self._clean_up) + + def _clean_up(self): + try: + os.remove(FSTAB_WRITE_FILE) + except FileNotFoundError: + pass + + def test_fstab(self): + + self.fstab.src_file = None + self.fstab.dest_file = FSTAB_WRITE_FILE + + # create new entries + self.fstab.add_entry_by_specs("/dev/sda_dummy", "/mnt/mountpath", "xfs", "defaults") + self.fstab.add_entry_by_specs("/dev/sdb_dummy", "/media/newpath", "ext4", "defaults") + + # try to find nonexistent entry based on device + entry = self.fstab.find_entry_by_specs("/dev/nonexistent", None, None, None) + self.assertEqual(entry, None) + + # try to find existing entry based on device + entry = self.fstab.find_entry_by_specs("/dev/sda_dummy", None, None, None) + # check that found item is the correct one + self.assertEqual(entry.target, "/mnt/mountpath") + + self.fstab.remove_entry_by_specs(None, "/mnt/mountpath", None, None) + entry = self.fstab.find_entry_by_specs(None, "/mnt/mountpath", None, None) + self.assertEqual(entry, None) + + # write new fstab + self.fstab.write() + + # read the file and verify its contents + with open(FSTAB_WRITE_FILE, "r") as f: + contents = f.read() + self.assertTrue("/media/newpath" in contents) + + def test_from_device(self): + + device = DiskDevice("test_device", fmt=get_format("ext4", exists=True)) + device.format.mountpoint = "/media/fstab_test" + + self.assertEqual(self.fstab._from_device(device), ('/dev/test_device', '/media/fstab_test', 'ext4', None)) + + def test_update(self): + + # Reset table + self.fstab.read(None) + + # Device already present in fstab._table that should be kept there after fstab.update() + dev1 = DiskDevice("already_present_keep", fmt=get_format("ext4", exists=True)) + dev1.format.mountpoint = "/media/fstab_test1" + + # Device already present in fstab._table which should be removed by fstab.update() + dev2 = DiskDevice("already_present_remove", fmt=get_format("ext4", exists=True)) + dev2.format.mountpoint = "/media/fstab_test2" + + # Device not at fstab._table which should not be added since it is not mountable + dev3 = DiskDevice("unmountable_skip") + + # Device not at fstab._table that should be added there after fstab.update() + dev4 = DiskDevice("new", fmt=get_format("ext4", exists=True)) + dev4.format.mountpoint = "/media/fstab_test3" + + self.fstab.add_entry_by_device(dev1) + self.fstab.add_entry_by_device(dev2) + + self.fstab.update([dev1, dev3, dev4]) + + # write new fstab + self.fstab.write("/tmp/test_blivet_fstab2") + + with open("/tmp/test_blivet_fstab2", "r") as f: + contents = f.read() + + self.assertTrue("already_present_keep" in contents) + self.assertFalse("already_present_remove" in contents) + self.assertFalse("unmountable_skip" in contents) + self.assertTrue("new" in contents) + + def test_find_device(self): + # Reset table + self.fstab.read(None) + + b = Blivet() + + dev1 = DiskDevice("sda_dummy", exists=True, fmt=get_format("xfs", exists=True)) + dev1.format.mountpoint = "/mnt/mountpath" + b.devicetree._add_device(dev1) + + dev2 = self.fstab.find_device_by_specs(b, "/dev/sda_dummy", "/mnt/mountpath", "xfs", "defaults") + + self.assertEqual(dev1, dev2) + + def test_get_device(self): + + # Reset table + self.fstab.read(None) + + b = Blivet() + + dev1 = DiskDevice("sda_dummy", exists=True, fmt=get_format("xfs", exists=True)) + dev1.format.mountpoint = "/mnt/mountpath" + b.devicetree._add_device(dev1) + + dev2 = self.fstab.get_device_by_specs(b, "/dev/sda_dummy", "/mnt/mountpath", "xfs", "defaults") + + self.assertEqual(dev1, dev2) From 0514047feed649aa049c04388f29496f94860709 Mon Sep 17 00:00:00 2001 From: Jan Pokorny Date: Wed, 22 Mar 2023 12:09:46 +0100 Subject: [PATCH 02/32] Incorporated review comments - changed API - other fixes --- .github/workflows/anaconda_tests.yml | 2 +- blivet/actionlist.py | 14 +- blivet/blivet.py | 5 +- blivet/fstab.py | 854 +++++++++++++++++++-------- misc/files/libmount_deb.py | 123 ++++ misc/install-test-dependencies.yml | 16 + tests/pylint/runpylint.py | 3 +- tests/storage_tests/fstab_test.py | 75 ++- tests/unit_tests/fstab_test.py | 129 ++-- 9 files changed, 900 insertions(+), 321 deletions(-) create mode 100644 misc/files/libmount_deb.py diff --git a/.github/workflows/anaconda_tests.yml b/.github/workflows/anaconda_tests.yml index b3e3188ec..2a94c3c60 100644 --- a/.github/workflows/anaconda_tests.yml +++ b/.github/workflows/anaconda_tests.yml @@ -39,7 +39,7 @@ jobs: podman run -i --rm -v ./blivet:/blivet:z -v ./anaconda:/anaconda:z quay.io/rhinstaller/anaconda-ci:$TARGET_BRANCH sh -c " \ set -xe; \ dnf remove -y python3-blivet; \ - dnf install -y python3-blockdev libblockdev-plugins-all python3-bytesize libbytesize python3-pyparted parted libselinux-python3; \ + dnf install -y python3-blockdev libblockdev-plugins-all python3-bytesize libbytesize python3-pyparted python3-libmount parted libselinux-python3; \ cd /blivet; \ python3 ./setup.py install; \ cd /anaconda; \ diff --git a/blivet/actionlist.py b/blivet/actionlist.py index d60fdb8ca..0e7b22d75 100644 --- a/blivet/actionlist.py +++ b/blivet/actionlist.py @@ -286,6 +286,12 @@ def process(self, callbacks=None, devices=None, fstab=None, dry_run=None): if dry_run: continue + # 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) + with blivet_lock: try: @@ -314,11 +320,11 @@ def process(self, callbacks=None, devices=None, fstab=None, dry_run=None): device.update_name() device.format.device = device.path - if fstab is not None: - fstab.update(devices) - fstab.write() - self._completed_actions.append(self._actions.pop(0)) _callbacks.action_executed(action=action) + if fstab is not None: + fstab.update(action, bae_entry) + fstab.write() + self._post_process(devices=devices) diff --git a/blivet/blivet.py b/blivet/blivet.py index 61b0b309f..e1c0042d8 100644 --- a/blivet/blivet.py +++ b/blivet/blivet.py @@ -56,6 +56,9 @@ log = logging.getLogger("blivet") +FSTAB_PATH = "/etc/fstab" + + @six.add_metaclass(SynchronizedMeta) class Blivet(object): @@ -74,7 +77,7 @@ def __init__(self): # fstab write location purposedly set to None. It has to be overriden # manually when using blivet. - self.fstab = FSTabManager(src_file="/etc/fstab", dest_file=None) + self.fstab = FSTabManager(src_file=FSTAB_PATH, dest_file=None) self._short_product_name = 'blivet' diff --git a/blivet/fstab.py b/blivet/fstab.py index e5d9ce9c8..59832cf70 100644 --- a/blivet/fstab.py +++ b/blivet/fstab.py @@ -1,9 +1,26 @@ -import os - -import gi +# fstab.py +# Fstab management. +# +# Copyright (C) 2023 Red Hat, Inc. +# +# This copyrighted material is made available to anyone wishing to use, +# modify, copy, or redistribute it subject to the terms and conditions of +# the GNU Lesser General Public License v.2, or (at your option) any later +# version. This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY expressed or implied, including the implied +# warranties of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See +# the GNU Lesser General Public License for more details. You should have +# received a copy of the GNU Lesser General Public License along with this +# program; if not, write to the Free Software Foundation, Inc., 51 Franklin +# Street, Fifth Floor, Boston, MA 02110-1301, USA. Any Red Hat trademarks +# that are incorporated in the source code or documentation are not subject +# to the GNU Lesser General Public License and may only be used or +# replicated with the express permission of Red Hat, Inc. +# +# Red Hat Author(s): Jan Pokorny +# -gi.require_version("BlockDev", "2.0") -from gi.repository import BlockDev as blockdev +import os from libmount import Table, Fs, MNT_ITER_FORWARD from libmount import Error as LibmountException @@ -12,19 +29,250 @@ log = logging.getLogger("blivet") -def parser_errcb(tb, fname, line): # pylint: disable=unused-argument - print("{:s}:{:d}: parse error".format(fname, line)) - return 1 +class FSTabEntry(object): + """ One processed line of fstab + """ + + def __init__(self, spec=None, file=None, vfstype=None, mntops=None, + freq=None, passno=None, comment=None, *, entry=None): + + # Note: "*" in arguments means that every following parameter can be used + # only with its key (e.g. "FSTabEntry(entry=Fs())") + + if entry is None: + self._entry = Fs() + else: + self._entry = entry + + if spec is not None: + self.spec = spec + if file is not None: + self.file = file + if vfstype is not None: + self.vfstype = vfstype + if mntops is not None: + self.mntops = mntops + if freq is not None: + self.freq = freq + if passno is not None: + self.passno = passno + if comment is not None: + self.comment = comment + + self._none_to_empty() + + def __repr__(self): + _comment = "" + if self._entry.comment not in ("", None): + _comment = "%s\n" % self._entry.comment + _line = "%s\t%s\t%s\t%s\t%s\t%s\t" % (self._entry.source, self._entry.target, self._entry.fstype, + self._entry.options, self._entry.freq, self._entry.passno) + return _comment + _line + + def __eq__(self, other): + if not isinstance(other, FSTabEntry): + return False + + if self._entry.source != other._entry.source: + return False + + if self._entry.target != other._entry.target: + return False + + if self._entry.fstype != other._entry.fstype: + return False + + if self._entry.options != other._entry.options: + return False + + if self._entry.freq != other._entry.freq: + return False + + if self._entry.passno != other._entry.passno: + return False + + return True + + def _none_to_empty(self): + """ Workaround function that internally replaces all None values with empty strings. + Reason: While libmount.Fs() initializes with parameters set to None, it does not + allow to store None as a valid value, blocking all value resets. + """ + + affected_params = [self._entry.source, + self._entry.target, + self._entry.fstype, + self._entry.options, + self._entry.comment] + + for param in affected_params: + if param is None: + param = "" + + @property + def entry(self): + return self._entry + + @entry.setter + def entry(self, value): + """ Setter for the whole internal entry value + + :param value: fstab entry + :type value: :class: `libmount.Fs` + """ + self._entry = value + + @property + def spec(self): + return self._entry.source if self._entry.source != "" else None + + @spec.setter + def spec(self, value): + self._entry.source = value if value is not None else "" + + @property + def file(self): + return self._entry.target if self._entry.target != "" else None + + @file.setter + def file(self, value): + self._entry.target = value if value is not None else "" + + @property + def vfstype(self): + return self._entry.fstype if self._entry.fstype != "" else None + + @vfstype.setter + def vfstype(self, value): + self._entry.fstype = value if value is not None else "" + @property + def mntops(self): + """ Return mount options -class FSTabManager(): - # Read, write and modify fstab file - # This class is meant to work even without blivet. - # However some of its methods require blivet and will not function without - # it. These methods will import what they need when they are run. + :returns: list of mount options or None when not set + :rtype: list of str + """ + + if self._entry.options == "": + return None + + return self._entry.options.split(',') + + def get_raw_mntops(self): + """ Return mount options + + :returns: comma separated string of mount options or None when not set + :rtype: str + """ + + return self._entry.options if self._entry.options != "" else None + + @mntops.setter + def mntops(self, values): + """ Set new mount options from the list of strings + + :param values: mount options (see man fstab(5) fs_mntops) + :type values: list of str + """ + + # libmount.Fs() internally stores options as a comma separated string + if values is None: + self._entry.options = "" + else: + self._entry.options = ','.join([x for x in values if x != ""]) + + def mntops_add(self, values): + """ Append new mount options to already existing ones + + :param values: mount options (see man fstab(5) fs_mntops) + :type values: list of str + """ + + self._entry.append_options(','.join([x for x in values if x != ""])) + + @property + def freq(self): + return self._entry.freq + + @freq.setter + def freq(self, value): + self._entry.freq = value + + @property + def passno(self): + return self._entry.passno + + @passno.setter + def passno(self, value): + self._entry.passno = value + + @property + def comment(self): + return self._entry.comment if self._entry.comment != "" else None + + @comment.setter + def comment(self, value): + if value is None: + self._entry.comment = "" + return + self._entry.comment = value + + def is_valid(self): + """ Verify that this instance has enough data for valid fstab entry + + :returns: False if any of the listed values is not set; otherwise True + :rtype: bool + """ + items = [self.spec, self.file, self.vfstype, self.mntops, self.freq, self.passno] + + # (Property getters replace empty strings with None) + return not any(x is None for x in items) + + +class FSTabManagerIterator(object): + """ Iterator class for FSTabManager + Iteration over libmount Table entries is weird - only one iterator can run at a time. + This class purpose is to mitigate that. + """ + + def __init__(self, fstab): + # To avoid messing up the libmount Table iterator which is singleton, + # set up independent entry list + entry = fstab._table.next_fs() + self.entries = [] + while entry: + self.entries.append(entry) + entry = fstab._table.next_fs() + self.cur_index = 0 + + def __iter__(self): + return self + + def __next__(self): + if self.cur_index < len(self.entries): + self.cur_index += 1 + return FSTabEntry(entry=self.entries[self.cur_index - 1]) + raise StopIteration + + +class FSTabManager(object): + """ Read, write and modify fstab file. + This class is meant to work even without blivet. + However some of its methods require blivet and will not function without it. + """ def __init__(self, src_file=None, dest_file=None): + """ Initialize internal table; load the file if specified + + :keyword src_file: Path to fstab file which will be read + :type src_file: str + :keyword dest_file: Path to file which will be overwritten with results + :type src_file: str + """ + self._table = Table() # Space for parsed fstab contents + self._table.enable_comments(True) self.src_file = src_file self.dest_file = dest_file @@ -37,81 +285,97 @@ def __deepcopy__(self, memo): clone._table = Table() clone._table.enable_comments(True) + # Two special variables for the first and last comment. When assigning None to them, libmount fails. + # Notice that we are copying the value from the instance of the same type. + if self._table.intro_comment is not None: + clone._table.intro_comment = self._table.intro_comment + if self._table.trailing_comment is not None: + clone._table.trailing_comment = self._table.trailing_comment + entry = self._table.next_fs() - entries = [entry] + entries = [] while entry: entries.append(entry) entry = self._table.next_fs() - # Libmount does not allow to simply use clone._table.add_fs(entry), so... for entry in entries: - new_entry = Fs() - new_entry.source = entry.source - new_entry.target = entry.target - new_entry.fstype = entry.fstype - new_entry.append_options(entry.options) - new_entry.freq = entry.freq - new_entry.passno = entry.passno - if entry.comment is not None: - new_entry.comment = entry.comment + new_entry = self._copy_fs_entry(entry) clone._table.add_fs(new_entry) return clone - def _get_containing_device(self, path, devicetree): - # Return the device that a path resides on - if not os.path.exists(path): - return None - - st = os.stat(path) - major = os.major(st.st_dev) - minor = os.minor(st.st_dev) - link = "/sys/dev/block/%s:%s" % (major, minor) - if not os.path.exists(link): - return None - - try: - device_name = os.path.basename(os.readlink(link)) - except Exception: # pylint: disable=broad-except - log.error("failed to find device name for path %s", path) - return None - - if device_name.startswith("dm-"): - # have I told you lately that I love you, device-mapper? - # (code and comment copied from anaconda, kept for entertaining purposes) - device_name = blockdev.dm.name_from_node(device_name) - - return devicetree.get_device_by_name(device_name) - - def _from_device(self, device): - # Return the list of fstab options obtained from device - # *(result) of this method is meant to be used as a parameter in other methods - - spec = getattr(device, 'fstab_spec', None) + def __repr__(self): + entry = self._table.next_fs() + entries_str = "" + while entry: + entries_str += repr(FSTabEntry(entry=entry)) + '\n' + entry = self._table.next_fs() + return entries_str + + def __iter__(self): + return FSTabManagerIterator(self) + + def _copy_fs_entry(self, entry): + """ Create copy of libmount.Fs() + """ + # Nope, it has to be done like this. Oh well... + new_entry = Fs() + new_entry.source = entry.source + new_entry.target = entry.target + new_entry.fstype = entry.fstype + new_entry.append_options(entry.options) + new_entry.freq = entry.freq + new_entry.passno = entry.passno + if entry.comment is not None: + new_entry.comment = entry.comment + return new_entry + + def _parser_errcb(self, tb, fname, line): # pylint: disable=unused-argument + """ Libmount interface error reporting function + """ + log.error("Fstab parse error '%s:%s'", fname, line) + return 1 + + def entry_from_device(self, device): + """ Generate FSTabEntry object based on given blivet Device + + :keyword device: device to process + :type device: :class: `blivet.devices.StorageDevice` + :returns: fstab entry object based on device + :rtype: :class: `FSTabEntry` + """ + + entry = FSTabEntry() + + if device.format.uuid: + entry.spec = "UUID=%s" % device.format.uuid + else: + entry.spec = getattr(device, "fstab_spec", None) - file = None + entry.file = None if device.format.mountable: - file = device.format.mountpoint - if device.format.type == 'swap': - file = 'swap' + entry.file = device.format.mountpoint + if device.format.type == "swap": + entry.file = "swap" - vfstype = device.format.type - mntops = None + entry.vfstype = device.format.type - return spec, file, vfstype, mntops + return entry - def _from_entry(self, entry): - return entry.source, entry.target, entry.fstype, ','.join(entry.options) + 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. - def read(self, src_file=''): - # Read fstab file + :keyword src_file: When set, reads fstab from path specified in it + :type src_file: str + """ # Reset table self._table = Table() self._table.enable_comments(True) # resolve which file to read - if src_file == '': + if src_file == "": if self.src_file is None: # No parameter given, no internal value return @@ -120,245 +384,327 @@ def read(self, src_file=''): else: self.src_file = src_file - self._table.errcb = parser_errcb - self._table.parse_fstab(self.src_file) - - def find_device_by_specs(self, blivet, spec, file_dummy=None, vfstype_dummy=None, mntops=""): # pylint: disable=unused-argument - # Parse an fstab entry for a device, return the corresponding device, - # return None if not found - # dummy arguments allow using result of _from_device/_from_entry as a parameter of this method - return blivet.devicetree.resolve_device(spec, options=mntops) - - def find_device_by_entry(self, blivet, entry): - args = self._from_entry(entry) - return self.find_device_by_specs(blivet, *args) - - def get_device_by_specs(self, blivet, spec, file, vfstype, mntops): - # Parse an fstab entry for a device, return the corresponding device, - # create new one if it does not exist + 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 + + 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. + If both entry and spec/mntops are given, spec/mntops are prioritized over entry values. + + :param blivet: Blivet instance with populated devicetree + :type blivet: :class: `Blivet` + :keyword spec: searched device specs (see man fstab(5) fs_spec) + :type spec: str + :keyword mntops: list of mount option strings (see man fstab(5) fs_mntops) + :type mnops: list + :keyword blkid_tab: Blkidtab object + :type blkid_tab: :class: `BlkidTab` + :keyword crypt_tab: Crypttab object + :type crypt_tab: :class: `CryptTab` + :keyword entry: fstab entry with its spec (and mntops) filled as an alternative input type + :type: :class: `FSTabEntry` + :returns: found device or None + :rtype: :class: `~.devices.StorageDevice` or None + """ + + _spec = spec or (entry.spec if entry is not None else None) + _mntops = mntops or (entry.mntops if entry is not None else None) + _mntops_str = ",".join(_mntops) if mntops is not None else None + + return blivet.devicetree.resolve_device(_spec, options=_mntops_str, blkid_tab=blkid_tab, crypt_tab=crypt_tab) + + def get_device(self, blivet, spec=None, file=None, vfstype=None, + mntops=None, blkid_tab=None, crypt_tab=None, *, entry=None): + """ Parse an fstab entry for a device and return the corresponding device from the devicetree. + If not found, try to create a new device based on given values. + Raises UnrecognizedFSTabError in case of invalid or incomplete data. + + :param blivet: Blivet instance with populated devicetree + :type blivet: :class: `Blivet` + :keyword spec: searched device specs (see man fstab(5) fs_spec) + :type spec: str + :keyword mntops: list of mount option strings (see man fstab(5) fs_mntops) + :type mnops: list + :keyword blkid_tab: Blkidtab object + :type blkid_tab: :class: `BlkidTab` + :keyword crypt_tab: Crypttab object + :type crypt_tab: :class: `CryptTab` + :keyword entry: fstab entry with its values filled as an alternative input type + :type: :class: `FSTabEntry` + :returns: found device + :rtype: :class: `~.devices.StorageDevice` + """ from blivet.formats import get_format - from blivet.devices import DirectoryDevice, NFSDevice, FileDevice, NoDevice - from blivet.formats import get_device_format_class + from blivet.devices import DirectoryDevice, FileDevice + from blivet.errors import UnrecognizedFSTabEntryError - # no sense in doing any legwork for a noauto entry - if "noauto" in mntops.split(","): - raise ValueError("Unrecognized fstab entry value 'noauto'") + _spec = spec or (entry.spec if entry is not None else None) + _mntops = mntops or (entry.mntops if entry is not None else None) + _mntops_str = ",".join(_mntops) if mntops is not None else None # find device in the tree - device = blivet.devicetree.resolve_device(spec, options=mntops) - - if device: - # fall through to the bottom of this block - pass - elif ":" in spec and vfstype.startswith("nfs"): - # NFS -- preserve but otherwise ignore - device = NFSDevice(spec, - fmt=get_format(vfstype, - exists=True, - device=spec)) - elif spec.startswith("/") and vfstype == "swap": - # swap file - device = FileDevice(spec, - parents=self._get_containing_device(spec, blivet.devicetree), - fmt=get_format(vfstype, - device=spec, - exists=True), - exists=True) - elif vfstype == "bind" or "bind" in mntops: - # bind mount... set vfstype so later comparison won't - # turn up false positives - vfstype = "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 = self._get_containing_device(spec, blivet.devicetree) - device = DirectoryDevice(spec, parents=parents, exists=True) - device.format = get_format("bind", - device=device.path, - exists=True) - elif file 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(vfstype) - fmt_class = get_device_format_class("nodev") - if spec == "none" or \ - (fmt_class and isinstance(fmt, fmt_class)): - device = NoDevice(fmt=fmt) + device = blivet.devicetree.resolve_device(_spec, options=_mntops_str, blkid_tab=blkid_tab, crypt_tab=crypt_tab) if device is None: - log.error("failed to resolve %s (%s) from fstab", spec, - vfstype) - raise ValueError() + if vfstype == "swap": + # swap file + device = FileDevice(_spec, + parents=blivet.devicetree.resolve_device(_spec), + fmt=get_format(vfstype, device=_spec, exists=True), + exists=True) + elif vfstype == "bind" or (_mntops is not None and "bind" in _mntops): + # bind mount... set vfstype so later comparison won't + # turn up false positives + vfstype = "bind" + + parents = blivet.devicetree.resolve_device(_spec) + device = DirectoryDevice(_spec, parents=parents, exists=True) + device.format = get_format("bind", device=device.path, exists=True) + + if device is None: + raise UnrecognizedFSTabEntryError("Could not resolve entry %s %s" % (_spec, vfstype)) - device.setup() fmt = get_format(vfstype, device=device.path, exists=True) if vfstype != "auto" and None in (device.format.type, fmt.type): - log.info("Unrecognized filesystem type for %s (%s)", - device.name, vfstype) - device.teardown() - raise ValueError() - - # 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 vfstype != "auto" and ftype != dtype: - log.info("fstab says %s at %s is %s", dtype, file, ftype) - if fmt.test_mount(): # pylint: disable=no-member - device.format = fmt - else: - device.teardown() - log.info("There is an entry in your fstab file that contains " - "unrecognized file system type. The file says that " - "%s at %s is %s.", dtype, file, ftype) - return None + raise UnrecognizedFSTabEntryError("Unrecognized filesystem type for %s: '%s'" % (_spec, vfstype)) if hasattr(device.format, "mountpoint"): device.format.mountpoint = file - device.format.options = mntops + device.format.options = _mntops return device - def get_device_by_entry(self, blivet, entry): - args = self._from_entry(entry) - return self.get_device_by_specs(blivet, *args) - - def add_entry_by_specs(self, spec, file, vfstype, mntops, freq=None, passno=None, comment=None): - # Add new entry into the table + def add_entry(self, spec=None, file=None, vfstype=None, mntops=None, + freq=None, passno=None, comment=None, *, entry=None): + """ Add a new entry into the table + If both entry and other values are given, these values are prioritized over entry values. + If mntops/freq/passno is not set uses their respective default values. + + :keyword spec: device specs (see man fstab(5) fs_spec) + :type spec: str + :keyword file: device mount path (see man fstab(5) fs_file) + :type file: str + :keyword vfstype: device file system type (see man fstab(5) fs_vfstype) + :type vfstype: str + :keyword mntops: list of mount option strings (see man fstab(5) fs_mntops) + :type mnops: list + :keyword freq: whether to dump the filesystem (see man fstab(5) fs_freq) + :type freq: int + :keyword passno: fsck order or disable fsck if 0 (see man fstab(5) fs_passno) + :type passno: int + :keyword comment: multiline comment added to fstab before entry; each line will be prefixed with "# " + :type comment: str + :keyword entry: fstab entry as an alternative input type + :type: :class: `FSTabEntry` + """ # Default mount options if mntops is None: - mntops = 'defaults' - - # Whether the fs should be dumped by dump(8), defaults to 0 - if freq is None: - freq = 0 - - # Order of fsck run at boot time. '/' should have 1, other 2, defaults to 0 - if passno is None: - if file is None: - file = '' - if file == '/': - passno = 1 - elif file.startswith('/boot'): - passno = 2 - else: - passno = 0 + mntops = ['defaults'] - entry = Fs() + # Use existing FSTabEntry or create a new one + _entry = entry or FSTabEntry() - entry.source = spec - entry.target = file - entry.fstype = vfstype - entry.append_options(mntops) - entry.freq = freq - entry.passno = passno + if spec is not None: + _entry.spec = spec + + if file is not None: + _entry.file = file + + if vfstype is not None: + _entry.vfstype = vfstype + + if mntops is not None: + _entry.mntops = mntops + elif _entry.mntops is None: + _entry.mntops = ['defaults'] + + if freq is not None: + # Whether the fs should be dumped by dump(8) (default: 0, i.e. no) + _entry.freq = freq + elif _entry.freq is None: + _entry.freq = 0 + + if passno is not None: + _entry.passno = freq + 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 + if _entry.file == '/': + _entry.passno = 1 + elif _entry.file.startswith('/boot'): + _entry.passno = 2 + else: + _entry.passno = 0 if comment is not None: - # add '# ' at the start of any comment line and newline to the end of comment + # Add '# ' at the start of any comment line and newline to the end of comment. + # Has to be done here since libmount won't do it. modif_comment = '# ' + comment.replace('\n', '\n# ') + '\n' - entry.comment = modif_comment + _entry.comment = modif_comment - self._table.add_fs(entry) + self._table.add_fs(_entry.entry) - def add_entry_by_device(self, device): - args = self._from_device(device) - return self.add_entry_by_specs(*args) + def remove_entry(self, spec=None, file=None, *, entry=None): + """ Find and remove entry from fstab based on spec/file. + If both entry and spec/file are given, spec/file are prioritized over entry values. - def remove_entry_by_specs(self, spec, file, vfstype=None, mntops=""): - fs = self.find_entry_by_specs(spec, file, vfstype, mntops) - if fs: - self._table.remove_fs(fs) + :keyword spec: device specs (see man fstab(5) fs_spec) + :type spec: str + :keyword file: device mount path (see man fstab(5) fs_file) + :type file: str + :keyword entry: fstab entry as an alternative input type + :type: :class: `FSTabEntry` + """ - def remove_entry_by_device(self, device): - args = self._from_device(device) - return self.remove_entry_by_specs(*args) + fs = self.find_entry(spec, file, entry=entry) + if fs: + self._table.remove_fs(fs.entry) def write(self, dest_file=None): - # Commit the self._table into the file. + """ Commit the self._table into the self._dest_file. Setting dest_file parameter overrides + writing path with its value. + + :keyword dest_file: When set, writes fstab to the path specified in it + :type dest_file: str + """ + if dest_file is None: dest_file = self.dest_file if dest_file is None: log.info("Fstab path for writing was not specified") return - if os.path.exists(dest_file): - self._table.replace_file(dest_file) - else: - # write will fail if underlying directories do not exist - self._table.write_file(dest_file) + # Output sanity check to prevent saving an incomplete file entries + # since libmount happily inserts incomplete lines into the fstab. + # Invalid lines should be skipped, but the whole table is written at once. + # Also the incomplete lines need to be preserved in the object. + # Conclusion: Create the second table, prune invalid/incomplete lines and write it. - def find_entry_by_specs(self, spec, file, vfstype_dummy=None, mntops_dummy=None): # pylint: disable=unused-argument - # Return line of self._table with given spec or file - # dummy arguments allow using result of _from_device/_from_entry as a parameter of this method + clean_table = Table() + clean_table.enable_comments(True) - entry = None + # Two special variables for the first and last comment. When assigning None libmount fails. + # Notice that we are copying the value from the same type instance. + if self._table.intro_comment is not None: + clean_table.intro_comment = self._table.intro_comment + if self._table.trailing_comment is not None: + clean_table.trailing_comment = self._table.trailing_comment - if spec is not None and file is not None: + entry = self._table.next_fs() + while entry: + if FSTabEntry(entry=entry).is_valid(): + new_entry = self._copy_fs_entry(entry) + clean_table.add_fs(new_entry) + else: + log.warning("Fstab entry: '%s' is not complete, it will not be written into the file", entry) + entry = self._table.next_fs() + + if os.path.exists(dest_file): + clean_table.replace_file(dest_file) + else: + try: + clean_table.write_file(dest_file) + except Exception as e: # pylint: disable=broad-except + # libmount throws general Exception if underlying directories do not exist. Okay... + if str(e) == "No such file or directory": + log.info("Underlying directory of fstab '%s' does not exist. creating...", dest_file) + os.makedirs(os.path.split(dest_file)[0]) + else: + raise + + def find_entry(self, spec=None, file=None, *, entry=None): + """ Return the line of loaded fstab with given spec and/or file. + If both entry and spec/file are given, spec/file are prioritized over entry values. + + :keyword spec: searched device specs (see man fstab(5) fs_spec) + :type spec: str + :keyword file: device mount path (see man fstab(5) fs_file) + :type file: str + :keyword entry: fstab entry as an alternative input type + :type: :class: `FSTabEntry` + :returns: found fstab entry object + :rtype: :class: `FSTabEntry` or None + """ + + _spec = spec or (entry.spec if entry is not None else None) + _file = file or (entry.file if entry is not None else None) + + found_entry = None + + if _spec is not None and _file is not None: try: - entry = self._table.find_pair(spec, file, MNT_ITER_FORWARD) + found_entry = self._table.find_pair(_spec, _file, MNT_ITER_FORWARD) except LibmountException: - entry = None + return None + return FSTabEntry(entry=found_entry) - if spec is not None: + if _spec is not None: try: - entry = self._table.find_source(spec, MNT_ITER_FORWARD) + found_entry = self._table.find_source(_spec, MNT_ITER_FORWARD) except LibmountException: - entry = None + return None + return FSTabEntry(entry=found_entry) if file is not None: try: - entry = self._table.find_target(file, MNT_ITER_FORWARD) + found_entry = self._table.find_target(file, MNT_ITER_FORWARD) except LibmountException: - entry = None - return entry + return None + return FSTabEntry(entry=found_entry) - def find_entry_by_device(self, device): - args = self._from_device(device) - return self.find_entry_by_specs(*args) + return None - def update(self, devices): - # Update self._table entries based on 'devices' list - # - keep unaffected devices entries unchanged - # - remove entries no longer tied to any device - # - add new entries for devices not present in self._table + def update(self, action, bae_entry): + """ Update fstab based on action type and device. Does not commit changes to a file. - # remove entries not tied to any device - fstab_entries = [] - entry = self._table.next_fs() - while entry: - fstab_entries.append(entry) - entry = self._table.next_fs() + :param action: just executed blivet action + :type action: :class: `~.deviceaction.DeviceAction` + :param bae_entry: fstab entry based on action.device before action.execute was called + :type bae_entry: :class: `FSTabEntry` or None + """ - for device in devices: - args = self._from_device(device) - entry = self.find_entry_by_specs(*args) + if not action._applied: + return - # remove from the list if found - try: - fstab_entries.remove(entry) - except ValueError: - pass - - # All entries left in the fstab_entries do not have their counterpart - # in devices and are to be removed from self._table - for entry in fstab_entries: - self._table.remove_fs(entry) - - # add new entries based on devices not in the table - for device in devices: - # if mountable the device should be in the fstab - if device.format.mountable: - args = self._from_device(device) - found = self.find_entry_by_specs(*args) - if found is None: - self.add_entry_by_specs(*args) - elif found.target != device.format.mountpoint: - self.add_entry_by_specs(args[0], device.format.mountpoint, args[2], args[3]) + if action.is_destroy and bae_entry is not None: + # remove destroyed device from the fstab + self.remove_entry(entry=bae_entry) + return + + if action.is_create and action.is_device and action.device.type == "luks/dm-crypt": + # when creating luks format, two actions are made. Device creation + # does not have UUID assigned yet, so we skip that one + return + + 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) + 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) + 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) + 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) + 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) + elif action.device.format.mountpoint is None: + self.remove_entry(entry=bae_entry) diff --git a/misc/files/libmount_deb.py b/misc/files/libmount_deb.py new file mode 100644 index 000000000..43812f5db --- /dev/null +++ b/misc/files/libmount_deb.py @@ -0,0 +1,123 @@ +#!/usr/bin/python + +# Debian based distributions do not have python3-libmount (and never will). This does manual +# installation of it. +# Requires autopoint and bison packages to work. + +from __future__ import print_function + +import os +import re +import shutil +import subprocess +import sys +import tempfile + +LM_GIT = 'https://github.com/util-linux/util-linux' +LM_CONFIG_CMD = './autogen.sh && ./configure --prefix=/usr ' \ + '--with-python=3 ' \ + '--disable-all-programs --enable-pylibmount ' \ + '--enable-libmount --enable-libblkid' +LM_BUILD_PATH = 'util-linux' + + +def run_command(command, cwd=None): + res = subprocess.Popen(command, shell=True, stdout=subprocess.PIPE, + stderr=subprocess.PIPE, cwd=cwd) + + out, err = res.communicate() + if res.returncode != 0: + output = out.decode().strip() + '\n' + err.decode().strip() + else: + output = out.decode().strip() + return (res.returncode, output) + +def get_arch(): + _ret, arch = run_command('uname -p') + return arch + +def get_distro(): + _ret, distro = run_command('cat /etc/os-release | grep ^ID= | cut -d= -f2 | tr -d \"') + return distro + +def main(): + + tempname = tempfile.mkdtemp() + + # clone the repo + print("Cloning '%s' repository into '%s'... " % (LM_GIT, tempname), end='', flush=True) + ret, out = run_command('git clone --depth 1 %s' % LM_GIT, tempname) + if ret != 0: + raise RuntimeError('Cloning libmount failed:\n%s' % out) + print('Done') + + print("Getting libmount version... ", end='', flush=True) + ret, out = run_command('mount --version') + if ret != 0: + raise RuntimeError('Getting mount version failed:\n%s' % out) + + match = re.match("^.*libmount (.*):.*$", out) + if not match: + raise RuntimeError('Getting mount version failed:\n%s' % out) + + libmount_version = 'v'+match.group(1) + + print('Done. (libmount version: %s)' % libmount_version) + + # Python-libmount wrapper is a part of util-linux repo, paths have to be set accordingly. + # Correct version of the repo has to be checked out as well. + + workpath = os.path.join(tempname, LM_BUILD_PATH, 'libmount') + + print("Fetching tags (takes a minute)... ", end='', flush=True) + ret, out = run_command('git fetch origin +refs/tags/%s:refs/tags/%s' % + (libmount_version, libmount_version), cwd=workpath) + if ret != 0: + raise RuntimeError('Fetching tags failed:\n%s' % out) + print('Done') + + print("Checking out '%s'... " % libmount_version, end='', flush=True) + + ret, out = run_command('git checkout tags/%s -b tag_temp' % libmount_version, cwd=workpath) + if ret != 0: + raise RuntimeError("Checking out tag '%s' failed:\n%s" % (libmount_version, out)) + print('Done') + + print("Running configure... ", end='', flush=True) + ret, out = run_command(LM_CONFIG_CMD, os.path.join(tempname, LM_BUILD_PATH)) + if ret != 0: + raise RuntimeError('Configure of libmount failed:\n%s' % out) + print('Done') + + print("Running make & make install... ", end='', flush=True) + ret, out = run_command('make -j6 && sudo make install', os.path.join(tempname, LM_BUILD_PATH)) + if ret != 0: + raise RuntimeError('Installing of libmount failed:\n%s' % out) + print('Done') + + print("Creating symlinks 'site-packages -> dist-packages'... ", end='', flush=True) + python_ver = '.'.join(sys.version.split('.')[0:2]) + install_dir = '/usr/lib/python%s/site-packages/libmount' % python_ver + target_dir = '/usr/lib/python%s/dist-packages/libmount' % python_ver + target_dir_local = '/usr/local/lib/python%s/dist-packages/libmount' % python_ver + + ret1, out1 = run_command('ln -fs %s %s' % (install_dir, target_dir)) + ret2, out2 = run_command('ln -fs %s %s' % (install_dir, target_dir_local)) + + # One can/will fail but not both + if ret1 + ret2 > 1: + raise RuntimeError('Symlink creation for libmount failed:\n%s\n%s' % (out1, out2)) + print('Done') + + shutil.rmtree(tempname) + + +if __name__ == '__main__': + + try: + main() + except RuntimeError as e: + print(e) + sys.exit(1) + + sys.exit(0) diff --git a/misc/install-test-dependencies.yml b/misc/install-test-dependencies.yml index 0902ad90d..3ba098d71 100644 --- a/misc/install-test-dependencies.yml +++ b/misc/install-test-dependencies.yml @@ -50,6 +50,7 @@ - libselinux-python3 - python3-blockdev - python3-bytesize + - python3-libmount - python3-libvirt - python3-paramiko - targetcli @@ -102,6 +103,7 @@ - libselinux-python3 - python3-blockdev - python3-bytesize + - python3-libmount - python3-libvirt - python3-pip - targetcli @@ -146,6 +148,8 @@ package: state: present name: + - autopoint + - bison - dosfstools - e2fsprogs - xfsprogs @@ -163,6 +167,18 @@ - open-iscsi when: ansible_distribution == 'Debian' or ansible_distribution == 'Ubuntu' + - name: Install libmount (Debian/Ubuntu) + block: + - name: Copy the libmount_deb script (Debian and CentOS non-x86_64) + copy: + src: files/libmount_deb.py + dest: "/tmp/libmount_deb.py" + mode: 0755 + + - name: Make and install libmount + shell: "python3 /tmp/libmount_deb.py" + when: ansible_distribution == 'Debian' or ansible_distribution == 'Ubuntu' + - name: Install pocketlint using pip (Debian/Ubuntu) pip: name=pocketlint executable=pip3 when: ansible_distribution == 'Debian' or ansible_distribution == 'Ubuntu' diff --git a/tests/pylint/runpylint.py b/tests/pylint/runpylint.py index 77f14a02c..196222495 100755 --- a/tests/pylint/runpylint.py +++ b/tests/pylint/runpylint.py @@ -28,7 +28,8 @@ def __init__(self): FalsePositive(r"Bad option value '(subprocess-popen-preexec-fn|try-except-raise|environment-modify|arguments-renamed|redundant-u-string-prefix)'"), FalsePositive(r"Instance of '(Action.*Device|Action.*Format|Action.*Member|Device|DeviceAction|DeviceFormat|Event|ObjectID|PartitionDevice|StorageDevice|BTRFS.*Device|LoopDevice)' has no 'id' member$"), FalsePositive(r"Instance of 'GError' has no 'message' member"), # overriding currently broken local pylint disable - FalsePositive(r"Module '(gi.repository.Gio|gi.repository.GLib)' has no .* member") # pylint/astroid has issues with GI modules https://github.com/PyCQA/pylint/issues/6352 + FalsePositive(r"Module '(gi.repository.Gio|gi.repository.GLib)' has no .* member"), # pylint/astroid has issues with GI modules https://github.com/PyCQA/pylint/issues/6352 + FalsePositive(r"No name '.*' in module 'libmount'") ] def _files(self): diff --git a/tests/storage_tests/fstab_test.py b/tests/storage_tests/fstab_test.py index 079a6ad00..05fb4e520 100644 --- a/tests/storage_tests/fstab_test.py +++ b/tests/storage_tests/fstab_test.py @@ -3,8 +3,7 @@ from .storagetestcase import StorageTestCase import blivet - -FSTAB_WRITE_FILE = "/tmp/test-blivet-fstab" +import tempfile class FstabTestCase(StorageTestCase): @@ -25,6 +24,8 @@ def setUp(self): def _clean_up(self): + self.storage.fstab.dest_file = None + self.storage.reset() for disk in self.storage.disks: if disk.path not in self.vdevs: @@ -36,40 +37,64 @@ def _clean_up(self): # restore original fstab target file self.storage.fstab.dest_file = "/etc/fstab" - os.remove(FSTAB_WRITE_FILE) - return super()._clean_up() def test_fstab(self): disk = self.storage.devicetree.get_device_by_path(self.vdevs[0]) self.assertIsNotNone(disk) - # change write path of blivet.fstab - self.storage.fstab.dest_file = FSTAB_WRITE_FILE + with tempfile.TemporaryDirectory() as tmpdirname: + fstab_path = os.path.join(tmpdirname, 'fstab') - self.storage.initialize_disk(disk) + # change write path of blivet.fstab + self.storage.fstab.dest_file = fstab_path - pv = self.storage.new_partition(size=blivet.size.Size("100 MiB"), fmt_type="lvmpv", - parents=[disk]) - self.storage.create_device(pv) + self.storage.initialize_disk(disk) - blivet.partitioning.do_partitioning(self.storage) + pv = self.storage.new_partition(size=blivet.size.Size("100 MiB"), fmt_type="lvmpv", + parents=[disk]) + self.storage.create_device(pv) - vg = self.storage.new_vg(name="blivetTestVG", parents=[pv]) - self.storage.create_device(vg) + blivet.partitioning.do_partitioning(self.storage) - lv = self.storage.new_lv(fmt_type="ext4", size=blivet.size.Size("50 MiB"), - parents=[vg], name="blivetTestLVMine") - self.storage.create_device(lv) + vg = self.storage.new_vg(name="blivetTestVG", parents=[pv]) + self.storage.create_device(vg) - # 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) + lv = self.storage.new_lv(fmt_type="ext4", size=blivet.size.Size("50 MiB"), + parents=[vg], name="blivetTestLVMine") + self.storage.create_device(lv) - self.storage.do_it() - self.storage.reset() + # 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) + + 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("blivetTestLVMine" in contents) + self.assertTrue("/mnt/test2" in contents) + + dev = self.storage.devicetree.get_device_by_name("blivetTestVG-blivetTestLVMine") + self.storage.recursive_remove(dev) + + self.storage.do_it() + self.storage.reset() + + # Check that previously added device is no longer in fstab + with open(fstab_path, "r") as f: + 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') - with open(FSTAB_WRITE_FILE, "r") as f: - contents = f.read() - self.assertTrue("blivetTestLVMine" in contents) - self.assertTrue("/mnt/test2" in contents) + # 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 965355eb9..c8353eb0e 100644 --- a/tests/unit_tests/fstab_test.py +++ b/tests/unit_tests/fstab_test.py @@ -1,7 +1,13 @@ +try: + from unittest.mock import Mock +except ImportError: + from mock import Mock + import os import unittest +import copy -from blivet.fstab import FSTabManager +from blivet.fstab import FSTabManager, FSTabEntry from blivet.devices import DiskDevice from blivet.formats import get_format from blivet import Blivet @@ -26,21 +32,33 @@ def test_fstab(self): self.fstab.src_file = None self.fstab.dest_file = FSTAB_WRITE_FILE + entry = FSTabEntry("/dev/sda_dummy", "/media/wrongpath", "xfs", ["defaults"]) + # create new entries - self.fstab.add_entry_by_specs("/dev/sda_dummy", "/mnt/mountpath", "xfs", "defaults") - self.fstab.add_entry_by_specs("/dev/sdb_dummy", "/media/newpath", "ext4", "defaults") + + # entry.file value should be overridden by add_entry.file parameter + self.fstab.add_entry(file="/mnt/mountpath", entry=entry) + self.fstab.add_entry("/dev/sdb_dummy", "/media/newpath", "ext4", ["defaults"]) + + # test the iterator and check that 2 entries are present + self.assertEqual((sum(1 for _ in self.fstab)), 2, "invalid number of entries in fstab table") # try to find nonexistent entry based on device - entry = self.fstab.find_entry_by_specs("/dev/nonexistent", None, None, None) + entry = self.fstab.find_entry("/dev/nonexistent") self.assertEqual(entry, None) # try to find existing entry based on device - entry = self.fstab.find_entry_by_specs("/dev/sda_dummy", None, None, None) - # check that found item is the correct one - self.assertEqual(entry.target, "/mnt/mountpath") + entry = self.fstab.find_entry("/dev/sda_dummy") + # check that item was found and it is the correct one + self.assertIsNotNone(entry, self.fstab) + self.assertEqual(entry.file, "/mnt/mountpath") + + self.fstab.remove_entry(file="/mnt/mountpath") + + # check that number of entries is now 1 + self.assertEqual((sum(1 for _ in self.fstab)), 1, "invalid number of entries in fstab table") - self.fstab.remove_entry_by_specs(None, "/mnt/mountpath", None, None) - entry = self.fstab.find_entry_by_specs(None, "/mnt/mountpath", None, None) + entry = self.fstab.find_entry(file="/mnt/mountpath") self.assertEqual(entry, None) # write new fstab @@ -51,48 +69,89 @@ def test_fstab(self): contents = f.read() self.assertTrue("/media/newpath" in contents) - def test_from_device(self): + def test_deepcopy(self): + fstab1 = FSTabManager(None, 'dest') + + fstab1.add_entry("/dev/dev1", "path1", "xfs", ["Ph'nglui", "mglw'nafh", "Cthulhu"]) + fstab1.add_entry("/dev/dev2", "path2", "ext4", ["R'lyeh", "wgah'nagl", "fhtagn"]) + + fstab2 = copy.deepcopy(fstab1) + + self.assertEqual(fstab1.src_file, fstab2.src_file) + self.assertEqual(fstab1.dest_file, fstab2.dest_file) + + # Both versions has the same length + self.assertEqual(sum(1 for _ in fstab1), sum(1 for _ in fstab1)) + + # All entries are the same in the same order + for entry1, entry2 in zip(fstab1, fstab2): + self.assertEqual(entry1, entry2) + + def test_entry_from_device(self): device = DiskDevice("test_device", fmt=get_format("ext4", exists=True)) device.format.mountpoint = "/media/fstab_test" - self.assertEqual(self.fstab._from_device(device), ('/dev/test_device', '/media/fstab_test', 'ext4', None)) + _entry = self.fstab.entry_from_device(device) + self.assertEqual(_entry, FSTabEntry('/dev/test_device', '/media/fstab_test', 'ext4', None, 0, 0)) def test_update(self): # Reset table self.fstab.read(None) - # Device already present in fstab._table that should be kept there after fstab.update() - dev1 = DiskDevice("already_present_keep", fmt=get_format("ext4", exists=True)) - dev1.format.mountpoint = "/media/fstab_test1" + # Device is not in the table and should be added + dev1 = DiskDevice("device1", fmt=get_format("ext4", exists=True)) + dev1.format.mountpoint = "/media/fstab_original_mountpoint" - # Device already present in fstab._table which should be removed by fstab.update() - dev2 = DiskDevice("already_present_remove", fmt=get_format("ext4", exists=True)) - dev2.format.mountpoint = "/media/fstab_test2" + action = Mock() + action.device = dev1 + action.is_create = True - # Device not at fstab._table which should not be added since it is not mountable - dev3 = DiskDevice("unmountable_skip") + self.fstab.update(action, None) - # Device not at fstab._table that should be added there after fstab.update() - dev4 = DiskDevice("new", fmt=get_format("ext4", exists=True)) - dev4.format.mountpoint = "/media/fstab_test3" + entry = self.fstab.entry_from_device(dev1) + self.assertIsNotNone(self.fstab.find_entry(entry=entry)) - self.fstab.add_entry_by_device(dev1) - self.fstab.add_entry_by_device(dev2) + # Device is in the table and its mountpoint has changed + action = Mock() + dev1.format.mountpoint = "/media/fstab_changed_mountpoint" + action.device = dev1 + action.is_destroy = False + action.is_create = False + action.is_configure = True + action.is_format = True + bae_entry = self.fstab.find_entry(entry=entry) - self.fstab.update([dev1, dev3, dev4]) + self.fstab.update(action, bae_entry) + self.assertIsNotNone(self.fstab.find_entry(spec="/dev/device1", file="/media/fstab_changed_mountpoint")) - # write new fstab - self.fstab.write("/tmp/test_blivet_fstab2") + # Device is already present in the table and should be removed + action = Mock() + action.device = dev1 + action.is_destroy = True + bae_entry = self.fstab.find_entry(entry=entry) - with open("/tmp/test_blivet_fstab2", "r") as f: - contents = f.read() + self.fstab.update(action, bae_entry) + self.assertIsNone(self.fstab.find_entry(entry=entry)) + + # Device not at fstab._table which should not be added since it is not mountable + dev2 = DiskDevice("unmountable_skip") + + # Add mountpoint just to mess with fstab manager + dev2.format.mountpoint = "/media/absent_in_fstab" + + # Make sure that the device is not mountable + self.assertFalse(dev2.format.mountable) + + action = Mock() + action.device = dev2 + action.is_create = True + + entry = self.fstab.entry_from_device(dev2) - self.assertTrue("already_present_keep" in contents) - self.assertFalse("already_present_remove" in contents) - self.assertFalse("unmountable_skip" in contents) - self.assertTrue("new" in contents) + self.fstab.update(action, None) + self.assertIsNone(self.fstab.find_entry(entry=entry)) def test_find_device(self): # Reset table @@ -104,7 +163,7 @@ def test_find_device(self): dev1.format.mountpoint = "/mnt/mountpath" b.devicetree._add_device(dev1) - dev2 = self.fstab.find_device_by_specs(b, "/dev/sda_dummy", "/mnt/mountpath", "xfs", "defaults") + dev2 = self.fstab.find_device(b, "/dev/sda_dummy") self.assertEqual(dev1, dev2) @@ -119,6 +178,6 @@ def test_get_device(self): dev1.format.mountpoint = "/mnt/mountpath" b.devicetree._add_device(dev1) - dev2 = self.fstab.get_device_by_specs(b, "/dev/sda_dummy", "/mnt/mountpath", "xfs", "defaults") + dev2 = self.fstab.get_device(b, "/dev/sda_dummy", "/mnt/mountpath", "xfs", ["defaults"]) self.assertEqual(dev1, dev2) From 6fa01700578d13f6c8a630a0d1cb0ef3f2d727b2 Mon Sep 17 00:00:00 2001 From: Jan Pokorny Date: Mon, 26 Jun 2023 13:32:17 +0200 Subject: [PATCH 03/32] 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 | 148 ++++++++++++++++++++++-------- tests/storage_tests/fstab_test.py | 20 ++-- tests/unit_tests/fstab_test.py | 13 +-- 5 files changed, 138 insertions(+), 55 deletions(-) diff --git a/blivet/actionlist.py b/blivet/actionlist.py index 0e7b22d75..68dbdb8f2 100644 --- a/blivet/actionlist.py +++ b/blivet/actionlist.py @@ -289,8 +289,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..6401f99f5 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,15 @@ 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 +547,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 +581,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 +678,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 +731,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() From 2778634e23eabb462287b7b5ff1432b3eae69f21 Mon Sep 17 00:00:00 2001 From: Jan Pokorny Date: Mon, 16 Oct 2023 14:00:56 +0200 Subject: [PATCH 04/32] fixed fstab.read issue (to be squashed) --- blivet/fstab.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/blivet/fstab.py b/blivet/fstab.py index 6401f99f5..7129648bf 100644 --- a/blivet/fstab.py +++ b/blivet/fstab.py @@ -301,7 +301,16 @@ def __init__(self, src_file=None, dest_file=None): self.spec_type = None if self.src_file is not None: - self.read() + # self.read() will raise an exception in case of invalid fstab path. + # This can interrupt object initialization thus preventing even setting read path + # to something else. + # This suppresses the exception. + if os.path.isfile(self.src_file): + self.read() + else: + # Acceptable at this point, but notify the user + log.info("Fstab file '%s' does not exist, setting fstab read path to None", self.src_file) + self.src_file = None def __deepcopy__(self, memo): clone = FSTabManager(src_file=self.src_file, dest_file=self.dest_file) From 1ac5b5385bfaa6ef274a1d197ef30f30a68d7b61 Mon Sep 17 00:00:00 2001 From: Vojtech Trefny Date: Mon, 6 Nov 2023 14:00:23 +0100 Subject: [PATCH 05/32] misc: Simplify the makebumpver script We no longer need the code for checking bugs and acks ecpecially after RHEL migration to Jira. --- Makefile | 31 +--- scripts/makebumpver | 391 +++----------------------------------------- 2 files changed, 25 insertions(+), 397 deletions(-) diff --git a/Makefile b/Makefile index 052e8fdb8..106f9a205 100644 --- a/Makefile +++ b/Makefile @@ -165,36 +165,7 @@ rpmlog: @echo bumpver: po-pull - @opts="-n $(PKGNAME) -v $(VERSION) -r $(RPMRELEASE)" ; \ - if [ ! -z "$(IGNORE)" ]; then \ - opts="$${opts} -i $(IGNORE)" ; \ - fi ; \ - if [ ! -z "$(MAP)" ]; then \ - opts="$${opts} -m $(MAP)" ; \ - fi ; \ - if [ ! -z "$(SKIP_ACKS)" ]; then \ - opts="$${opts} -s" ; \ - fi ; \ - if [ ! -z "$(BZDEBUG)" ]; then \ - opts="$${opts} -d" ; \ - fi ; \ - ( scripts/makebumpver $${opts} ) || exit 1 ; \ - -scratch-bumpver: - @opts="-n $(PKGNAME) -v $(RPMVERSION) -r $(RPMRELEASE) --newrelease $(RC_RELEASE)" ; \ - if [ ! -z "$(IGNORE)" ]; then \ - opts="$${opts} -i $(IGNORE)" ; \ - fi ; \ - if [ ! -z "$(MAP)" ]; then \ - opts="$${opts} -m $(MAP)" ; \ - fi ; \ - if [ ! -z "$(SKIP_ACKS)" ]; then \ - opts="$${opts} -s" ; \ - fi ; \ - if [ ! -z "$(BZDEBUG)" ]; then \ - opts="$${opts} -d" ; \ - fi ; \ - ( scripts/makebumpver $${opts} ) || exit 1 ; \ + ( scripts/makebumpver -n $(PKGNAME) -v $(VERSION) -r $(RPMRELEASE) ) || exit 1 ; scratch: @rm -f ChangeLog diff --git a/scripts/makebumpver b/scripts/makebumpver index 66bbc254c..0871ea887 100755 --- a/scripts/makebumpver +++ b/scripts/makebumpver @@ -20,17 +20,8 @@ # # Author: David Cantrell -import logging -logging.basicConfig(level=logging.DEBUG) -log = logging.getLogger("bugzilla") -log.setLevel(logging.INFO) -urllib_log = logging.getLogger("urllib3") -urllib_log.setLevel(logging.ERROR) - -import bugzilla +import argparse import datetime -import getopt -import getpass import os import re import six @@ -38,54 +29,18 @@ import subprocess import sys import textwrap + class MakeBumpVer: def __init__(self, **kwargs): - log.debug("%s", kwargs) - self.bzserver = 'bugzilla.redhat.com' - self.bzurl = "https://%s/xmlrpc.cgi" % self.bzserver - self.username = None - self.password = None - self.bz = None - self._bz_cache = {} - - authfile = os.path.realpath(os.getenv('HOME') + '/.rhbzauth') - if os.path.isfile(authfile): - f = open(authfile, 'r') - lines = map(lambda x: x.strip(), f.readlines()) - f.close() - - for line in lines: - if line.startswith('RHBZ_USER='): - self.username = line[10:].strip('"\'') - elif line.startswith('RHBZ_PASSWORD='): - self.password = line[14:].strip('"\'') - self.gituser = self._gitConfig('user.name') self.gitemail = self._gitConfig('user.email') self.name = kwargs.get('name') - self.fixedin_name = kwargs.get('fixedin', self.name) self.version = kwargs.get('version') self.release = kwargs.get('release') - self.new_release = kwargs.get('new_release') or self.release - self.ignore = kwargs.get('ignore') - - self.bugmap = {} - bugmap = kwargs.get('bugmap') - if bugmap and bugmap != '': - maps = bugmap.split(',') - for mapping in maps: - bugs = mapping.split('=') - if len(bugs) == 2: - self.bugmap[bugs[0]] = bugs[1] - - self.configure = kwargs.get('configure') + self.spec = kwargs.get('spec') self.version_files = kwargs.get('version_files') - self.skip_acks = kwargs.get('skip_acks', False) - - # RHEL release number or None - self.rhel = self._isRHEL() def _gitConfig(self, field): proc = subprocess.Popen(['git', 'config', field], @@ -103,28 +58,6 @@ class MakeBumpVer: new = ".".join(fields) return new - def _isRHEL(self): - proc = subprocess.Popen(['git', 'branch'], - stdout=subprocess.PIPE, - stderr=subprocess.PIPE) - out, _err = proc.communicate() - if six.PY3: - out = out.decode("utf-8") - lines = [x for x in out.strip('\n').split('\n') - if x.startswith("*")] - - if lines == [] or len(lines) > 1: - return False - - fields = lines[0].split(' ') - - if len(fields) == 2 and fields[1].startswith('rhel'): - branch_pattern=r"^rhel(\d+)-(.*)" - m = re.match(branch_pattern, fields[1]) - if m: - return m.group(1) - return False - def _getCommitDetail(self, commit, field): proc = subprocess.Popen(['git', 'log', '-1', "--pretty=format:%s" % field, commit], @@ -145,103 +78,7 @@ class MakeBumpVer: return ret - def _queryBug(self, bugid): - if not self.bz: - sys.stdout.write("Connecting to %s...\n" % self.bzserver) - - if not self.username: - sys.stdout.write('Username: ') - self.username = sys.stdin.readline() - self.username = self.username.strip() - - if not self.password: - self.password = getpass.getpass() - - bzclass = bugzilla.Bugzilla - self.bz = bzclass(url=self.bzurl) - print() - - if not self.bz.logged_in: - rc = self.bz.login(self.username, self.password) - log.debug("login rc = %s", rc) - - if bugid in self._bz_cache: - return self._bz_cache[bugid] - - bug = self.bz.getbug(bugid, extra_fields="flags") - log.debug("bug = %s", bug) - - if not bug: - return None - else: - self._bz_cache[bugid] = bug - return bug - - def _isRHELBug(self, bug, commit, summary): - bzentry = self._queryBug(bug) - - if not bzentry: - print("*** Bugzilla query for %s failed.\n" % bug) - return False - - if bzentry.product.startswith('Red Hat Enterprise Linux'): - return True - else: - print("*** Bug %s is not a RHEL bug." % bug) - print("*** Commit: %s" % commit) - print("*** %s\n" % summary) - return False - - def _isRHELBugInCorrectState(self, bug, commit, summary): - bzentry = self._queryBug(bug) - - if not bzentry: - print("*** Bugzilla query for %s failed.\n" % bug) - return False - - if bzentry.bug_status in ['MODIFIED', 'ON_QA']: - return True - else: - print("*** Bug %s is not in MODIFIED or ON_QA." % bug) - print("*** Commit: %s" % commit) - print("*** %s\n" % summary) - return False - - def _isRHELBugFixedInVersion(self, bug, commit, summary, fixedIn): - bzentry = self._queryBug(bug) - - if not bzentry: - print("*** Bugzilla query for %s failed.\n" % bug) - return False - - if bzentry.fixed_in == fixedIn: - return True - else: - print("*** Bug %s does not have correct Fixed In Version." % bug) - print("*** Found: %s" % bzentry.fixed_in) - print("*** Expected: %s" % fixedIn) - print("*** Commit: %s" % commit) - print("*** %s\n" % summary) - return False - - def _isRHELBugAcked(self, bug, commit, summary): - """ Check the bug's ack state - """ - if not self.rhel or self.skip_acks: - return True - - bzentry = self._queryBug(bug) - ack_pattern=r"rhel-%s\.\d+\.\d+" % self.rhel - for f in bzentry.flags: - if re.match(ack_pattern, f['name']) and f['status'] == '+': - return True - - print("*** Bug %s does not have ACK" % bug) - print("*** Commit: %s" % commit) - print("*** %s\n" % summary) - return False - - def _rpmLog(self, fixedIn): + def _rpmLog(self): git_range = "%s-%s-%s.." % (self.name, self.version, self.release) proc = subprocess.Popen(['git', 'log', '--pretty=oneline', '--no-merges', git_range], stdout=subprocess.PIPE, @@ -251,13 +88,7 @@ class MakeBumpVer: out = out.decode("utf-8") lines = out.strip('\n').split('\n') - if self.ignore: - ignore_commits = self.ignore.split(',') - lines = [l for l in lines if not any(l.startswith(c) for c in ignore_commits)] - rpm_log = [] - bad_bump = False - bad = False for line in lines: if not line: @@ -266,116 +97,9 @@ class MakeBumpVer: commit = fields[0] summary = self._getCommitDetail(commit, "%s")[0] - body = self._getCommitDetail(commit, "%b") author = self._getCommitDetail(commit, "%aE")[0] - if self.rhel: - rhbz = set() - bad = False - - # look for a bug in the summary line, validate if found - m = re.search(r"\(#\d+(\,.*)*\)", summary) - if m: - fullbug = summary[m.start():m.end()] - bugstr = summary[m.start()+2:m.end()-1] - - bug = '' - for c in bugstr: - if c.isdigit(): - bug += c - else: - break - - if len(bugstr) > len(bug): - tmp = bugstr[len(bug):] - - for c in tmp: - if not c.isalpha(): - tmp = tmp[1:] - else: - break - - if len(tmp) > 0: - author = tmp - - ckbug = self.bugmap.get(bug, bug) - valid = self._isRHELBug(ckbug, commit, summary) - - if valid: - summary = summary.replace(fullbug, "(%s)" % author) - rhbz.add("Resolves: rhbz#%s" % ckbug) - - if not self._isRHELBugInCorrectState(ckbug, commit, - summary): - bad = True - - if not self._isRHELBugFixedInVersion(ckbug, commit, - summary, fixedIn): - bad = True - - if not self._isRHELBugAcked(ckbug, commit, summary): - bad = True - else: - bad = True - summary_bug = ckbug - else: - summary = summary.strip() - summary += " (%s)" % author - summary_bug = None - - for bodyline in body: - m = re.match(r"^(Resolves|Related|Conflicts):\ +rhbz#\d+.*$", - bodyline) - if not m: - continue - - actionre = re.search("(Resolves|Related|Conflicts)", - bodyline) - bugre = re.search(r"\d+", bodyline) - if actionre and bugre: - action = actionre.group() - bug = bugre.group() - ckbug = self.bugmap.get(bug, bug) - valid = self._isRHELBug(ckbug, commit, summary) - - if valid: - rhbz.add("%s: rhbz#%s" % (action, ckbug)) - - # Remove the summary bug's Resolves action if it is for the same bug - if action != 'Resolves': - summary_str = "Resolves: rhbz#%s" % summary_bug - if summary_bug and ckbug == summary_bug and summary_str in rhbz: - rhbz.remove(summary_str) - else: - bad = True - - if valid and action == 'Resolves' and \ - (not self._isRHELBugInCorrectState(ckbug, commit, - summary) or \ - not self._isRHELBugFixedInVersion(ckbug, commit, - summary, - fixedIn) or \ - not self._isRHELBugAcked(ckbug, commit, summary)): - bad = True - elif valid and action == 'Related' and \ - self._isRHELBugAcked(ckbug, commit, summary): - print("*** Bug %s Related commit %s is allowed\n" % (bug, commit)) - # Related bugs only need to be valid and have an ack - bad = False - - if len(rhbz) == 0: - print("*** No bugs referenced in commit %s\n" % commit) - bad = True - - rpm_log.append((summary.strip(), list(rhbz))) - else: - rpm_log.append(("%s (%s)" % (summary.strip(), author), None)) - - if bad: - bad_bump = True - - if bad_bump: - sys.exit(1) + rpm_log.append("%s (%s)" % (summary.strip(), author)) return rpm_log @@ -399,7 +123,7 @@ class MakeBumpVer: self._replaceString(l, "Version: %s\n" % (self.version), "Version: %s\n" % (newVersion)) self._replaceString(l, "Release: %s\n" % (self.release+r"%{\?prerelease}%{\?dist}"), - "Release: %s\n" % (self.new_release+r"%{?prerelease}%{?dist}")) + "Release: %s\n" % (self.release+r"%{?prerelease}%{?dist}")) i = l.index('%changelog\n') top = l[:i] @@ -412,9 +136,9 @@ class MakeBumpVer: today = datetime.date.today() # pylint: disable=no-member stamp = today.strftime("%a %b %d %Y") f.write("* %s %s <%s> - %s-%s\n" % (stamp, self.gituser, self.gitemail, - newVersion, self.new_release)) + newVersion, self.release)) - for msg, rhbz in rpmlog: + for msg in rpmlog: msg = re.sub('(? Date: Mon, 6 Nov 2023 14:01:35 +0100 Subject: [PATCH 06/32] misc: Update test dependencies ansible playbook We no longer need python3-mock, pykickstart and python3-bugzilla. Also NTFS is now fully supported so we need ntfsprogs/ntfs-3g for testing. --- misc/install-test-dependencies.yml | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/misc/install-test-dependencies.yml b/misc/install-test-dependencies.yml index 3ba098d71..19c3b4f72 100644 --- a/misc/install-test-dependencies.yml +++ b/misc/install-test-dependencies.yml @@ -38,13 +38,11 @@ - e2fsprogs - xfsprogs - hfsplus-tools - - python3-mock + - ntfsprogs - python3-coverage - python3-pocketlint - - python3-bugzilla - python3-pycodestyle - python3-six - - python3-kickstart - python3-pyudev - python3-pyparted - libselinux-python3 @@ -94,10 +92,8 @@ - e2fsprogs - xfsprogs - python3-coverage - - python3-bugzilla - python3-pycodestyle - python3-six - - python3-kickstart - python3-pyudev - python3-pyparted - libselinux-python3 @@ -137,7 +133,6 @@ - libblockdev-plugins-all - gir1.2-blockdev-2.0 - python3-bytesize - - python3-mock - python3-selinux - python3-pyudev - python3-parted @@ -153,14 +148,13 @@ - dosfstools - e2fsprogs - xfsprogs - - python3-mock + - ntfs-3g - python3-coverage - python3-pycodestyle - pycodestyle - gettext - python3-polib - python3-paramiko - - python3-bugzilla - python3-libvirt - python3-pip - targetcli-fb From 179381cb87aa1ee9f394ef836cd686fa5124ddb5 Mon Sep 17 00:00:00 2001 From: Jan Pokorny Date: Thu, 2 Nov 2023 11:55:41 +0100 Subject: [PATCH 07/32] Added missing fstab object to SwapSpace Anaconda tests revealed missing FSTabOptions object in SwapSpace class. This issue is related to the recent merge of fstab rework PR #1119 which added this object to FS class. SwapSpace does not inherit from FS and was overlooked. This change adds the missing object. It also adds logic that skips all fstab related operations if fstab file is not to be written (i.e. fstab.dest_file is None). The reason for this is to eliminate the risk of potential issues caused by unused component. The test for above has been added as well. --- blivet/actionlist.py | 6 ++++-- blivet/formats/swap.py | 3 +++ blivet/fstab.py | 2 +- tests/storage_tests/fstab_test.py | 19 +++++++++++++++++++ 4 files changed, 27 insertions(+), 3 deletions(-) diff --git a/blivet/actionlist.py b/blivet/actionlist.py index 68dbdb8f2..9b3f727e9 100644 --- a/blivet/actionlist.py +++ b/blivet/actionlist.py @@ -281,6 +281,8 @@ def process(self, callbacks=None, devices=None, fstab=None, dry_run=None): devices = devices or [] self._pre_process(devices=devices) + skip_fstab = fstab is None or fstab.dest_file is None + for action in self._actions[:]: log.info("executing action: %s", action) if dry_run: @@ -288,7 +290,7 @@ 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: + if not skip_fstab: try: entry = fstab.entry_from_device(action.device) except ValueError: @@ -328,7 +330,7 @@ def process(self, callbacks=None, devices=None, fstab=None, dry_run=None): self._completed_actions.append(self._actions.pop(0)) _callbacks.action_executed(action=action) - if fstab is not None: + if not skip_fstab: fstab.update(action, bae_entry) fstab.write() diff --git a/blivet/formats/swap.py b/blivet/formats/swap.py index 03aa3cc68..77deb2a4d 100644 --- a/blivet/formats/swap.py +++ b/blivet/formats/swap.py @@ -24,6 +24,7 @@ from parted import PARTITION_SWAP, fileSystemType from ..errors import FSWriteUUIDError, SwapSpaceError +from ..fstab import FSTabOptions from ..storage_log import log_method_call from ..tasks import availability from ..tasks import fsuuid @@ -78,6 +79,8 @@ def __init__(self, **kwargs): log_method_call(self, **kwargs) DeviceFormat.__init__(self, **kwargs) + self.fstab = FSTabOptions() + self.priority = kwargs.get("priority", -1) self.label = kwargs.get("label") diff --git a/blivet/fstab.py b/blivet/fstab.py index 7129648bf..5981a9514 100644 --- a/blivet/fstab.py +++ b/blivet/fstab.py @@ -629,7 +629,7 @@ def write(self, dest_file=None): new_entry = self._copy_fs_entry(entry) clean_table.add_fs(new_entry) else: - log.warning("Fstab entry: '%s' is not complete, it will not be written into the file", entry) + log.warning("Fstab entry: '%s' is incomplete, it will not be written into the file", entry) entry = self._table.next_fs() if os.path.exists(dest_file): diff --git a/tests/storage_tests/fstab_test.py b/tests/storage_tests/fstab_test.py index 87a4dc48c..a6710fb45 100644 --- a/tests/storage_tests/fstab_test.py +++ b/tests/storage_tests/fstab_test.py @@ -96,3 +96,22 @@ def test_fstab(self): contents = f.read() self.assertFalse("blivetTestLVMine" in contents) self.assertFalse("/mnt/test2" 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]) + 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 + + self.storage.format_device(disk, blivet.formats.get_format("swap")) + + try: + self.storage.do_it() + except AttributeError as e: + if "has no attribute 'fstab'" in str(e): + self.fail("swap creation test failed on missing FSTabOptions object: %s" % str(e)) From d050119351ad94c7b559a84c82509e40a5af1df4 Mon Sep 17 00:00:00 2001 From: Vojtech Trefny Date: Fri, 3 Nov 2023 13:17:37 +0100 Subject: [PATCH 08/32] fcoe/iscsi: Use libblockdev to load modules instead of modprobe --- blivet/fcoe.py | 19 ++++++++++++++++--- blivet/iscsi.py | 14 ++++++++++++-- 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/blivet/fcoe.py b/blivet/fcoe.py index 31297d2e3..371f8c201 100644 --- a/blivet/fcoe.py +++ b/blivet/fcoe.py @@ -18,6 +18,12 @@ # import os + +import gi +gi.require_version("BlockDev", "3.0") + +from gi.repository import BlockDev + from . import errors from . import udev from . import util @@ -33,11 +39,18 @@ def has_fcoe(): global _fcoe_module_loaded if not _fcoe_module_loaded: - util.run_program(["modprobe", "libfc"]) - _fcoe_module_loaded = True + try: + BlockDev.utils.load_kernel_module("libfc", None) + except BlockDev.UtilsError as e: + log.error("failed to load libfc: %s", str(e)) + else: + _fcoe_module_loaded = True if "bnx2x" in util.lsmod(): log.info("fcoe: loading bnx2fc") - util.run_program(["modprobe", "bnx2fc"]) + try: + BlockDev.utils.load_kernel_module("bnx2fc", None) + except BlockDev.UtilsError as e: + log.error("failed to load bnx2fc: %s", str(e)) return os.access("/sys/module/libfc", os.X_OK) diff --git a/blivet/iscsi.py b/blivet/iscsi.py index 3b243f6fb..b63468e9b 100644 --- a/blivet/iscsi.py +++ b/blivet/iscsi.py @@ -33,7 +33,9 @@ import gi gi.require_version("GLib", "2.0") +gi.require_version("BlockDev", "3.0") from gi.repository import GLib +from gi.repository import BlockDev import logging log = logging.getLogger("blivet") @@ -314,7 +316,10 @@ def _start_ibft(self): # Make sure iscsi_ibft is loaded otherwise any atttempts will fail with # 'Could not get list of targets from firmware. (err 21)' - util.run_program(['modprobe', '-a', 'iscsi_ibft']) + try: + BlockDev.utils.load_kernel_module("iscsi_ibft", None) + except BlockDev.UtilsError as e: + log.error("failed to load iscsi_ibft: %s", str(e)) args = GLib.Variant("(a{sv})", ([], )) try: @@ -394,7 +399,12 @@ def startup(self): os.makedirs(fulldir, 0o755) log.info("iSCSI startup") - util.run_program(['modprobe', '-a'] + ISCSI_MODULES) + for module in ISCSI_MODULES: + try: + BlockDev.utils.load_kernel_module(module, None) + except BlockDev.UtilsError as e: + log.error("failed to load %s: %s", module, str(e)) + # iscsiuio is needed by Broadcom offload cards (bnx2i). Currently # not present in iscsi-initiator-utils for Fedora. iscsiuio = shutil.which('iscsiuio') From ec8e187f67d436881cdc794804f93e09beac1bcf Mon Sep 17 00:00:00 2001 From: Vojtech Trefny Date: Fri, 27 Oct 2023 13:28:50 +0200 Subject: [PATCH 09/32] ci: Update default branch for Packit to 3.9-devel/release --- .packit.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.packit.yaml b/.packit.yaml index 24d364dd4..66c389f24 100644 --- a/.packit.yaml +++ b/.packit.yaml @@ -4,7 +4,7 @@ actions: - 'git config user.email "blivet-ci@example.com"' - 'git config user.name "Blivet CI"' # merge the release branch to get correct version in spec - - 'git merge --ff origin/3.8-release' + - 'git merge --ff origin/3.9-release' get-current-version: - "python3 ./setup.py --version" create-archive: @@ -28,7 +28,7 @@ jobs: trigger: commit owner: "@storage" project: blivet-daily - branch: 3.8-devel + branch: 3.9-devel preserve_project: true srpm_build_deps: From 8ca3d5545a353f054d4e1d22491a182c48f30f9d Mon Sep 17 00:00:00 2001 From: Vojtech Trefny Date: Tue, 24 Oct 2023 11:40:45 +0200 Subject: [PATCH 10/32] Remove support for ReiserFS ReiserFs support is scheluded to be removed from the kernel and the userspace tools were removed in Fedora 35. This removes only support for creating and managing ReiserFS, we will still recognize the filesystem. --- blivet/flags.py | 2 -- blivet/formats/fs.py | 20 ------------------- blivet/tasks/availability.py | 3 --- blivet/tasks/fsinfo.py | 5 ----- blivet/tasks/fslabeling.py | 9 --------- blivet/tasks/fsmkfs.py | 13 ------------ blivet/tasks/fssize.py | 4 ---- blivet/tasks/fsuuid.py | 6 ------ blivet/tasks/fswritelabel.py | 8 -------- blivet/tasks/fswriteuuid.py | 8 -------- doc/api/formats.rst | 1 - tests/storage_tests/formats_test/fs_test.py | 4 ---- .../formats_test/labeling_test.py | 9 --------- tests/storage_tests/formats_test/uuid_test.py | 16 ++------------- 14 files changed, 2 insertions(+), 106 deletions(-) diff --git a/blivet/flags.py b/blivet/flags.py index 716f0df47..be4eaa899 100644 --- a/blivet/flags.py +++ b/blivet/flags.py @@ -51,7 +51,6 @@ def __init__(self): self.gfs2 = True self.jfs = True - self.reiserfs = True # for this flag to take effect, # blockdev.mpath.set_friendly_names(flags.multipath_friendly_names) must @@ -113,7 +112,6 @@ def update_from_boot_cmdline(self): self.noiswmd = "noiswmd" in self.boot_cmdline self.gfs2 = "gfs2" in self.boot_cmdline self.jfs = "jfs" in self.boot_cmdline - self.reiserfs = "reiserfs" in self.boot_cmdline flags = Flags() diff --git a/blivet/formats/fs.py b/blivet/formats/fs.py index acc1f9cb9..b2cbe8ca6 100644 --- a/blivet/formats/fs.py +++ b/blivet/formats/fs.py @@ -1077,27 +1077,7 @@ class ReiserFS(FS): """ reiserfs filesystem """ _type = "reiserfs" - _labelfs = fslabeling.ReiserFSLabeling() - _uuidfs = fsuuid.ReiserFSUUID() - _modules = ["reiserfs"] - _max_size = Size("16 TiB") - _formattable = True _linux_native = True - _dump = True - _check = True - _packages = ["reiserfs-utils"] - _info_class = fsinfo.ReiserFSInfo - _mkfs_class = fsmkfs.ReiserFSMkfs - _size_info_class = fssize.ReiserFSSize - _writelabel_class = fswritelabel.ReiserFSWriteLabel - _writeuuid_class = fswriteuuid.ReiserFSWriteUUID - _metadata_size_factor = 0.98 # reiserfs metadata may take 2% of space - parted_system = fileSystemType["reiserfs"] - - @property - def supported(self): - """ Is this filesystem a supported type? """ - return self.utils_available if flags.reiserfs else self._supported register_device_format(ReiserFS) diff --git a/blivet/tasks/availability.py b/blivet/tasks/availability.py index 4b947d6cd..ffbc6b087 100644 --- a/blivet/tasks/availability.py +++ b/blivet/tasks/availability.py @@ -500,7 +500,6 @@ def available_resource(name): DOSFSTOOLS_VERSION = VersionMethod(DOSFSTOOLS_INFO) # applications -DEBUGREISERFS_APP = application("debugreiserfs") DF_APP = application("df") DOSFSCK_APP = application("dosfsck") DOSFSLABEL_APP = application("dosfslabel") @@ -521,13 +520,11 @@ def available_resource(name): MKFS_JFS_APP = application("mkfs.jfs") MKFS_XFS_APP = application("mkfs.xfs") MKNTFS_APP = application("mkntfs") -MKREISERFS_APP = application("mkreiserfs") MLABEL_APP = application("mlabel") MULTIPATH_APP = application("multipath") NTFSINFO_APP = application("ntfsinfo") NTFSLABEL_APP = application("ntfslabel") NTFSRESIZE_APP = application("ntfsresize") -REISERFSTUNE_APP = application("reiserfstune") RESIZE2FS_APP = application_by_version("resize2fs", E2FSPROGS_VERSION) TUNE2FS_APP = application_by_version("tune2fs", E2FSPROGS_VERSION) XFSADMIN_APP = application("xfs_admin") diff --git a/blivet/tasks/fsinfo.py b/blivet/tasks/fsinfo.py index b5432e9ce..af35af5ab 100644 --- a/blivet/tasks/fsinfo.py +++ b/blivet/tasks/fsinfo.py @@ -88,11 +88,6 @@ class NTFSInfo(FSInfo): options = ["-m"] -class ReiserFSInfo(FSInfo): - ext = availability.DEBUGREISERFS_APP - options = [] - - class XFSInfo(FSInfo): ext = availability.XFSDB_APP options = ["-c", "sb 0", "-c", "p dblocks", "-c", "p blocksize", "-r"] diff --git a/blivet/tasks/fslabeling.py b/blivet/tasks/fslabeling.py index 5bc0111d1..1a1c4b841 100644 --- a/blivet/tasks/fslabeling.py +++ b/blivet/tasks/fslabeling.py @@ -74,15 +74,6 @@ def label_format_ok(cls, label): return len(label) < 17 -class ReiserFSLabeling(FSLabeling): - - default_label = "" - - @classmethod - def label_format_ok(cls, label): - return len(label) < 17 - - class XFSLabeling(FSLabeling): default_label = "" diff --git a/blivet/tasks/fsmkfs.py b/blivet/tasks/fsmkfs.py index 408b0ef7a..832be49d4 100644 --- a/blivet/tasks/fsmkfs.py +++ b/blivet/tasks/fsmkfs.py @@ -316,19 +316,6 @@ def args(self): return ["-F", "-f"] -class ReiserFSMkfs(FSMkfs): - ext = availability.MKREISERFS_APP - label_option = "-l" - nodiscard_option = None - - def get_uuid_args(self, uuid): - return ["-u", uuid] - - @property - def args(self): - return ["-f", "-f"] - - class XFSMkfs(FSMkfs): ext = availability.MKFS_XFS_APP label_option = "-L" diff --git a/blivet/tasks/fssize.py b/blivet/tasks/fssize.py index 31b8d6174..6b3b86723 100644 --- a/blivet/tasks/fssize.py +++ b/blivet/tasks/fssize.py @@ -115,10 +115,6 @@ class NTFSSize(FSSize): tags = _Tags(size="Cluster Size:", count="Volume Size in Clusters:") -class ReiserFSSize(FSSize): - tags = _Tags(size="Blocksize:", count="Count of blocks on the device:") - - class XFSSize(FSSize): tags = _Tags(size="blocksize =", count="dblocks =") diff --git a/blivet/tasks/fsuuid.py b/blivet/tasks/fsuuid.py index 79dd07fc1..3c7c932a5 100644 --- a/blivet/tasks/fsuuid.py +++ b/blivet/tasks/fsuuid.py @@ -60,12 +60,6 @@ def uuid_format_ok(cls, uuid): return cls._check_rfc4122_uuid(uuid) -class ReiserFSUUID(FSUUID): - @classmethod - def uuid_format_ok(cls, uuid): - return cls._check_rfc4122_uuid(uuid) - - class XFSUUID(FSUUID): @classmethod def uuid_format_ok(cls, uuid): diff --git a/blivet/tasks/fswritelabel.py b/blivet/tasks/fswritelabel.py index 1f032c435..6fee83f36 100644 --- a/blivet/tasks/fswritelabel.py +++ b/blivet/tasks/fswritelabel.py @@ -99,14 +99,6 @@ def args(self): return [self.fs.device, self.fs.label] -class ReiserFSWriteLabel(FSWriteLabel): - ext = availability.REISERFSTUNE_APP - - @property - def args(self): - return ["-l", self.fs.label, self.fs.device] - - class XFSWriteLabel(FSWriteLabel): ext = availability.XFSADMIN_APP diff --git a/blivet/tasks/fswriteuuid.py b/blivet/tasks/fswriteuuid.py index 406bf7e45..c040ed76c 100644 --- a/blivet/tasks/fswriteuuid.py +++ b/blivet/tasks/fswriteuuid.py @@ -65,14 +65,6 @@ def args(self): return ["--new-serial=" + self.fs.uuid, self.fs.device] -class ReiserFSWriteUUID(FSWriteUUID): - ext = availability.REISERFSTUNE_APP - - @property - def args(self): - return ["-u", self.fs.uuid, self.fs.device] - - class XFSWriteUUID(FSWriteUUID): ext = availability.XFSADMIN_APP diff --git a/doc/api/formats.rst b/doc/api/formats.rst index 5fec17b4a..08b1a39ef 100644 --- a/doc/api/formats.rst +++ b/doc/api/formats.rst @@ -84,7 +84,6 @@ formats * :class:`~blivet.formats.fs.NoDevFS` (see :ref:`inherited public API `) * :class:`~blivet.formats.fs.NTFS` (see :ref:`inherited public API `) * :class:`~blivet.formats.fs.ProcFS` (see :ref:`inherited public API `) - * :class:`~blivet.formats.fs.ReiserFS` (see :ref:`inherited public API `) * :class:`~blivet.formats.fs.SELinuxFS` (see :ref:`inherited public API `) * :class:`~blivet.formats.fs.SysFS` (see :ref:`inherited public API `) * :class:`~blivet.formats.fs.TmpFS` (see :ref:`inherited public API `) diff --git a/tests/storage_tests/formats_test/fs_test.py b/tests/storage_tests/formats_test/fs_test.py index 66bd6c1a2..ce44a218a 100644 --- a/tests/storage_tests/formats_test/fs_test.py +++ b/tests/storage_tests/formats_test/fs_test.py @@ -89,10 +89,6 @@ class JFSTestCase(fstesting.FSAsRoot): _fs_class = fs.JFS -class ReiserFSTestCase(fstesting.FSAsRoot): - _fs_class = fs.ReiserFS - - class XFSTestCase(fstesting.FSAsRoot): _fs_class = fs.XFS _DEVICE_SIZE = Size("500 MiB") diff --git a/tests/storage_tests/formats_test/labeling_test.py b/tests/storage_tests/formats_test/labeling_test.py index 0702260ab..22a4e569c 100644 --- a/tests/storage_tests/formats_test/labeling_test.py +++ b/tests/storage_tests/formats_test/labeling_test.py @@ -28,10 +28,6 @@ def test_labels(self): self.assertFalse(fs.JFS().label_format_ok("root___filesystem")) self.assertTrue(fs.JFS().label_format_ok("root__filesystem")) - # ReiserFS has a maximum length of 16 - self.assertFalse(fs.ReiserFS().label_format_ok("root___filesystem")) - self.assertTrue(fs.ReiserFS().label_format_ok("root__filesystem")) - # XFS has a maximum length 12 and does not allow spaces self.assertFalse(fs.XFS().label_format_ok("root_filesyst")) self.assertFalse(fs.XFS().label_format_ok("root file")) @@ -80,11 +76,6 @@ class JFSTestCase(fslabeling.LabelingWithRelabeling): _invalid_label = "root___filesystem" -class ReiserFSTestCase(fslabeling.LabelingWithRelabeling): - _fs_class = fs.ReiserFS - _invalid_label = "root___filesystem" - - class HFSTestCase(fslabeling.LabelingAsRoot): _fs_class = fs.HFS _invalid_label = "n" * 28 diff --git a/tests/storage_tests/formats_test/uuid_test.py b/tests/storage_tests/formats_test/uuid_test.py index af35c0ee6..b183c56f8 100644 --- a/tests/storage_tests/formats_test/uuid_test.py +++ b/tests/storage_tests/formats_test/uuid_test.py @@ -15,7 +15,7 @@ def test_uuids(self): """Initialize some filesystems with valid and invalid UUIDs.""" # File systems that accept real UUIDs (RFC 4122) - for fscls in [fs.Ext2FS, fs.JFS, fs.ReiserFS, fs.XFS, fs.HFSPlus]: + for fscls in [fs.Ext2FS, fs.JFS, fs.XFS, fs.HFSPlus]: uuid = "0invalid-uuid-with-righ-tlength00000" self.assertFalse(fscls().uuid_format_ok(uuid)) uuid = "01234567-12341234123401234567891a" @@ -42,7 +42,7 @@ def test_uuids(self): def test_generate_new_uuid(self): """Test that newly generated UUIDs are considered valid""" - for fscls in (fs.Ext2FS, fs.JFS, fs.ReiserFS, fs.XFS, fs.HFSPlus, + for fscls in (fs.Ext2FS, fs.JFS, fs.XFS, fs.HFSPlus, fs.FATFS, fs.NTFS): an_fs = fscls() for _i in range(100): @@ -87,18 +87,6 @@ class JFSTestCase(fsuuid.SetUUIDAfterMkFs): _valid_uuid = "ac54f987-b371-45d9-8846-7d6204081e5c" -class ReiserFSTestCase(fsuuid.SetUUIDWithMkFs): - _fs_class = fs.ReiserFS - _invalid_uuid = "abcdefgh-ijkl-mnop-qrst-uvwxyz123456" - _valid_uuid = "1761023e-bab8-4919-a2cb-f26c89fe1cfe" - - -class ReiserFSAfterTestCase(fsuuid.SetUUIDAfterMkFs): - _fs_class = fs.ReiserFS - _invalid_uuid = "abcdefgh-ijkl-mnop-qrst-uvwxyz123456" - _valid_uuid = "1761023e-bab8-4919-a2cb-f26c89fe1cfe" - - class HFSPlusTestCase(fsuuid.SetUUIDAfterMkFs): _fs_class = fs.HFSPlus _invalid_uuid = "abcdefgh-ijkl-mnop-qrst-uvwxyz123456" From 9353fedf807aae0a3aa33ec1f9f5a4eeec830c15 Mon Sep 17 00:00:00 2001 From: Vojtech Trefny Date: Tue, 24 Oct 2023 14:16:30 +0200 Subject: [PATCH 11/32] Remove JFS support JFS is still supported but isn't really used or actively developed anymore. It also isn't packaged for CentOS/RHEL and not supported by libblockdev. --- blivet/flags.py | 2 -- blivet/formats/fs.py | 19 ------------------- blivet/tasks/availability.py | 2 -- blivet/tasks/fsinfo.py | 5 ----- blivet/tasks/fslabeling.py | 9 --------- blivet/tasks/fsmkfs.py | 11 ----------- blivet/tasks/fssize.py | 4 ---- blivet/tasks/fsuuid.py | 6 ------ blivet/tasks/fswritelabel.py | 8 -------- blivet/tasks/fswriteuuid.py | 8 -------- doc/api/formats.rst | 1 - tests/storage_tests/formats_test/fs_test.py | 4 ---- .../formats_test/labeling_test.py | 9 --------- tests/storage_tests/formats_test/uuid_test.py | 10 ++-------- 14 files changed, 2 insertions(+), 96 deletions(-) diff --git a/blivet/flags.py b/blivet/flags.py index be4eaa899..e1f2279b2 100644 --- a/blivet/flags.py +++ b/blivet/flags.py @@ -50,7 +50,6 @@ def __init__(self): self.noiswmd = False self.gfs2 = True - self.jfs = True # for this flag to take effect, # blockdev.mpath.set_friendly_names(flags.multipath_friendly_names) must @@ -111,7 +110,6 @@ def update_from_boot_cmdline(self): self.multipath = "nompath" not in self.boot_cmdline self.noiswmd = "noiswmd" in self.boot_cmdline self.gfs2 = "gfs2" in self.boot_cmdline - self.jfs = "jfs" in self.boot_cmdline flags = Flags() diff --git a/blivet/formats/fs.py b/blivet/formats/fs.py index b2cbe8ca6..cca0b907a 100644 --- a/blivet/formats/fs.py +++ b/blivet/formats/fs.py @@ -1048,26 +1048,7 @@ class JFS(FS): """ JFS filesystem """ _type = "jfs" - _modules = ["jfs"] - _labelfs = fslabeling.JFSLabeling() - _uuidfs = fsuuid.JFSUUID() - _max_size = Size("8 TiB") - _formattable = True _linux_native = True - _dump = True - _check = True - _info_class = fsinfo.JFSInfo - _mkfs_class = fsmkfs.JFSMkfs - _size_info_class = fssize.JFSSize - _writelabel_class = fswritelabel.JFSWriteLabel - _writeuuid_class = fswriteuuid.JFSWriteUUID - _metadata_size_factor = 0.99 # jfs metadata may take 1% of space - parted_system = fileSystemType["jfs"] - - @property - def supported(self): - """ Is this filesystem a supported type? """ - return self.utils_available if flags.jfs else self._supported register_device_format(JFS) diff --git a/blivet/tasks/availability.py b/blivet/tasks/availability.py index ffbc6b087..d5a6e87dc 100644 --- a/blivet/tasks/availability.py +++ b/blivet/tasks/availability.py @@ -508,7 +508,6 @@ def available_resource(name): E2LABEL_APP = application_by_version("e2label", E2FSPROGS_VERSION) FSCK_HFSPLUS_APP = application("fsck.hfsplus") HFORMAT_APP = application("hformat") -JFSTUNE_APP = application("jfs_tune") KPARTX_APP = application("kpartx") LVMDEVICES = application("lvmdevices") MKDOSFS_APP = application("mkdosfs") @@ -517,7 +516,6 @@ def available_resource(name): MKFS_BTRFS_APP = application("mkfs.btrfs") MKFS_GFS2_APP = application("mkfs.gfs2") MKFS_HFSPLUS_APP = application("mkfs.hfsplus") -MKFS_JFS_APP = application("mkfs.jfs") MKFS_XFS_APP = application("mkfs.xfs") MKNTFS_APP = application("mkntfs") MLABEL_APP = application("mlabel") diff --git a/blivet/tasks/fsinfo.py b/blivet/tasks/fsinfo.py index af35af5ab..e91c2cb60 100644 --- a/blivet/tasks/fsinfo.py +++ b/blivet/tasks/fsinfo.py @@ -78,11 +78,6 @@ class Ext2FSInfo(FSInfo): options = ["-h"] -class JFSInfo(FSInfo): - ext = availability.JFSTUNE_APP - options = ["-l"] - - class NTFSInfo(FSInfo): ext = availability.NTFSINFO_APP options = ["-m"] diff --git a/blivet/tasks/fslabeling.py b/blivet/tasks/fslabeling.py index 1a1c4b841..a76db1d37 100644 --- a/blivet/tasks/fslabeling.py +++ b/blivet/tasks/fslabeling.py @@ -65,15 +65,6 @@ def label_format_ok(cls, label): return len(label) < 12 -class JFSLabeling(FSLabeling): - - default_label = "" - - @classmethod - def label_format_ok(cls, label): - return len(label) < 17 - - class XFSLabeling(FSLabeling): default_label = "" diff --git a/blivet/tasks/fsmkfs.py b/blivet/tasks/fsmkfs.py index 832be49d4..f4d0b3834 100644 --- a/blivet/tasks/fsmkfs.py +++ b/blivet/tasks/fsmkfs.py @@ -293,17 +293,6 @@ def args(self): return [] -class JFSMkfs(FSMkfs): - ext = availability.MKFS_JFS_APP - label_option = "-L" - nodiscard_option = None - get_uuid_args = None - - @property - def args(self): - return ["-q"] - - class NTFSMkfs(FSMkfs): ext = availability.MKNTFS_APP label_option = "-L" diff --git a/blivet/tasks/fssize.py b/blivet/tasks/fssize.py index 6b3b86723..138c86229 100644 --- a/blivet/tasks/fssize.py +++ b/blivet/tasks/fssize.py @@ -107,10 +107,6 @@ class Ext2FSSize(FSSize): tags = _Tags(size="Block size:", count="Block count:") -class JFSSize(FSSize): - tags = _Tags(size="Physical block size:", count="Aggregate size:") - - class NTFSSize(FSSize): tags = _Tags(size="Cluster Size:", count="Volume Size in Clusters:") diff --git a/blivet/tasks/fsuuid.py b/blivet/tasks/fsuuid.py index 3c7c932a5..ce48b999f 100644 --- a/blivet/tasks/fsuuid.py +++ b/blivet/tasks/fsuuid.py @@ -54,12 +54,6 @@ def uuid_format_ok(cls, uuid): for char in (uuid[:4] + uuid[5:])) -class JFSUUID(FSUUID): - @classmethod - def uuid_format_ok(cls, uuid): - return cls._check_rfc4122_uuid(uuid) - - class XFSUUID(FSUUID): @classmethod def uuid_format_ok(cls, uuid): diff --git a/blivet/tasks/fswritelabel.py b/blivet/tasks/fswritelabel.py index 6fee83f36..7f4203f60 100644 --- a/blivet/tasks/fswritelabel.py +++ b/blivet/tasks/fswritelabel.py @@ -83,14 +83,6 @@ def args(self): return [self.fs.device, self.fs.label] -class JFSWriteLabel(FSWriteLabel): - ext = availability.JFSTUNE_APP - - @property - def args(self): - return ["-L", self.fs.label, self.fs.device] - - class NTFSWriteLabel(FSWriteLabel): ext = availability.NTFSLABEL_APP diff --git a/blivet/tasks/fswriteuuid.py b/blivet/tasks/fswriteuuid.py index c040ed76c..dddb03b03 100644 --- a/blivet/tasks/fswriteuuid.py +++ b/blivet/tasks/fswriteuuid.py @@ -49,14 +49,6 @@ def args(self): return ["-U", self.fs.uuid, self.fs.device] -class JFSWriteUUID(FSWriteUUID): - ext = availability.JFSTUNE_APP - - @property - def args(self): - return ["-U", self.fs.uuid, self.fs.device] - - class NTFSWriteUUID(FSWriteUUID): ext = availability.NTFSLABEL_APP diff --git a/doc/api/formats.rst b/doc/api/formats.rst index 08b1a39ef..a77dc1571 100644 --- a/doc/api/formats.rst +++ b/doc/api/formats.rst @@ -77,7 +77,6 @@ formats * :class:`~blivet.formats.fs.HFSPlus` (see :ref:`inherited public API `) * :class:`~blivet.formats.fs.Iso9660FS` (see :ref:`inherited public API `) * :class:`~blivet.formats.fs.UDFFS` (see :ref:`inherited public API `) - * :class:`~blivet.formats.fs.JFS` (see :ref:`inherited public API `) * :class:`~blivet.formats.fs.MacEFIFS` (see :ref:`inherited public API `) * :class:`~blivet.formats.fs.NFS` (see :ref:`inherited public API `) * :class:`~blivet.formats.fs.NFSv4` (see :ref:`inherited public API `) diff --git a/tests/storage_tests/formats_test/fs_test.py b/tests/storage_tests/formats_test/fs_test.py index ce44a218a..3ff5161e2 100644 --- a/tests/storage_tests/formats_test/fs_test.py +++ b/tests/storage_tests/formats_test/fs_test.py @@ -85,10 +85,6 @@ class GFS2TestCase(fstesting.FSAsRoot): _fs_class = fs.GFS2 -class JFSTestCase(fstesting.FSAsRoot): - _fs_class = fs.JFS - - class XFSTestCase(fstesting.FSAsRoot): _fs_class = fs.XFS _DEVICE_SIZE = Size("500 MiB") diff --git a/tests/storage_tests/formats_test/labeling_test.py b/tests/storage_tests/formats_test/labeling_test.py index 22a4e569c..9f48e3078 100644 --- a/tests/storage_tests/formats_test/labeling_test.py +++ b/tests/storage_tests/formats_test/labeling_test.py @@ -24,10 +24,6 @@ def test_labels(self): self.assertFalse(fs.FATFS().label_format_ok("rtfilesystem")) self.assertTrue(fs.FATFS().label_format_ok("rfilesystem")) - # JFS has a maximum length of 16 - self.assertFalse(fs.JFS().label_format_ok("root___filesystem")) - self.assertTrue(fs.JFS().label_format_ok("root__filesystem")) - # XFS has a maximum length 12 and does not allow spaces self.assertFalse(fs.XFS().label_format_ok("root_filesyst")) self.assertFalse(fs.XFS().label_format_ok("root file")) @@ -71,11 +67,6 @@ class Ext2FSTestCase(fslabeling.CompleteLabelingAsRoot): _invalid_label = "root___filesystem" -class JFSTestCase(fslabeling.LabelingWithRelabeling): - _fs_class = fs.JFS - _invalid_label = "root___filesystem" - - class HFSTestCase(fslabeling.LabelingAsRoot): _fs_class = fs.HFS _invalid_label = "n" * 28 diff --git a/tests/storage_tests/formats_test/uuid_test.py b/tests/storage_tests/formats_test/uuid_test.py index b183c56f8..31f1f42c3 100644 --- a/tests/storage_tests/formats_test/uuid_test.py +++ b/tests/storage_tests/formats_test/uuid_test.py @@ -15,7 +15,7 @@ def test_uuids(self): """Initialize some filesystems with valid and invalid UUIDs.""" # File systems that accept real UUIDs (RFC 4122) - for fscls in [fs.Ext2FS, fs.JFS, fs.XFS, fs.HFSPlus]: + for fscls in [fs.Ext2FS, fs.XFS, fs.HFSPlus]: uuid = "0invalid-uuid-with-righ-tlength00000" self.assertFalse(fscls().uuid_format_ok(uuid)) uuid = "01234567-12341234123401234567891a" @@ -42,7 +42,7 @@ def test_uuids(self): def test_generate_new_uuid(self): """Test that newly generated UUIDs are considered valid""" - for fscls in (fs.Ext2FS, fs.JFS, fs.XFS, fs.HFSPlus, + for fscls in (fs.Ext2FS, fs.XFS, fs.HFSPlus, fs.FATFS, fs.NTFS): an_fs = fscls() for _i in range(100): @@ -81,12 +81,6 @@ class Ext2FSAfterTestCase(fsuuid.SetUUIDAfterMkFs): _valid_uuid = "bad19a10-075a-4e99-8922-e4638722a567" -class JFSTestCase(fsuuid.SetUUIDAfterMkFs): - _fs_class = fs.JFS - _invalid_uuid = "abcdefgh-ijkl-mnop-qrst-uvwxyz123456" - _valid_uuid = "ac54f987-b371-45d9-8846-7d6204081e5c" - - class HFSPlusTestCase(fsuuid.SetUUIDAfterMkFs): _fs_class = fs.HFSPlus _invalid_uuid = "abcdefgh-ijkl-mnop-qrst-uvwxyz123456" From 7a2c5c22e27228e32d430546b652e8f5e62891d5 Mon Sep 17 00:00:00 2001 From: Vojtech Trefny Date: Tue, 24 Oct 2023 14:49:41 +0200 Subject: [PATCH 12/32] availability: Do not check e2fsprogs version The minimum required version 1.41 was released 8 years ago, we can now safely assume it is available everywhere. --- blivet/tasks/availability.py | 22 ++++++---------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/blivet/tasks/availability.py b/blivet/tasks/availability.py index d5a6e87dc..c5730b10b 100644 --- a/blivet/tasks/availability.py +++ b/blivet/tasks/availability.py @@ -482,16 +482,6 @@ def available_resource(name): BLOCKDEV_MPATH_PLUGIN = blockdev_plugin("libblockdev mpath plugin", BLOCKDEV_MPATH_TECH) BLOCKDEV_SWAP_PLUGIN = blockdev_plugin("libblockdev swap plugin", BLOCKDEV_SWAP_TECH) -# applications with versions -# we need e2fsprogs newer than 1.41 and we are checking the version by running -# the "e2fsck" tool and parsing its ouput for version number -E2FSPROGS_INFO = AppVersionInfo(app_name="e2fsck", - required_version="1.41.0", - version_opt="-V", - version_regex=r"e2fsck ([0-9+\.]+)[\-rc0-9+]* .*") -E2FSPROGS_VERSION = VersionMethod(E2FSPROGS_INFO) - - # new version of dosftools changed behaviour of many tools DOSFSTOOLS_INFO = AppVersionInfo(app_name="mkdosfs", required_version="4.2", @@ -503,16 +493,16 @@ def available_resource(name): DF_APP = application("df") DOSFSCK_APP = application("dosfsck") DOSFSLABEL_APP = application("dosfslabel") -DUMPE2FS_APP = application_by_version("dumpe2fs", E2FSPROGS_VERSION) -E2FSCK_APP = application_by_version("e2fsck", E2FSPROGS_VERSION) -E2LABEL_APP = application_by_version("e2label", E2FSPROGS_VERSION) +DUMPE2FS_APP = application("dumpe2fs") +E2FSCK_APP = application("e2fsck") +E2LABEL_APP = application("e2label") FSCK_HFSPLUS_APP = application("fsck.hfsplus") HFORMAT_APP = application("hformat") KPARTX_APP = application("kpartx") LVMDEVICES = application("lvmdevices") MKDOSFS_APP = application("mkdosfs") MKDOSFS_NEW_APP = application_by_version("mkdosfs", DOSFSTOOLS_VERSION) -MKE2FS_APP = application_by_version("mke2fs", E2FSPROGS_VERSION) +MKE2FS_APP = application("mke2fs") MKFS_BTRFS_APP = application("mkfs.btrfs") MKFS_GFS2_APP = application("mkfs.gfs2") MKFS_HFSPLUS_APP = application("mkfs.hfsplus") @@ -523,8 +513,8 @@ def available_resource(name): NTFSINFO_APP = application("ntfsinfo") NTFSLABEL_APP = application("ntfslabel") NTFSRESIZE_APP = application("ntfsresize") -RESIZE2FS_APP = application_by_version("resize2fs", E2FSPROGS_VERSION) -TUNE2FS_APP = application_by_version("tune2fs", E2FSPROGS_VERSION) +RESIZE2FS_APP = application("resize2fs") +TUNE2FS_APP = application("tune2fs") XFSADMIN_APP = application("xfs_admin") XFSDB_APP = application("xfs_db") XFSFREEZE_APP = application("xfs_freeze") From 9cb3b73f060be5bfb8a0ae8bbc737b0b711609f5 Mon Sep 17 00:00:00 2001 From: Vojtech Trefny Date: Tue, 24 Oct 2023 14:54:20 +0200 Subject: [PATCH 13/32] availability: Simplify checks for LVM VDO and shared LVM support We now require libblockdev >= 3.0 which will always have these technologies. --- blivet/tasks/availability.py | 28 +++++++++++----------------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/blivet/tasks/availability.py b/blivet/tasks/availability.py index c5730b10b..cad6b0c11 100644 --- a/blivet/tasks/availability.py +++ b/blivet/tasks/availability.py @@ -419,23 +419,17 @@ def available_resource(name): blockdev.LVMTechMode.MODIFY)}) BLOCKDEV_LVM_TECH = BlockDevMethod(BLOCKDEV_LVM) -if hasattr(blockdev.LVMTech, "VDO"): - BLOCKDEV_LVM_VDO = BlockDevTechInfo(plugin_name="lvm", - check_fn=blockdev.lvm_is_tech_avail, - technologies={blockdev.LVMTech.VDO: (blockdev.LVMTechMode.CREATE | - blockdev.LVMTechMode.REMOVE | - blockdev.LVMTechMode.QUERY)}) - BLOCKDEV_LVM_TECH_VDO = BlockDevMethod(BLOCKDEV_LVM_VDO) -else: - BLOCKDEV_LVM_TECH_VDO = _UnavailableMethod(error_msg="Installed version of libblockdev doesn't support LVM VDO technology") - -if hasattr(blockdev.LVMTech, "SHARED"): - BLOCKDEV_LVM_SHARED = BlockDevTechInfo(plugin_name="lvm", - check_fn=blockdev.lvm_is_tech_avail, - technologies={blockdev.LVMTech.SHARED: blockdev.LVMTechMode.MODIFY}) # pylint: disable=no-member - BLOCKDEV_LVM_TECH_SHARED = BlockDevMethod(BLOCKDEV_LVM_SHARED) -else: - BLOCKDEV_LVM_TECH_SHARED = _UnavailableMethod(error_msg="Installed version of libblockdev doesn't support shared LVM technology") +BLOCKDEV_LVM_VDO = BlockDevTechInfo(plugin_name="lvm", + check_fn=blockdev.lvm_is_tech_avail, + technologies={blockdev.LVMTech.VDO: (blockdev.LVMTechMode.CREATE | + blockdev.LVMTechMode.REMOVE | + blockdev.LVMTechMode.QUERY)}) +BLOCKDEV_LVM_TECH_VDO = BlockDevMethod(BLOCKDEV_LVM_VDO) + +BLOCKDEV_LVM_SHARED = BlockDevTechInfo(plugin_name="lvm", + check_fn=blockdev.lvm_is_tech_avail, + technologies={blockdev.LVMTech.SHARED: blockdev.LVMTechMode.MODIFY}) +BLOCKDEV_LVM_TECH_SHARED = BlockDevMethod(BLOCKDEV_LVM_SHARED) # libblockdev mdraid plugin required technologies and modes BLOCKDEV_MD_ALL_MODES = (blockdev.MDTechMode.CREATE | From a188402579a15e2d68020975db066a63dd081840 Mon Sep 17 00:00:00 2001 From: Vojtech Trefny Date: Tue, 24 Oct 2023 15:02:43 +0200 Subject: [PATCH 14/32] availability: Remove unused "mlabel" application --- blivet/tasks/availability.py | 1 - 1 file changed, 1 deletion(-) diff --git a/blivet/tasks/availability.py b/blivet/tasks/availability.py index cad6b0c11..a20f6b3a0 100644 --- a/blivet/tasks/availability.py +++ b/blivet/tasks/availability.py @@ -502,7 +502,6 @@ def available_resource(name): MKFS_HFSPLUS_APP = application("mkfs.hfsplus") MKFS_XFS_APP = application("mkfs.xfs") MKNTFS_APP = application("mkntfs") -MLABEL_APP = application("mlabel") MULTIPATH_APP = application("multipath") NTFSINFO_APP = application("ntfsinfo") NTFSLABEL_APP = application("ntfslabel") From 8333ede2af109f5069df20e0c4c43eef6df48be8 Mon Sep 17 00:00:00 2001 From: Vojtech Trefny Date: Tue, 24 Oct 2023 15:32:44 +0200 Subject: [PATCH 15/32] Add libblockdev filesystem plugin to the list of required plugins --- README.md | 2 +- blivet/__init__.py | 4 ++-- blivet/tasks/availability.py | 15 +++++++++++++++ python-blivet.spec | 1 + 4 files changed, 19 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 7986a84fb..b0655f625 100644 --- a/README.md +++ b/README.md @@ -53,7 +53,7 @@ To install these dependencies use following commands: # dnf install python3-blockdev libblockdev-plugins-all python3-bytesize libbytesize python3-pyparted parted libselinux-python3 * On Debian and Ubuntu based distributions: - # apt-get install python3-blockdev python3-bytesize python3-parted python3-selinux gir1.2-blockdev-3.0 libblockdev-lvm3 libblockdev-btrfs3 libblockdev-swap3 libblockdev-loop3 libblockdev-crypto3 libblockdev-mpath3 libblockdev-dm3 libblockdev-mdraid3 libblockdev-nvdimm3 + # apt-get install python3-blockdev python3-bytesize python3-parted python3-selinux gir1.2-blockdev-3.0 libblockdev-lvm3 libblockdev-btrfs3 libblockdev-swap3 libblockdev-loop3 libblockdev-crypto3 libblockdev-mpath3 libblockdev-dm3 libblockdev-mdraid3 libblockdev-nvdimm3 libblockdev-fs3 ### Development diff --git a/blivet/__init__.py b/blivet/__init__.py index 0253cc61c..7582bb73b 100644 --- a/blivet/__init__.py +++ b/blivet/__init__.py @@ -63,9 +63,9 @@ def log_bd_message(level, msg): from gi.repository import GLib from gi.repository import BlockDev as blockdev if arch.is_s390(): - _REQUESTED_PLUGIN_NAMES = set(("lvm", "btrfs", "swap", "crypto", "loop", "mdraid", "mpath", "dm", "s390", "nvdimm", "nvme")) + _REQUESTED_PLUGIN_NAMES = set(("lvm", "btrfs", "swap", "crypto", "loop", "mdraid", "mpath", "dm", "s390", "nvdimm", "nvme", "fs")) else: - _REQUESTED_PLUGIN_NAMES = set(("lvm", "btrfs", "swap", "crypto", "loop", "mdraid", "mpath", "dm", "nvdimm", "nvme")) + _REQUESTED_PLUGIN_NAMES = set(("lvm", "btrfs", "swap", "crypto", "loop", "mdraid", "mpath", "dm", "nvdimm", "nvme", "fs")) _requested_plugins = blockdev.plugin_specs_from_names(_REQUESTED_PLUGIN_NAMES) try: diff --git a/blivet/tasks/availability.py b/blivet/tasks/availability.py index a20f6b3a0..29c9bf79d 100644 --- a/blivet/tasks/availability.py +++ b/blivet/tasks/availability.py @@ -459,6 +459,20 @@ def available_resource(name): technologies={blockdev.SwapTech.SWAP: BLOCKDEV_SWAP_ALL_MODES}) BLOCKDEV_SWAP_TECH = BlockDevMethod(BLOCKDEV_SWAP) +# libblockdev fs plugin required technologies +# no modes, we will check for specific functionality separately +BLOCKDEV_FS = BlockDevTechInfo(plugin_name="fs", + check_fn=blockdev.fs_is_tech_avail, + technologies={blockdev.FSTech.GENERIC: 0, + blockdev.FSTech.MOUNT: 0, + blockdev.FSTech.EXT2: 0, + blockdev.FSTech.EXT3: 0, + blockdev.FSTech.EXT4: 0, + blockdev.FSTech.XFS: 0, + blockdev.FSTech.VFAT: 0, + blockdev.FSTech.NTFS: 0}) +BLOCKDEV_FS_TECH = BlockDevMethod(BLOCKDEV_FS) + # libblockdev plugins # we can't just check if the plugin is loaded, we also need to make sure # that all technologies required by us our supported (some may be missing @@ -475,6 +489,7 @@ def available_resource(name): BLOCKDEV_MDRAID_PLUGIN = blockdev_plugin("libblockdev mdraid plugin", BLOCKDEV_MD_TECH) BLOCKDEV_MPATH_PLUGIN = blockdev_plugin("libblockdev mpath plugin", BLOCKDEV_MPATH_TECH) BLOCKDEV_SWAP_PLUGIN = blockdev_plugin("libblockdev swap plugin", BLOCKDEV_SWAP_TECH) +BLOCKDEV_FS_PLUGIN = blockdev_plugin("libblockdev fs plugin", BLOCKDEV_FS_TECH) # new version of dosftools changed behaviour of many tools DOSFSTOOLS_INFO = AppVersionInfo(app_name="mkdosfs", diff --git a/python-blivet.spec b/python-blivet.spec index d94874fd0..a22c746f8 100644 --- a/python-blivet.spec +++ b/python-blivet.spec @@ -60,6 +60,7 @@ Requires: python3-blockdev >= %{libblockdevver} Recommends: libblockdev-btrfs >= %{libblockdevver} Recommends: libblockdev-crypto >= %{libblockdevver} Recommends: libblockdev-dm >= %{libblockdevver} +Recommends: libblockdev-fs >= %{libblockdevver} Recommends: libblockdev-loop >= %{libblockdevver} Recommends: libblockdev-lvm >= %{libblockdevver} Recommends: libblockdev-mdraid >= %{libblockdevver} From 7dcdbc911ee24b3142bbadfd1783f14727b4583d Mon Sep 17 00:00:00 2001 From: Vojtech Trefny Date: Tue, 24 Oct 2023 15:34:17 +0200 Subject: [PATCH 16/32] Use libblockdev for the filesystem sync operation libblockdev uses the FIFREEZE and FITHAW ioctls directly so we no longer need to require the command line tool. --- blivet/tasks/availability.py | 1 - blivet/tasks/fssync.py | 26 +++++++++++--------------- 2 files changed, 11 insertions(+), 16 deletions(-) diff --git a/blivet/tasks/availability.py b/blivet/tasks/availability.py index 29c9bf79d..7c366353e 100644 --- a/blivet/tasks/availability.py +++ b/blivet/tasks/availability.py @@ -525,7 +525,6 @@ def available_resource(name): TUNE2FS_APP = application("tune2fs") XFSADMIN_APP = application("xfs_admin") XFSDB_APP = application("xfs_db") -XFSFREEZE_APP = application("xfs_freeze") XFSRESIZE_APP = application("xfs_growfs") XFSREPAIR_APP = application("xfs_repair") diff --git a/blivet/tasks/fssync.py b/blivet/tasks/fssync.py index e6690c281..85cac3b03 100644 --- a/blivet/tasks/fssync.py +++ b/blivet/tasks/fssync.py @@ -24,12 +24,15 @@ from six import add_metaclass from ..errors import FSError -from .. import util from . import availability from . import fstask from . import task +import gi +gi.require_version("BlockDev", "3.0") +from gi.repository import BlockDev + @add_metaclass(abc.ABCMeta) class FSSync(task.BasicApplication, fstask.FSTask): @@ -47,7 +50,7 @@ class XFSSync(FSSync): """ Sync application for XFS. """ - ext = availability.XFSFREEZE_APP + ext = availability.BLOCKDEV_FS_PLUGIN def _get_mountpoint(self, root=None): mountpoint = self.fs.system_mountpoint @@ -59,12 +62,6 @@ def _get_mountpoint(self, root=None): return mountpoint - def _freeze_command(self, root=None): - return [str(self.ext), "-f", self._get_mountpoint(root=root)] - - def _unfreeze_command(self, root=None): - return [str(self.ext), "-u", self._get_mountpoint(root=root)] - def do_task(self, root="/"): # pylint: disable=arguments-differ error_msgs = self.availability_errors @@ -72,17 +69,16 @@ def do_task(self, root="/"): raise FSError("\n".join(error_msgs)) error_msg = None + mountpoint = self._get_mountpoint(root=root) try: - rc = util.run_program(self._freeze_command(root=root), root=root) - except OSError as e: + BlockDev.fs.freeze(mountpoint) + except BlockDev.FSError as e: error_msg = "failed to sync filesytem: %s" % e - error_msg = error_msg or rc try: - rc = util.run_program(self._unfreeze_command(root=root), root=root) - except OSError as e: - error_msg = error_msg or "failed to sync filesystem: %s" % e - error_msg = error_msg or rc + BlockDev.fs.unfreeze(mountpoint) + except BlockDev.FSError as e: + error_msg = "failed to sync filesytem: %s" % e if error_msg: raise FSError(error_msg) From 7bf0c4dcd27db86b3bdaf2253b4c45b31f676163 Mon Sep 17 00:00:00 2001 From: Vojtech Trefny Date: Wed, 25 Oct 2023 08:51:05 +0200 Subject: [PATCH 17/32] swap: Simplify creating swap with UUID BlockDev.swap.mkswap supports the UUID parameter directly since 3.0. --- blivet/formats/swap.py | 19 ++++++------------- .../unit_tests/formats_tests/methods_test.py | 4 +++- 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/blivet/formats/swap.py b/blivet/formats/swap.py index 03aa3cc68..47dd687cf 100644 --- a/blivet/formats/swap.py +++ b/blivet/formats/swap.py @@ -228,19 +228,12 @@ def _teardown(self, **kwargs): def _create(self, **kwargs): log_method_call(self, device=self.device, type=self.type, status=self.status) - if self.uuid is None: - try: - blockdev.swap.mkswap(self.device, label=self.label) - except blockdev.SwapError as err: - raise SwapSpaceError(str(err)) - else: - if not self.uuid_format_ok(self.uuid): - raise FSWriteUUIDError("bad UUID format for swap filesystem") - try: - blockdev.swap.mkswap(self.device, label=self.label, - extra={"-U": self.uuid}) - except blockdev.SwapError as err: - raise SwapSpaceError(str(err)) + if self.uuid and not self.uuid_format_ok(self.uuid): + raise FSWriteUUIDError("bad UUID format for swap filesystem") + try: + blockdev.swap.mkswap(self.device, label=self.label, uuid=self.uuid) + except blockdev.SwapError as err: + raise SwapSpaceError(str(err)) register_device_format(SwapSpace) diff --git a/tests/unit_tests/formats_tests/methods_test.py b/tests/unit_tests/formats_tests/methods_test.py index 4f33c0952..17253e699 100644 --- a/tests/unit_tests/formats_tests/methods_test.py +++ b/tests/unit_tests/formats_tests/methods_test.py @@ -428,7 +428,9 @@ def set_patches(self): def _test_create_backend(self): self.format.exists = False self.format.create() - self.patches["blockdev"].swap.mkswap.assert_called_with(self.format.device, label=self.format.label) # pylint: disable=no-member + self.patches["blockdev"].swap.mkswap.assert_called_with(self.format.device, + label=self.format.label, # pylint: disable=no-member + uuid=self.format.uuid) def _test_setup_backend(self): self.format.setup() From affcc5169e6ed564cb477747b2ba73a3ee0d1584 Mon Sep 17 00:00:00 2001 From: Vojtech Trefny Date: Wed, 25 Oct 2023 09:41:49 +0200 Subject: [PATCH 18/32] Use libblockdev for setting and checking filesystem label and UUID --- blivet/tasks/availability.py | 60 ++++++++++++++--- blivet/tasks/fslabeling.py | 40 +++++------- blivet/tasks/fsuuid.py | 26 +++++--- blivet/tasks/fswritelabel.py | 64 +++++-------------- blivet/tasks/fswriteuuid.py | 52 +++++---------- .../storage_tests/formats_test/fslabeling.py | 14 ++-- tests/storage_tests/formats_test/fstesting.py | 5 +- .../formats_test/labeling_test.py | 7 +- tests/storage_tests/formats_test/uuid_test.py | 10 +-- 9 files changed, 143 insertions(+), 135 deletions(-) diff --git a/blivet/tasks/availability.py b/blivet/tasks/availability.py index 7c366353e..39db81a74 100644 --- a/blivet/tasks/availability.py +++ b/blivet/tasks/availability.py @@ -251,6 +251,43 @@ def availability_errors(self, resource): return [] +class BlockDevFSMethod(Method): + + """ Methods for when application is actually a libblockdev FS plugin functionality. """ + + def __init__(self, check_fn, fstype): + """ Initializer. + + :param check_fn: function used to check for support availability + :param fstype: filesystem type to check for the support availability + """ + self.check_fn = check_fn + self.fstype = fstype + self._availability_errors = None + + def availability_errors(self, resource): + """ Returns [] if the plugin is loaded and functionality available. + + :param resource: a libblockdev plugin + :type resource: :class:`ExternalResource` + + :returns: [] if the name of the plugin is loaded + :rtype: list of str + """ + if "fs" not in blockdev.get_available_plugin_names(): + return ["libblockdev fs plugin not loaded"] + else: + try: + avail, utility = self.check_fn(self.fstype) + except blockdev.FSError as e: + return [str(e)] + if not avail: + return ["libblockdev fs plugin is loaded but some required runtime " + "dependencies are not available: %s" % utility] + else: + return [] + + class DBusMethod(Method): """ Methods for when application is actually a DBus service. """ @@ -334,6 +371,11 @@ def blockdev_plugin(name, blockdev_method): return ExternalResource(blockdev_method, name) +def blockdev_fs_plugin(blockdev_fs_method): + """ Construct an external resource that is a libblockdev FS plugin functionality. """ + return ExternalResource(blockdev_fs_method, "libblockdev FS plugin method") + + def dbus_service(name, dbus_method): """ Construct an external resource that is a DBus service. """ return ExternalResource(dbus_method, name) @@ -473,6 +515,16 @@ def available_resource(name): blockdev.FSTech.NTFS: 0}) BLOCKDEV_FS_TECH = BlockDevMethod(BLOCKDEV_FS) +# libblockdev fs plugin methods +BLOCKDEV_EXT_UUID = blockdev_fs_plugin(BlockDevFSMethod(blockdev.fs.can_set_uuid, "ext2")) +BLOCKDEV_XFS_UUID = blockdev_fs_plugin(BlockDevFSMethod(blockdev.fs.can_set_uuid, "xfs")) +BLOCKDEV_NTFS_UUID = blockdev_fs_plugin(BlockDevFSMethod(blockdev.fs.can_set_uuid, "ntfs")) + +BLOCKDEV_EXT_LABEL = blockdev_fs_plugin(BlockDevFSMethod(blockdev.fs.can_set_label, "ext2")) +BLOCKDEV_XFS_LABEL = blockdev_fs_plugin(BlockDevFSMethod(blockdev.fs.can_set_label, "xfs")) +BLOCKDEV_VFAT_LABEL = blockdev_fs_plugin(BlockDevFSMethod(blockdev.fs.can_set_label, "vfat")) +BLOCKDEV_NTFS_LABEL = blockdev_fs_plugin(BlockDevFSMethod(blockdev.fs.can_set_label, "ntfs")) + # libblockdev plugins # we can't just check if the plugin is loaded, we also need to make sure # that all technologies required by us our supported (some may be missing @@ -491,13 +543,6 @@ def available_resource(name): BLOCKDEV_SWAP_PLUGIN = blockdev_plugin("libblockdev swap plugin", BLOCKDEV_SWAP_TECH) BLOCKDEV_FS_PLUGIN = blockdev_plugin("libblockdev fs plugin", BLOCKDEV_FS_TECH) -# new version of dosftools changed behaviour of many tools -DOSFSTOOLS_INFO = AppVersionInfo(app_name="mkdosfs", - required_version="4.2", - version_opt="--help", - version_regex=r"mkfs\.fat ([0-9+\.]+) .*") -DOSFSTOOLS_VERSION = VersionMethod(DOSFSTOOLS_INFO) - # applications DF_APP = application("df") DOSFSCK_APP = application("dosfsck") @@ -510,7 +555,6 @@ def available_resource(name): KPARTX_APP = application("kpartx") LVMDEVICES = application("lvmdevices") MKDOSFS_APP = application("mkdosfs") -MKDOSFS_NEW_APP = application_by_version("mkdosfs", DOSFSTOOLS_VERSION) MKE2FS_APP = application("mke2fs") MKFS_BTRFS_APP = application("mkfs.btrfs") MKFS_GFS2_APP = application("mkfs.gfs2") diff --git a/blivet/tasks/fslabeling.py b/blivet/tasks/fslabeling.py index a76db1d37..e1a75330a 100644 --- a/blivet/tasks/fslabeling.py +++ b/blivet/tasks/fslabeling.py @@ -23,7 +23,9 @@ from six import add_metaclass -from . import availability +import gi +gi.require_version("BlockDev", "3.0") +from gi.repository import BlockDev @add_metaclass(abc.ABCMeta) @@ -32,9 +34,6 @@ class FSLabeling(object): """An abstract class that represents filesystem labeling actions. """ - default_label = abc.abstractproperty( - doc="Default label set on this filesystem at creation.") - @classmethod @abc.abstractmethod def label_format_ok(cls, label): @@ -46,38 +45,39 @@ def label_format_ok(cls, label): """ raise NotImplementedError + @classmethod + def _blockdev_check_label(cls, fstype, label): + try: + BlockDev.fs.check_label(fstype, label) + except BlockDev.FSError: + return False + else: + return True -class Ext2FSLabeling(FSLabeling): - default_label = "" +class Ext2FSLabeling(FSLabeling): @classmethod def label_format_ok(cls, label): - return len(label) < 17 + return cls._blockdev_check_label("ext2", label) class FATFSLabeling(FSLabeling): - default_label = "" if availability.MKDOSFS_NEW_APP.available else "NO NAME" - @classmethod def label_format_ok(cls, label): - return len(label) < 12 + return cls._blockdev_check_label("vfat", label) class XFSLabeling(FSLabeling): - default_label = "" - @classmethod def label_format_ok(cls, label): - return ' ' not in label and len(label) < 13 + return cls._blockdev_check_label("xfs", label) class HFSLabeling(FSLabeling): - default_label = "Untitled" - @classmethod def label_format_ok(cls, label): return ':' not in label and len(label) < 28 and len(label) > 0 @@ -85,8 +85,6 @@ def label_format_ok(cls, label): class HFSPlusLabeling(FSLabeling): - default_label = "Untitled" - @classmethod def label_format_ok(cls, label): return ':' not in label and 0 < len(label) < 129 @@ -94,17 +92,13 @@ def label_format_ok(cls, label): class NTFSLabeling(FSLabeling): - default_label = "" - @classmethod def label_format_ok(cls, label): - return len(label) < 129 + return cls._blockdev_check_label("ntfs", label) class F2FSLabeling(FSLabeling): - default_label = "" - @classmethod def label_format_ok(cls, label): - return len(label) < 513 + return cls._blockdev_check_label("f2fs", label) diff --git a/blivet/tasks/fsuuid.py b/blivet/tasks/fsuuid.py index ce48b999f..849d7d975 100644 --- a/blivet/tasks/fsuuid.py +++ b/blivet/tasks/fsuuid.py @@ -2,6 +2,10 @@ from six import add_metaclass +import gi +gi.require_version("BlockDev", "3.0") +from gi.repository import BlockDev + @add_metaclass(abc.ABCMeta) class FSUUID(object): @@ -38,26 +42,32 @@ def _check_rfc4122_uuid(cls, uuid): if all(char in "0123456789abcdef" for char in chunk)] return chunklens == [8, 4, 4, 4, 12] + @classmethod + def _blockdev_check_uuid(cls, fstype, uuid): + try: + BlockDev.fs.check_uuid(fstype, uuid) + except BlockDev.FSError: + return False + else: + return True + class Ext2FSUUID(FSUUID): @classmethod def uuid_format_ok(cls, uuid): - return cls._check_rfc4122_uuid(uuid) + return cls._blockdev_check_uuid("ext2", uuid) class FATFSUUID(FSUUID): @classmethod def uuid_format_ok(cls, uuid): - if len(uuid) != 9 or uuid[4] != '-': - return False - return all(char in "0123456789ABCDEF" - for char in (uuid[:4] + uuid[5:])) + return cls._blockdev_check_uuid("vfat", uuid) class XFSUUID(FSUUID): @classmethod def uuid_format_ok(cls, uuid): - return cls._check_rfc4122_uuid(uuid) + return cls._blockdev_check_uuid("xfs", uuid) class HFSPlusUUID(FSUUID): @@ -69,6 +79,4 @@ def uuid_format_ok(cls, uuid): class NTFSUUID(FSUUID): @classmethod def uuid_format_ok(cls, uuid): - if len(uuid) != 16: - return False - return all(char in "0123456789ABCDEF" for char in uuid) + return cls._blockdev_check_uuid("ntfs", uuid) diff --git a/blivet/tasks/fswritelabel.py b/blivet/tasks/fswritelabel.py index 7f4203f60..afea2fc1b 100644 --- a/blivet/tasks/fswritelabel.py +++ b/blivet/tasks/fswritelabel.py @@ -19,84 +19,54 @@ # # Red Hat Author(s): Anne Mulhern -import abc - -from six import add_metaclass - -from .. import util from ..errors import FSWriteLabelError from . import availability from . import fstask from . import task +import gi +gi.require_version("BlockDev", "3.0") +from gi.repository import BlockDev -@add_metaclass(abc.ABCMeta) -class FSWriteLabel(task.BasicApplication, fstask.FSTask): +class FSWriteLabel(task.BasicApplication, fstask.FSTask): """ An abstract class that represents writing a label for a filesystem. """ description = "write filesystem label" - - args = abc.abstractproperty(doc="arguments for writing a label") + fstype = None # IMPLEMENTATION methods - @property - def _set_command(self): - """Get the command to label the filesystem. - - :return: the command - :rtype: list of str - """ - return [str(self.ext)] + self.args - def do_task(self): # pylint: disable=arguments-differ error_msgs = self.availability_errors if error_msgs: raise FSWriteLabelError("\n".join(error_msgs)) - rc = util.run_program(self._set_command) - if rc: - raise FSWriteLabelError("label failed") + try: + BlockDev.fs.set_label(self.fs.device, self.fs.label, self.fstype) + except BlockDev.FSError as e: + raise FSWriteLabelError(str(e)) class DosFSWriteLabel(FSWriteLabel): - ext = availability.DOSFSLABEL_APP - - @property - def args(self): - if availability.MKDOSFS_NEW_APP.available: - if self.fs.label: - return [self.fs.device, self.fs.label] - else: - return [self.fs.device, "--reset"] - else: - return [self.fs.device, self.fs.label] + fstype = "vfat" + ext = availability.BLOCKDEV_VFAT_LABEL class Ext2FSWriteLabel(FSWriteLabel): - ext = availability.E2LABEL_APP - - @property - def args(self): - return [self.fs.device, self.fs.label] + fstype = "ext2" + ext = availability.BLOCKDEV_EXT_LABEL class NTFSWriteLabel(FSWriteLabel): - ext = availability.NTFSLABEL_APP - - @property - def args(self): - return [self.fs.device, self.fs.label] + fstype = "ntfs" + ext = availability.BLOCKDEV_NTFS_LABEL class XFSWriteLabel(FSWriteLabel): - ext = availability.XFSADMIN_APP - - @property - def args(self): - return ["-L", self.fs.label if self.fs.label != "" else "--", self.fs.device] + fstype = "xfs" + ext = availability.BLOCKDEV_XFS_LABEL class UnimplementedFSWriteLabel(fstask.UnimplementedFSTask): diff --git a/blivet/tasks/fswriteuuid.py b/blivet/tasks/fswriteuuid.py index dddb03b03..8168f856e 100644 --- a/blivet/tasks/fswriteuuid.py +++ b/blivet/tasks/fswriteuuid.py @@ -1,68 +1,46 @@ -import abc - -from six import add_metaclass - -from .. import util from ..errors import FSWriteUUIDError from . import availability from . import fstask from . import task +import gi +gi.require_version("BlockDev", "3.0") +from gi.repository import BlockDev -@add_metaclass(abc.ABCMeta) -class FSWriteUUID(task.BasicApplication, fstask.FSTask): +class FSWriteUUID(task.BasicApplication, fstask.FSTask): """ An abstract class that represents writing an UUID for a filesystem. """ description = "write filesystem UUID" - - args = abc.abstractproperty(doc="arguments for writing a UUID") + fstype = None # IMPLEMENTATION methods - @property - def _set_command(self): - """Get the command to set UUID of the filesystem. - - :return: the command - :rtype: list of str - """ - return [str(self.ext)] + self.args - def do_task(self): # pylint: disable=arguments-differ error_msgs = self.availability_errors if error_msgs: raise FSWriteUUIDError("\n".join(error_msgs)) - rc = util.run_program(self._set_command) - if rc: - msg = "setting UUID via {} failed".format(self._set_command) - raise FSWriteUUIDError(msg) + try: + BlockDev.fs.set_uuid(self.fs.device, self.fs.uuid, self.fstype) + except BlockDev.FSError as e: + raise FSWriteUUIDError(str(e)) class Ext2FSWriteUUID(FSWriteUUID): - ext = availability.TUNE2FS_APP - - @property - def args(self): - return ["-U", self.fs.uuid, self.fs.device] + fstype = "ext2" + ext = availability.BLOCKDEV_EXT_UUID class NTFSWriteUUID(FSWriteUUID): - ext = availability.NTFSLABEL_APP - - @property - def args(self): - return ["--new-serial=" + self.fs.uuid, self.fs.device] + fstype = "ntfs" + ext = availability.BLOCKDEV_NTFS_UUID class XFSWriteUUID(FSWriteUUID): - ext = availability.XFSADMIN_APP - - @property - def args(self): - return ["-U", self.fs.uuid, self.fs.device] + fstype = "xfs" + ext = availability.BLOCKDEV_XFS_UUID class UnimplementedFSWriteUUID(fstask.UnimplementedFSTask): diff --git a/tests/storage_tests/formats_test/fslabeling.py b/tests/storage_tests/formats_test/fslabeling.py index ebe0b70a2..80bb048b0 100644 --- a/tests/storage_tests/formats_test/fslabeling.py +++ b/tests/storage_tests/formats_test/fslabeling.py @@ -131,11 +131,14 @@ def test_labeling(self): if an_fs._readlabel.availability_errors or not an_fs.relabels(): self.skipTest("can not read or write label for filesystem %s" % an_fs.name) self.assertIsNone(an_fs.create()) - self.assertEqual(an_fs.read_label(), an_fs._labelfs.default_label) + self.assertEqual(an_fs.read_label(), self._default_label) an_fs.label = "an_fs" self.assertIsNone(an_fs.write_label()) - self.assertEqual(an_fs.read_label(), an_fs.label) + if an_fs.type in ("vfat", "efi"): + self.assertEqual(an_fs.read_label(), an_fs.label.upper()) + else: + self.assertEqual(an_fs.read_label(), an_fs.label) an_fs.label = "" self.assertIsNone(an_fs.write_label()) @@ -157,7 +160,10 @@ def test_creating(self): if an_fs._readlabel.availability_errors: self.skipTest("can not read label for filesystem %s" % an_fs.name) self.assertIsNone(an_fs.create()) - self.assertEqual(an_fs.read_label(), "start") + if an_fs.type in ("vfat", "efi"): + self.assertEqual(an_fs.read_label(), "START") + else: + self.assertEqual(an_fs.read_label(), "start") def test_creating_none(self): """Create a filesystem with the label None. @@ -167,7 +173,7 @@ def test_creating_none(self): if an_fs._readlabel.availability_errors: self.skipTest("can not read label for filesystem %s" % an_fs.name) self.assertIsNone(an_fs.create()) - self.assertEqual(an_fs.read_label(), an_fs._labelfs.default_label) + self.assertEqual(an_fs.read_label(), self._default_label) def test_creating_empty(self): """Create a filesystem with an empty label. diff --git a/tests/storage_tests/formats_test/fstesting.py b/tests/storage_tests/formats_test/fstesting.py index e34584d88..42090a30e 100644 --- a/tests/storage_tests/formats_test/fstesting.py +++ b/tests/storage_tests/formats_test/fstesting.py @@ -132,7 +132,10 @@ def test_labeling(self): self.assertIsNone(an_fs.create()) try: label = an_fs.read_label() - self.assertEqual(label, "label") + if an_fs.type in ("vfat", "efi"): + self.assertEqual(label, "LABEL") + else: + self.assertEqual(label, "label") except FSError: pass diff --git a/tests/storage_tests/formats_test/labeling_test.py b/tests/storage_tests/formats_test/labeling_test.py index 9f48e3078..f5d655f02 100644 --- a/tests/storage_tests/formats_test/labeling_test.py +++ b/tests/storage_tests/formats_test/labeling_test.py @@ -54,33 +54,38 @@ def test_labels(self): class XFSTestCase(fslabeling.CompleteLabelingAsRoot): _fs_class = fs.XFS _invalid_label = "root filesystem" + _default_label = "" _DEVICE_SIZE = Size("500 MiB") class FATFSTestCase(fslabeling.CompleteLabelingAsRoot): _fs_class = fs.FATFS _invalid_label = "root___filesystem" + _default_label = "" class Ext2FSTestCase(fslabeling.CompleteLabelingAsRoot): _fs_class = fs.Ext2FS _invalid_label = "root___filesystem" + _default_label = "" class HFSTestCase(fslabeling.LabelingAsRoot): _fs_class = fs.HFS _invalid_label = "n" * 28 + _default_label = "Untitled" class HFSPlusTestCase(fslabeling.LabelingAsRoot): _fs_class = fs.HFSPlus _invalid_label = "n" * 129 + _default_label = "Untitled" -@unittest.skip("Unable to create NTFS filesystem.") class NTFSTestCase(fslabeling.CompleteLabelingAsRoot): _fs_class = fs.NTFS _invalid_label = "n" * 129 + _default_label = "" class LabelingSwapSpaceTestCase(loopbackedtestcase.LoopBackedTestCase): diff --git a/tests/storage_tests/formats_test/uuid_test.py b/tests/storage_tests/formats_test/uuid_test.py index 31f1f42c3..d397cd499 100644 --- a/tests/storage_tests/formats_test/uuid_test.py +++ b/tests/storage_tests/formats_test/uuid_test.py @@ -28,14 +28,14 @@ def test_uuids(self): self.assertTrue(fscls().uuid_format_ok(uuid)) self.assertFalse(fs.FATFS().uuid_format_ok("1234-56789")) - self.assertFalse(fs.FATFS().uuid_format_ok("abcd-ef00")) - self.assertFalse(fs.FATFS().uuid_format_ok("12345678")) + self.assertTrue(fs.FATFS().uuid_format_ok("abcd-ef00")) + self.assertTrue(fs.FATFS().uuid_format_ok("12345678")) self.assertTrue(fs.FATFS().uuid_format_ok("1234-5678")) self.assertTrue(fs.FATFS().uuid_format_ok("ABCD-EF01")) self.assertFalse(fs.NTFS().uuid_format_ok("12345678901234567")) self.assertFalse(fs.NTFS().uuid_format_ok("abcdefgh")) - self.assertFalse(fs.NTFS().uuid_format_ok("abcdefabcdefabcd")) + self.assertTrue(fs.NTFS().uuid_format_ok("abcdefabcdefabcd")) self.assertTrue(fs.NTFS().uuid_format_ok("1234567890123456")) self.assertTrue(fs.NTFS().uuid_format_ok("ABCDEFABCDEFABCD")) @@ -65,7 +65,7 @@ class XFSAfterTestCase(fsuuid.SetUUIDAfterMkFs): class FATFSTestCase(fsuuid.SetUUIDWithMkFs): _fs_class = fs.FATFS - _invalid_uuid = "c87ab0e1" + _invalid_uuid = "z87ab0e1" _valid_uuid = "DEAD-BEEF" @@ -89,7 +89,7 @@ class HFSPlusTestCase(fsuuid.SetUUIDAfterMkFs): class NTFSTestCase(fsuuid.SetUUIDAfterMkFs): _fs_class = fs.NTFS - _invalid_uuid = "b22193477ac947fb" + _invalid_uuid = "z22193477ac947fb" _valid_uuid = "BC3B34461B8344A6" From fbd2e6296bbb981925a9d31936814a37b064552c Mon Sep 17 00:00:00 2001 From: Vojtech Trefny Date: Wed, 25 Oct 2023 14:39:34 +0200 Subject: [PATCH 19/32] Use libblockdev for filesystem resizing --- blivet/formats/fs.py | 2 +- blivet/tasks/availability.py | 38 +++++++--- blivet/tasks/fsresize.py | 143 +++++++---------------------------- 3 files changed, 55 insertions(+), 128 deletions(-) diff --git a/blivet/formats/fs.py b/blivet/formats/fs.py index cca0b907a..4aa2c1bd4 100644 --- a/blivet/formats/fs.py +++ b/blivet/formats/fs.py @@ -1438,7 +1438,7 @@ def _size_option(self, size): This is not impossible, since a special option for mounting is size=%. """ - return "size=%s" % (self._resize.size_fmt % size.convert_to(self._resize.unit)) + return "size=%sm" % size.convert_to(self._resize.unit) def _get_options(self): # Returns the regular mount options with the special size option, diff --git a/blivet/tasks/availability.py b/blivet/tasks/availability.py index 39db81a74..8037328e8 100644 --- a/blivet/tasks/availability.py +++ b/blivet/tasks/availability.py @@ -255,12 +255,14 @@ class BlockDevFSMethod(Method): """ Methods for when application is actually a libblockdev FS plugin functionality. """ - def __init__(self, check_fn, fstype): + def __init__(self, operation, check_fn, fstype): """ Initializer. + :param operation: operation to check for support availability :param check_fn: function used to check for support availability :param fstype: filesystem type to check for the support availability """ + self.operation = operation self.check_fn = check_fn self.fstype = fstype self._availability_errors = None @@ -278,7 +280,10 @@ def availability_errors(self, resource): return ["libblockdev fs plugin not loaded"] else: try: - avail, utility = self.check_fn(self.fstype) + if self.operation in (FSOperation.UUID, FSOperation.LABEL): + avail, utility = self.check_fn(self.fstype) + elif self.operation == FSOperation.RESIZE: + avail, _mode, utility = self.check_fn(self.fstype) except blockdev.FSError as e: return [str(e)] if not avail: @@ -371,7 +376,7 @@ def blockdev_plugin(name, blockdev_method): return ExternalResource(blockdev_method, name) -def blockdev_fs_plugin(blockdev_fs_method): +def blockdev_fs_plugin_operation(blockdev_fs_method): """ Construct an external resource that is a libblockdev FS plugin functionality. """ return ExternalResource(blockdev_fs_method, "libblockdev FS plugin method") @@ -515,15 +520,26 @@ def available_resource(name): blockdev.FSTech.NTFS: 0}) BLOCKDEV_FS_TECH = BlockDevMethod(BLOCKDEV_FS) + # libblockdev fs plugin methods -BLOCKDEV_EXT_UUID = blockdev_fs_plugin(BlockDevFSMethod(blockdev.fs.can_set_uuid, "ext2")) -BLOCKDEV_XFS_UUID = blockdev_fs_plugin(BlockDevFSMethod(blockdev.fs.can_set_uuid, "xfs")) -BLOCKDEV_NTFS_UUID = blockdev_fs_plugin(BlockDevFSMethod(blockdev.fs.can_set_uuid, "ntfs")) - -BLOCKDEV_EXT_LABEL = blockdev_fs_plugin(BlockDevFSMethod(blockdev.fs.can_set_label, "ext2")) -BLOCKDEV_XFS_LABEL = blockdev_fs_plugin(BlockDevFSMethod(blockdev.fs.can_set_label, "xfs")) -BLOCKDEV_VFAT_LABEL = blockdev_fs_plugin(BlockDevFSMethod(blockdev.fs.can_set_label, "vfat")) -BLOCKDEV_NTFS_LABEL = blockdev_fs_plugin(BlockDevFSMethod(blockdev.fs.can_set_label, "ntfs")) +class FSOperation(): + UUID = 0 + LABEL = 1 + RESIZE = 2 + + +BLOCKDEV_EXT_UUID = blockdev_fs_plugin_operation(BlockDevFSMethod(FSOperation.UUID, blockdev.fs.can_set_uuid, "ext2")) +BLOCKDEV_XFS_UUID = blockdev_fs_plugin_operation(BlockDevFSMethod(FSOperation.UUID, blockdev.fs.can_set_uuid, "xfs")) +BLOCKDEV_NTFS_UUID = blockdev_fs_plugin_operation(BlockDevFSMethod(FSOperation.UUID, blockdev.fs.can_set_uuid, "ntfs")) + +BLOCKDEV_EXT_LABEL = blockdev_fs_plugin_operation(BlockDevFSMethod(FSOperation.LABEL, blockdev.fs.can_set_label, "ext2")) +BLOCKDEV_XFS_LABEL = blockdev_fs_plugin_operation(BlockDevFSMethod(FSOperation.LABEL, blockdev.fs.can_set_label, "xfs")) +BLOCKDEV_VFAT_LABEL = blockdev_fs_plugin_operation(BlockDevFSMethod(FSOperation.LABEL, blockdev.fs.can_set_label, "vfat")) +BLOCKDEV_NTFS_LABEL = blockdev_fs_plugin_operation(BlockDevFSMethod(FSOperation.LABEL, blockdev.fs.can_set_label, "ntfs")) + +BLOCKDEV_EXT_RESIZE = blockdev_fs_plugin_operation(BlockDevFSMethod(FSOperation.RESIZE, blockdev.fs.can_resize, "ext2")) +BLOCKDEV_XFS_RESIZE = blockdev_fs_plugin_operation(BlockDevFSMethod(FSOperation.RESIZE, blockdev.fs.can_resize, "xfs")) +BLOCKDEV_NTFS_RESIZE = blockdev_fs_plugin_operation(BlockDevFSMethod(FSOperation.RESIZE, blockdev.fs.can_resize, "ntfs")) # libblockdev plugins # we can't just check if the plugin is loaded, we also need to make sure diff --git a/blivet/tasks/fsresize.py b/blivet/tasks/fsresize.py index e101e6096..e420fe966 100644 --- a/blivet/tasks/fsresize.py +++ b/blivet/tasks/fsresize.py @@ -19,16 +19,8 @@ # # Red Hat Author(s): Anne Mulhern -import abc -import os -import tempfile - -from contextlib import contextmanager -from six import add_metaclass - from ..errors import FSError -from ..size import B, KiB, MiB, GiB, KB, MB, GB -from ..import util +from ..size import B, MiB from . import availability from . import task @@ -38,36 +30,23 @@ import logging log = logging.getLogger("blivet") +import gi +gi.require_version("BlockDev", "3.0") +from gi.repository import BlockDev + -@add_metaclass(abc.ABCMeta) class FSResizeTask(fstask.FSTask): """ The abstract properties that any resize task must have. """ - size_fmt = abc.abstractproperty(doc="Size format string.") - -@add_metaclass(abc.ABCMeta) class FSResize(task.BasicApplication, FSResizeTask): """ An abstract class for resizing a filesystem. """ description = "resize filesystem" - args = abc.abstractproperty(doc="Resize arguments.") - # IMPLEMENTATION methods - @abc.abstractmethod - def size_spec(self): - """ Returns a string specification for the target size of the command. - :returns: size specification - :rtype: str - """ - raise NotImplementedError() - - def _resize_command(self): - return [str(self.ext)] + self.args - def do_task(self): # pylint: disable=arguments-differ """ Resize the device. @@ -78,118 +57,50 @@ def do_task(self): # pylint: disable=arguments-differ raise FSError("\n".join(error_msgs)) try: - ret = util.run_program(self._resize_command()) - except OSError as e: - raise FSError(e) - - if ret: - raise FSError("resize failed: %s" % ret) + BlockDev.fs.resize(self.fs.device, self.fs.target_size.convert_to(B), self.fs.type) + except BlockDev.FSError as e: + raise FSError(str(e)) class Ext2FSResize(FSResize): - ext = availability.RESIZE2FS_APP + ext = availability.BLOCKDEV_EXT_RESIZE unit = MiB - # No bytes specification is described in the man pages. A number without - # any suffix is interpreted as indicating the number of filesystem blocks. - # A suffix of "s" specifies a 512 byte sector. It is omitted here because - # the lookup is only by standard binary units. - size_fmt = {KiB: "%dK", MiB: "%dM", GiB: "%dG"}[unit] - - def size_spec(self): - return self.size_fmt % self.fs.target_size.convert_to(self.unit) - - @property - def args(self): - return ["-p", self.fs.device, self.size_spec()] - class NTFSResize(FSResize): - ext = availability.NTFSRESIZE_APP + ext = availability.BLOCKDEV_NTFS_RESIZE unit = B - size_fmt = {B: "%d", KB: "%dK", MB: "%dM", GB: "%dG"}[unit] - - def size_spec(self): - return self.size_fmt % self.fs.target_size.convert_to(self.unit) - - @property - def args(self): - return [ - "-ff", # need at least two 'f's to fully suppress interaction - "-s", self.size_spec(), - self.fs.device - ] class XFSResize(FSResize): - ext = availability.XFSRESIZE_APP + ext = availability.BLOCKDEV_XFS_RESIZE unit = B - size_fmt = None - - @contextmanager - def _do_temp_mount(self): - if self.fs.status: - yield - else: - dev_name = os.path.basename(self.fs.device) - tmpdir = tempfile.mkdtemp(prefix="xfs-tempmount-%s" % dev_name) - log.debug("mounting XFS on '%s' to '%s' for resize", self.fs.device, tmpdir) - try: - self.fs.mount(mountpoint=tmpdir) - except FSError as e: - raise FSError("Failed to mount XFS filesystem for resize: %s" % str(e)) - - try: - yield - finally: - util.umount(mountpoint=tmpdir) - os.rmdir(tmpdir) - - def _get_block_size(self): - if self.fs._current_info: - # this should be set by update_size_info() - for line in self.fs._current_info.split("\n"): - if line.startswith("blocksize ="): - return int(line.split("=")[-1]) - - raise FSError("Failed to get XFS filesystem block size for resize") - - def size_spec(self): - # size for xfs_growfs is in blocks - return str(self.fs.target_size.convert_to(self.unit) / self._get_block_size()) - - @property - def args(self): - return [self.fs.system_mountpoint, "-D", self.size_spec()] - - def do_task(self): # pylint: disable=arguments-differ - """ Resizes the XFS format. """ - - with self._do_temp_mount(): - super(XFSResize, self).do_task() class TmpFSResize(FSResize): - - ext = availability.MOUNT_APP + ext = availability.BLOCKDEV_FS_PLUGIN unit = MiB - size_fmt = {KiB: "%dk", MiB: "%dm", GiB: "%dg"}[unit] - def size_spec(self): - return "size=%s" % (self.size_fmt % self.fs.target_size.convert_to(self.unit)) + def do_task(self): # pylint: disable=arguments-differ + """ Resize the device. + + :raises FSError: on failure + """ + error_msgs = self.availability_errors + if error_msgs: + raise FSError("\n".join(error_msgs)) - @property - def args(self): # This is too closely mixed in w/ TmpFS object, due to the # fact that resizing is done by mounting and that the options are # therefore mount options. The situation is hard to avoid, though. opts = self.fs.mountopts or ",".join(self.fs._mount.options) - options = ("remount", opts, self.size_spec()) - return ['-o', ",".join(options), self.fs._type, self.fs.system_mountpoint] + options = ("remount", opts, "size=%dm" % self.fs.target_size.convert_to(MiB)) + try: + BlockDev.fs.mount(device=None, mountpoint=self.fs.system_mountpoint, fstype=self.fs._type, options=",".join(options)) + except BlockDev.FSError as e: + raise FSError(str(e)) -class UnimplementedFSResize(dfresize.UnimplementedDFResize, FSResizeTask): - @property - def size_fmt(self): - raise NotImplementedError() +class UnimplementedFSResize(dfresize.UnimplementedDFResize, FSResizeTask): + unit = B From 7fee5e0638831331a017f0fbcc19e900077d4918 Mon Sep 17 00:00:00 2001 From: Vojtech Trefny Date: Thu, 26 Oct 2023 11:01:11 +0200 Subject: [PATCH 20/32] Use libblockdev for getting filesystem info and size --- blivet/tasks/availability.py | 7 ++++- blivet/tasks/fsinfo.py | 50 ++++++++++++++++------------------- blivet/tasks/fsminsize.py | 10 +------ blivet/tasks/fssize.py | 51 ++++++++---------------------------- 4 files changed, 41 insertions(+), 77 deletions(-) diff --git a/blivet/tasks/availability.py b/blivet/tasks/availability.py index 8037328e8..1d6c42086 100644 --- a/blivet/tasks/availability.py +++ b/blivet/tasks/availability.py @@ -280,7 +280,7 @@ def availability_errors(self, resource): return ["libblockdev fs plugin not loaded"] else: try: - if self.operation in (FSOperation.UUID, FSOperation.LABEL): + if self.operation in (FSOperation.UUID, FSOperation.LABEL, FSOperation.INFO): avail, utility = self.check_fn(self.fstype) elif self.operation == FSOperation.RESIZE: avail, _mode, utility = self.check_fn(self.fstype) @@ -526,6 +526,7 @@ class FSOperation(): UUID = 0 LABEL = 1 RESIZE = 2 + INFO = 3 BLOCKDEV_EXT_UUID = blockdev_fs_plugin_operation(BlockDevFSMethod(FSOperation.UUID, blockdev.fs.can_set_uuid, "ext2")) @@ -541,6 +542,10 @@ class FSOperation(): BLOCKDEV_XFS_RESIZE = blockdev_fs_plugin_operation(BlockDevFSMethod(FSOperation.RESIZE, blockdev.fs.can_resize, "xfs")) BLOCKDEV_NTFS_RESIZE = blockdev_fs_plugin_operation(BlockDevFSMethod(FSOperation.RESIZE, blockdev.fs.can_resize, "ntfs")) +BLOCKDEV_EXT_INFO = blockdev_fs_plugin_operation(BlockDevFSMethod(FSOperation.INFO, blockdev.fs.can_get_size, "ext2")) +BLOCKDEV_XFS_INFO = blockdev_fs_plugin_operation(BlockDevFSMethod(FSOperation.INFO, blockdev.fs.can_get_size, "xfs")) +BLOCKDEV_NTFS_INFO = blockdev_fs_plugin_operation(BlockDevFSMethod(FSOperation.INFO, blockdev.fs.can_get_size, "ntfs")) + # libblockdev plugins # we can't just check if the plugin is loaded, we also need to make sure # that all technologies required by us our supported (some may be missing diff --git a/blivet/tasks/fsinfo.py b/blivet/tasks/fsinfo.py index e91c2cb60..ea092b0e3 100644 --- a/blivet/tasks/fsinfo.py +++ b/blivet/tasks/fsinfo.py @@ -24,12 +24,15 @@ from six import add_metaclass from ..errors import FSError -from .. import util from . import availability from . import fstask from . import task +import gi +gi.require_version("BlockDev", "3.0") +from gi.repository import BlockDev + @add_metaclass(abc.ABCMeta) class FSInfo(task.BasicApplication, fstask.FSTask): @@ -38,17 +41,9 @@ class FSInfo(task.BasicApplication, fstask.FSTask): description = "filesystem info" - options = abc.abstractproperty( - doc="Options for invoking the application.") - - @property - def _info_command(self): - """ Returns the command for reading filesystem information. - - :returns: a list of appropriate options - :rtype: list of str - """ - return [str(self.ext)] + self.options + [self.fs.device] + @abc.abstractmethod + def _get_info(self): + raise NotImplementedError def do_task(self): # pylint: disable=arguments-differ """ Returns information from the command. @@ -61,31 +56,32 @@ def do_task(self): # pylint: disable=arguments-differ if error_msgs: raise FSError("\n".join(error_msgs)) - error_msg = None try: - (rc, out) = util.run_program_and_capture_output(self._info_command) - if rc: - error_msg = "failed to gather fs info: %s" % rc - except OSError as e: - error_msg = "failed to gather fs info: %s" % e - if error_msg: - raise FSError(error_msg) - return out + info = self._get_info() + except BlockDev.FSError as e: + raise FSError("failed to gather fs info: %s" % e) + return info class Ext2FSInfo(FSInfo): - ext = availability.DUMPE2FS_APP - options = ["-h"] + ext = availability.BLOCKDEV_EXT_INFO + + def _get_info(self): + return BlockDev.fs.ext2_get_info(self.fs.device) class NTFSInfo(FSInfo): - ext = availability.NTFSINFO_APP - options = ["-m"] + ext = availability.BLOCKDEV_NTFS_INFO + + def _get_info(self): + return BlockDev.fs.ntfs_get_info(self.fs.device) class XFSInfo(FSInfo): - ext = availability.XFSDB_APP - options = ["-c", "sb 0", "-c", "p dblocks", "-c", "p blocksize", "-r"] + ext = availability.BLOCKDEV_XFS_INFO + + def _get_info(self): + return BlockDev.fs.xfs_get_info(self.fs.device) class UnimplementedFSInfo(fstask.UnimplementedFSTask): diff --git a/blivet/tasks/fsminsize.py b/blivet/tasks/fsminsize.py index 620b91f73..eac2fe567 100644 --- a/blivet/tasks/fsminsize.py +++ b/blivet/tasks/fsminsize.py @@ -91,15 +91,7 @@ def _extract_block_size(self): if self.fs._current_info is None: return None - block_size = None - for line in (l.strip() for l in self.fs._current_info.splitlines() if l.startswith("Block size:")): - try: - block_size = int(line.split(" ")[-1]) - break - except ValueError: - continue - - return Size(block_size) if block_size else None + return Size(self.fs._current_info.block_size) def _extract_num_blocks(self, info): """ Extract the number of blocks from the resizefs info. diff --git a/blivet/tasks/fssize.py b/blivet/tasks/fssize.py index 138c86229..79ceda5e0 100644 --- a/blivet/tasks/fssize.py +++ b/blivet/tasks/fssize.py @@ -20,7 +20,6 @@ # Red Hat Author(s): Anne Mulhern import abc -from collections import namedtuple import six @@ -32,9 +31,6 @@ from . import fstask from . import task -_tags = ("count", "size") -_Tags = namedtuple("_Tags", _tags) - @six.add_metaclass(abc.ABCMeta) class FSSize(fstask.FSTask): @@ -42,9 +38,6 @@ class FSSize(fstask.FSTask): """ An abstract class that represents size information extraction. """ description = "current filesystem size" - tags = abc.abstractproperty( - doc="Strings used for extracting components of size.") - # TASK methods @property @@ -55,6 +48,10 @@ def _availability_errors(self): def depends_on(self): return [self.fs._info] + @abc.abstractmethod + def _get_size(self): + raise NotImplementedError + # IMPLEMENTATION methods def do_task(self): # pylint: disable=arguments-differ @@ -71,48 +68,22 @@ def do_task(self): # pylint: disable=arguments-differ if self.fs._current_info is None: raise FSError("No info available for size computation.") - # Setup initial values - values = {} - for k in _tags: - values[k] = None - - # Attempt to set values from info - for line in (l.strip() for l in self.fs._current_info.splitlines()): - key = six.next((k for k in _tags if line.startswith(getattr(self.tags, k))), None) - if not key: - continue - - if values[key] is not None: - raise FSError("found two matches for key %s" % key) - - # Look for last numeric value in matching line - fields = line.split() - fields.reverse() - for field in fields: - try: - values[key] = int(field) - break - except ValueError: - continue - - # Raise an error if a value is missing - missing = six.next((k for k in _tags if values[k] is None), None) - if missing is not None: - raise FSError("Failed to parse info for %s." % missing) - - return values["count"] * Size(values["size"]) + return self._get_size() class Ext2FSSize(FSSize): - tags = _Tags(size="Block size:", count="Block count:") + def _get_size(self): + return Size(self.fs._current_info.block_size * self.fs._current_info.block_count) class NTFSSize(FSSize): - tags = _Tags(size="Cluster Size:", count="Volume Size in Clusters:") + def _get_size(self): + return Size(self.fs._current_info.size) class XFSSize(FSSize): - tags = _Tags(size="blocksize =", count="dblocks =") + def _get_size(self): + return Size(self.fs._current_info.block_size * self.fs._current_info.block_count) class TmpFSSize(task.BasicApplication, fstask.FSTask): From ffa9d7bb20da14b1ea960afe167d30f15231f6c2 Mon Sep 17 00:00:00 2001 From: Vojtech Trefny Date: Thu, 26 Oct 2023 11:14:21 +0200 Subject: [PATCH 21/32] Use libblockdev for reading filesystem label We can now use the FSInfo task to get the filesystem label too. --- blivet/formats/fs.py | 7 ++++ blivet/tasks/availability.py | 1 + blivet/tasks/fsreadlabel.py | 70 ++++++------------------------------ 3 files changed, 18 insertions(+), 60 deletions(-) diff --git a/blivet/formats/fs.py b/blivet/formats/fs.py index 4aa2c1bd4..4e30a9475 100644 --- a/blivet/formats/fs.py +++ b/blivet/formats/fs.py @@ -661,6 +661,13 @@ def read_label(self): if not self._readlabel.available: raise FSReadLabelError("can not read label for filesystem %s" % self.type) + + try: + if self._info.available: + self._current_info = self._info.do_task() + except FSError as e: + log.info("Failed to obtain info for device %s: %s", self.device, e) + return self._readlabel.do_task() def write_label(self, dry_run=False): diff --git a/blivet/tasks/availability.py b/blivet/tasks/availability.py index 1d6c42086..4c823f59a 100644 --- a/blivet/tasks/availability.py +++ b/blivet/tasks/availability.py @@ -545,6 +545,7 @@ class FSOperation(): BLOCKDEV_EXT_INFO = blockdev_fs_plugin_operation(BlockDevFSMethod(FSOperation.INFO, blockdev.fs.can_get_size, "ext2")) BLOCKDEV_XFS_INFO = blockdev_fs_plugin_operation(BlockDevFSMethod(FSOperation.INFO, blockdev.fs.can_get_size, "xfs")) BLOCKDEV_NTFS_INFO = blockdev_fs_plugin_operation(BlockDevFSMethod(FSOperation.INFO, blockdev.fs.can_get_size, "ntfs")) +BLOCKDEV_VFAT_INFO = blockdev_fs_plugin_operation(BlockDevFSMethod(FSOperation.INFO, blockdev.fs.can_get_size, "vfat")) # libblockdev plugins # we can't just check if the plugin is loaded, we also need to make sure diff --git a/blivet/tasks/fsreadlabel.py b/blivet/tasks/fsreadlabel.py index 6f2c8a021..5c6003a36 100644 --- a/blivet/tasks/fsreadlabel.py +++ b/blivet/tasks/fsreadlabel.py @@ -20,12 +20,10 @@ # Red Hat Author(s): Anne Mulhern import abc -import re from six import add_metaclass from ..errors import FSReadLabelError -from .. import util from . import availability from . import fstask @@ -38,36 +36,11 @@ class FSReadLabel(task.BasicApplication, fstask.FSTask): """ An abstract class that represents reading a filesystem's label. """ description = "read filesystem label" - label_regex = abc.abstractproperty( - doc="Matches the string output by the reading application.") - - args = abc.abstractproperty(doc="arguments for reading a label.") - - # IMPLEMENTATION methods - @property - def _read_command(self): - """Get the command to read the filesystem label. - - :return: the command - :rtype: list of str - """ - return [str(self.ext)] + self.args - - def _extract_label(self, labelstr): - """Extract the label from an output string. - - :param str labelstr: the string containing the label information + def depends_on(self): + return [self.fs._info] - :return: the label - :rtype: str - - Raises an FSReadLabelError if the label can not be extracted. - """ - match = re.match(self.label_regex, labelstr) - if match is None: - raise FSReadLabelError("Unknown format for application %s" % self.ext) - return match.group('label') + # IMPLEMENTATION methods def do_task(self): # pylint: disable=arguments-differ """ Get the label. @@ -79,49 +52,26 @@ def do_task(self): # pylint: disable=arguments-differ if error_msgs: raise FSReadLabelError("\n".join(error_msgs)) - (rc, out) = util.run_program_and_capture_output(self._read_command) - if rc != 0: - raise FSReadLabelError("read label failed") - - label = out.strip() + if self.fs._current_info is None: + raise FSReadLabelError("No info available for size computation.") - return label if label == "" else self._extract_label(label) + return self.fs._current_info.label class DosFSReadLabel(FSReadLabel): - ext = availability.DOSFSLABEL_APP - label_regex = r'(?P