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

@@ -287,8 +287,13 @@ def process(self, callbacks=None, devices=None, fstab=None, dry_run=None):
             # get (b)efore (a)ction.(e)xecute fstab entry
             # (device may not exist afterwards)
             if fstab is not None:
-                entry = fstab.entry_from_device(action.device)
-                bae_entry = fstab.find_entry(entry=entry)
+                try:
+                    entry = fstab.entry_from_device(action.device)
+                except ValueError:
+                    # this device should not be in fstab
+                    bae_entry = None
+                else:
+                    bae_entry = fstab.find_entry(entry=entry)
 
             with blivet_lock:
 

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)

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,52 +370,42 @@ 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
         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:
+        if self.src_file is None:
             return
-        else:
-            self.src_file = src_file
 
         self._table.errcb = self._parser_errcb
-        try:
-            self._table.parse_fstab(self.src_file)
-        except Exception as e:  # pylint: disable=broad-except
-            # libmount throws general Exception. Okay...
-            if str(e) == "No such file or directory":
-                log.warning("Fstab file '%s' does not exist", self.src_file)
-            else:
-                raise
+
+        if not os.path.isfile(self.src_file):
+            raise FileNotFoundError("Fstab file '%s' does not exist" % self.src_file)
+
+        self._table.parse_fstab(self.src_file)
+
 
     def find_device(self, blivet, spec=None, mntops=None, blkid_tab=None, crypt_tab=None, *, entry=None):
         """ Find a blivet device, based on spec or entry. Mount options can be used to refine the search.
@@ -535,7 +548,7 @@ def add_entry(self, spec=None, file=None, vfstype=None, mntops=None,
             _entry.freq = 0
 
         if passno is not None:
-            _entry.passno = freq
+            _entry.passno = passno
         elif _entry.passno is None:
             # 'passno' represents order of fsck run at the boot time (default: 0, i.e. disabled).
             # '/' should have 1, other checked should have 2
@@ -569,6 +582,8 @@ def remove_entry(self, spec=None, file=None, *, entry=None):
         fs = self.find_entry(spec, file, entry=entry)
         if fs:
             self._table.remove_fs(fs.entry)
+        else:
+            raise ValueError("Cannot remove entry (%s) from fstab, because it is not there" % entry)
 
     def write(self, dest_file=None):
         """ Commit the self._table into the self._dest_file. Setting dest_file parameter overrides
@@ -664,6 +679,34 @@ def find_entry(self, spec=None, file=None, *, entry=None):
 
         return None
 
+    def _get_spec(self, device):
+        """ Resolve which device spec should be used and return it in a form accepted by fstab.
+            Returns None if desired spec was not found
+        """
+
+        # Use device specific spec type if it is set
+        # Use "globally" set (on FSTabManager level) spec type otherwise
+
+        spec = None
+        spec_type = device.format.fstab.spec_type or self.spec_type
+
+        if spec_type == "LABEL" and device.format.label:
+            spec = "LABEL=%s" % device.format.label
+        elif spec_type == "PARTUUID" and device.uuid:
+            spec = "PARTUUID=%s" % device.uuid
+        elif spec_type == "PARTLABEL" and device.format.name:
+            spec = "PARTLABEL=%s" % device.format.name
+        elif spec_type == "PATH":
+            spec = device.path
+        elif device.format.uuid:
+            # default choice
+            spec = "UUID=%s" % device.format.uuid
+        else:
+            # if everything else failed, let blivet decide
+            return None
+
+        return spec
+
     def update(self, action, bae_entry):
         """ Update fstab based on action type and device. Does not commit changes to a file.
 
@@ -689,22 +732,56 @@ def update(self, action, bae_entry):
         if action.is_create and action.device.format.mountable:
             # add the device to the fstab
             # make sure it is not already present there
-            entry = self.entry_from_device(action.device)
-            found = self.find_entry(entry=entry)
+            try:
+                entry = self.entry_from_device(action.device)
+            except ValueError:
+                # this device should not be at fstab
+                found = None
+            else:
+                found = self.find_entry(entry=entry)
+
+            # get correct spec type to use (if None, the one already present in entry is used)
+            spec = self._get_spec(action.device)
+
             if found is None and action.device.format.mountpoint is not None:
                 # device is not present in fstab and has a defined mountpoint => add it
-                self.add_entry(entry=entry)
+                self.add_entry(spec=spec,
+                               mntops=action.device.format.fstab.mntops,
+                               freq=action.device.format.fstab.freq,
+                               passno=action.device.format.fstab.passno,
+                               entry=entry)
+            elif found and found.spec != spec and action.device.format.mountpoint is not None:
+                # allow change of spec of existing devices
+                self.remove_entry(entry=found)
+                self.add_entry(spec=spec,
+                               mntops=action.device.format.fstab.mntops,
+                               freq=action.device.format.fstab.freq,
+                               passno=action.device.format.fstab.passno,
+                               entry=found)
             elif found and found.file != action.device.format.mountpoint and action.device.format.mountpoint is not None:
                 # device already exists in fstab but with a different mountpoint => add it
-                self.add_entry(file=action.device.format.mountpoint, entry=found)
+                self.add_entry(spec=spec,
+                               file=action.device.format.mountpoint,
+                               mntops=action.device.format.fstab.mntops,
+                               freq=action.device.format.fstab.freq,
+                               passno=action.device.format.fstab.passno,
+                               entry=found)
             return
 
         if action.is_configure and action.is_format and bae_entry is not None:
             # Handle change of the mountpoint:
             # Change its value if it is defined, remove the fstab entry if it is None
-            entry = self.entry_from_device(action.device)
+
+            # get correct spec type to use (if None, the one already present in entry is used)
+            spec = self._get_spec(action.device)
+
             if action.device.format.mountpoint is not None and bae_entry.file != action.device.format.mountpoint:
                 self.remove_entry(entry=bae_entry)
-                self.add_entry(file=action.device.format.mountpoint, entry=bae_entry)
+                self.add_entry(spec=spec,
+                               file=action.device.format.mountpoint,
+                               mntops=action.device.format.fstab.mntops,
+                               freq=action.device.format.fstab.freq,
+                               passno=action.device.format.fstab.passno,
+                               entry=bae_entry)
             elif action.device.format.mountpoint is None:
                 self.remove_entry(entry=bae_entry)

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

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()