Fstab cleanup fix

Formats with multiple layers (e.g. LUKS) were not handled properly in
fstab. This resulted in fstab entries not being properly cleaned from
fstab when destroy actions were executed. This fixes the issue by using
different variables with correct values based on action type.

blivet/actionlist.py

@@ -292,7 +292,7 @@ def process(self, callbacks=None, devices=None, fstab=None, dry_run=None):
             # (device may not exist afterwards)
             if not skip_fstab:
                 try:
-                    entry = fstab.entry_from_device(action.device)
+                    entry = fstab.entry_from_action(action)
                 except ValueError:
                     # this device should not be in fstab
                     bae_entry = None

blivet/blivet.py

@@ -56,7 +56,10 @@
 log = logging.getLogger("blivet")
 
 
-FSTAB_PATH = "/etc/fstab"
+# Default path to fstab file. Left empty to prevent blivet from using
+# fstab functionality by default.
+# TODO Change to "/etc/fstab" at next major version
+FSTAB_PATH = ""
 
 
 @six.add_metaclass(SynchronizedMeta)

blivet/fstab.py

@@ -391,11 +391,48 @@ def entry_from_device(self, device):
         entry.spec = self._get_spec(device)
         if entry.spec is None:
             entry.spec = getattr(device, "fstab_spec", None)
-
         entry.vfstype = device.format.type
 
         return entry
 
+    def entry_from_action(self, action):
+        """ Generate FSTabEntry object based on given blivet Action
+
+            :keyword action: action to process
+            :type action: :class: `blivet.deviceaction.DeviceAction`
+            :returns: fstab entry object based on device processed by action
+            :rtype: :class: `FSTabEntry`
+        """
+
+        if not action.is_format:
+            raise ValueError("""cannot generate fstab entry from action '%s' because
+                                its type is not 'format'""" % action)
+
+        fmt = action.format
+        if action.is_destroy:
+            fmt = action.orig_format
+        if action.is_create:
+            fmt = action.device.format
+
+        entry = FSTabEntry()
+
+        entry.file = None
+        if fmt.mountable:
+            entry.file = fmt.mountpoint
+        elif fmt.type == "swap":
+            entry.file = "swap"
+        else:
+            raise ValueError("""cannot generate fstab entry from action '%s' because
+                                it is neither mountable nor swap type""" % action)
+
+        entry.spec = self._get_spec(action.device)
+        if entry.spec is None:
+            entry.spec = getattr(action.device, "fstab_spec", None)
+
+        entry.vfstype = action.device.format.type
+
+        return entry
+
     def read(self):
         """ Read the fstab file from path stored in self.src_file. Resets currently loaded table contents.
         """
@@ -546,7 +583,7 @@ def add_entry(self, spec=None, file=None, vfstype=None, mntops=None,
 
         if mntops is not None:
             _entry.mntops = mntops
-        elif _entry.mntops is None:
+        if _entry.mntops is None:
             _entry.mntops = ['defaults']
 
         if freq is not None:
@@ -696,7 +733,11 @@ def _get_spec(self, device):
         # Use "globally" set (on FSTabManager level) spec type otherwise
 
         spec = None
-        spec_type = device.format.fstab.spec_type or self.spec_type
+
+        if hasattr(device.format, 'fstab') and device.format.fstab.spec_type:
+            spec_type = device.format.fstab.spec_type
+        else:
+            spec_type = self.spec_type
 
         if spec_type == "LABEL" and device.format.label:
             spec = "LABEL=%s" % device.format.label
@@ -737,14 +778,15 @@ def update(self, action, bae_entry):
             # does not have UUID assigned yet, so we skip that one
             return
 
-        if action.is_create and action.device.format.mountable:
+        if action.is_create and action.is_format and action.device.format.mountable:
             # add the device to the fstab
             # make sure it is not already present there
             try:
                 entry = self.entry_from_device(action.device)
             except ValueError:
                 # this device should not be at fstab
                 found = None
+                entry = None
             else:
                 found = self.find_entry(entry=entry)
 
@@ -754,6 +796,7 @@ def update(self, action, bae_entry):
             if found is None and action.device.format.mountpoint is not None:
                 # device is not present in fstab and has a defined mountpoint => add it
                 self.add_entry(spec=spec,
+                               file=action.device.format.mountpoint,
                                mntops=action.device.format.fstab.mntops,
                                freq=action.device.format.fstab.freq,
                                passno=action.device.format.fstab.passno,

tests/storage_tests/fstab_test.py

@@ -1,10 +1,9 @@
 import os
-
-from .storagetestcase import StorageTestCase
-
 import blivet
 import tempfile
 
+from .storagetestcase import StorageTestCase
+
 
 class FstabTestCase(StorageTestCase):
 
@@ -97,6 +96,50 @@ def test_fstab(self):
                 self.assertFalse("blivetTestLVMine" in contents)
                 self.assertFalse("/mnt/test2" in contents)
 
+    def test_luks_creation(self):
+        # test creation of a multiple layer device
+        disk = self.storage.devicetree.get_device_by_path(self.vdevs[0])
+        self.assertIsNotNone(disk)
+
+        fstab_path = '/tmp/myfstab'
+
+        with tempfile.TemporaryDirectory() as tmpdirname:
+            fstab_path = os.path.join(tmpdirname, 'fstab')
+
+            # change write path of blivet.fstab
+            self.storage.fstab.dest_file = fstab_path
+
+            self.storage.initialize_disk(disk)
+
+            var = self.storage.new_partition(fmt_type="luks",
+                                             fmt_args={"passphrase": "opensaysme"},
+                                             size=blivet.size.Size("200MiB"),
+                                             parents=[disk],
+                                             mountpoint="/mnt/test_fstab_luks_wrong")
+
+            self.storage.create_device(var)
+
+            varenc = blivet.devices.LUKSDevice(name="blivetTest_fstab_luks",
+                                               size=var.size,
+                                               parents=var)
+            self.storage.create_device(varenc)
+
+            varfs = blivet.formats.get_format(fmt_type="ext4",
+                                              device=varenc.path,
+                                              mountpoint="/mnt/test_fstab_luks_correct")
+            self.storage.format_device(varenc, varfs)
+
+            blivet.partitioning.do_partitioning(self.storage)
+
+            self.storage.do_it()
+            self.storage.reset()
+
+            # Check fstab contents for added device
+            with open(fstab_path, "r") as f:
+                contents = f.read()
+                self.assertTrue("/mnt/test_fstab_luks_correct" in contents)
+                self.assertFalse("/mnt/test_fstab_luks_wrong" in contents)
+
     def test_swap_creation(self):
         # test swap creation for presence of FSTabOptions object
         disk = self.storage.devicetree.get_device_by_path(self.vdevs[0])