curtin-dev team mailing list archive

[Dan Bungert <daniel.bungert@xxxxxxxxxxxxx>]

Dan Bungert has proposed merging ~dbungert/curtin:nonzero-fs_pass into curtin:master.

Commit message:
Output a 1 or 2 passno for fstab lines if not overridden

When outputing the fstab line, examine the path and use it to determine to
output a passno of 1 or 2.  'none' paths get a 0.

Also document that passno/freq can be set in the yaml.

LP: #1717584, LP: #1785354

Requested reviews:
  Michael Hudson-Doyle (mwhudson)

For more details, see:
https://code.launchpad.net/~dbungert/curtin/+git/curtin/+merge/399817

Assign a default fstab passno of -1, and if that has not been overridden with a
real value, output 0/1/2 depending on the target path.
The decision was made to not pay attention to bind mounts and fsck them anyhow,
with the belief of use cases where this would not be redundant.
tmpfs and similar end up with a passno of 2, but this seems harmless.
-- 
Your team curtin developers is subscribed to branch curtin:master.

diff --git a/curtin/commands/block_meta.py b/curtin/commands/block_meta.py
index cf6bc02..9199afb 100644
--- a/curtin/commands/block_meta.py
+++ b/curtin/commands/block_meta.py
@@ -27,7 +27,7 @@ import time
 FstabData = namedtuple(
     "FstabData", ('spec', 'path', 'fstype', 'options', 'freq', 'passno',
                   'device'))
-FstabData.__new__.__defaults__ = (None, None, None, "", "0", "0", None)
+FstabData.__new__.__defaults__ = (None, None, None, "", "0", "-1", None)
 
 
 SIMPLE = 'simple'
@@ -1020,7 +1020,7 @@ def mount_data(info, storage_config):
     fstype = info.get('fstype')
     path = info.get('path')
     freq = str(info.get('freq', 0))
-    passno = str(info.get('passno', 0))
+    passno = str(info.get('passno', -1))
 
     # turn empty options into "defaults", which works in fstab and mount -o.
     if not info.get('options'):
@@ -1151,8 +1151,18 @@ def fstab_line_for_data(fdata):
     else:
         comment = None
 
+    passno = fdata.passno
+    if passno == "-1":
+        # / and /boot things get 1
+        if path == "/" or path[1:].split('/')[0] in ('', 'boot'):
+            passno = "1"
+        elif path != "none":
+            passno = "2"
+        else:
+            passno = "0"
+
     entry = ' '.join((spec, path, fdata.fstype, options,
-                      fdata.freq, fdata.passno)) + "\n"
+                      fdata.freq, passno)) + "\n"
     line = '\n'.join([comment, entry] if comment else [entry])
     return line
 
diff --git a/doc/topics/storage.rst b/doc/topics/storage.rst
index 75c5537..58e93f7 100644
--- a/doc/topics/storage.rst
+++ b/doc/topics/storage.rst
@@ -542,6 +542,11 @@ One of ``device`` or ``spec`` must be present.
   fstab entry will contain ``_netdev`` to indicate networking is
   required to mount this filesystem.
 
+**freq**: *<dump(8) integer from 0-9 inclusive>*
+
+The ``freq`` key refers to the freq as defined in dump(8).
+Defaults to ``0`` if unspecified.
+
 **fstype**: *<fileystem type>*
 
 ``fstype`` is only required if ``device`` is not present.  It indicates
@@ -552,7 +557,7 @@ to ``/etc/fstab``
 
 The ``options`` key will replace the default options value of ``defaults``.
 
-.. warning:: 
+.. warning::
   The kernel and user-space utilities may differ between the install
   environment and the runtime environment.  Not all kernels and user-space
   combinations will support all options.  Providing options for a mount point
@@ -565,6 +570,11 @@ The ``options`` key will replace the default options value of ``defaults``.
   If either of the environments (install or target) do not have support for
   the provided options, the behavior is undefined.
 
+**passno**: *<fsck(8) non-negative integer, typically 0-2>*
+
+The ``passno`` key refers to the fs_passno as defined in fsck(8).
+If unspecified, ``curtin`` will choose a value based on the other properties.
+
 **spec**: *<fs_spec>*
 
 The ``spec`` attribute defines the fsspec as defined in fstab(5).
@@ -596,7 +606,7 @@ Below is an example of configuring a bind mount.
 
 That would result in a fstab entry like::
 
-  /my/bind-over-var-lib /var/lib none bind 0 0
+  /my/bind-over-var-lib /var/lib none bind 0 2
 
 **Tmpfs Mount**
 
@@ -613,7 +623,7 @@ Below is an example of configuring a tmpfsbind mount.
 
 That would result in a fstab entry like::
 
-  none /my/tmpfs tmpfs size=4194304 0 0
+  none /my/tmpfs tmpfs size=4194304 0 2
 
 
 Lvm Volgroup Command
diff --git a/examples/tests/filesystem_battery.yaml b/examples/tests/filesystem_battery.yaml
index 8166360..406a087 100644
--- a/examples/tests/filesystem_battery.yaml
+++ b/examples/tests/filesystem_battery.yaml
@@ -111,15 +111,18 @@ storage:
       spec: "none"
       path: "/my/ramfs"
       fstype: "ramfs"
+      passno: 0
     - id: bind1
       fstype: "none"
       options: "bind"
       path: "/var/cache"
       spec: "/my/bind-over-var-cache"
       type: mount
+      freq: 3
     - id: bind2
       fstype: "none"
       options: "bind,ro"
       path: "/my/bind-ro-etc"
       spec: "/etc"
       type: mount
+      freq: 1
diff --git a/tests/unittests/test_commands_block_meta.py b/tests/unittests/test_commands_block_meta.py
index 8cfd6af..da8b2e2 100644
--- a/tests/unittests/test_commands_block_meta.py
+++ b/tests/unittests/test_commands_block_meta.py
@@ -361,7 +361,7 @@ class TestBlockMeta(CiTestCase):
         block_meta.mount_handler(mount_info, self.storage_config)
         options = 'defaults'
         comment = "# / was on /wark/xxx during curtin installation"
-        expected = "%s\n%s %s %s %s 0 0\n" % (comment,
+        expected = "%s\n%s %s %s %s 0 1\n" % (comment,
                                               disk_info['path'],
                                               mount_info['path'],
                                               fs_info['fstype'], options)
@@ -389,7 +389,7 @@ class TestBlockMeta(CiTestCase):
         block_meta.mount_handler(mount_info, self.storage_config)
         options = 'ro'
         comment = "# /readonly was on /wark/xxx during curtin installation"
-        expected = "%s\n%s %s %s %s 0 0\n" % (comment,
+        expected = "%s\n%s %s %s %s 0 2\n" % (comment,
                                               disk_info['path'],
                                               mount_info['path'],
                                               fs_info['fstype'], options)
@@ -418,7 +418,7 @@ class TestBlockMeta(CiTestCase):
         block_meta.mount_handler(mount_info, self.storage_config)
         options = 'defaults'
         comment = "# /readonly was on /wark/xxx during curtin installation"
-        expected = "%s\n%s %s %s %s 0 0\n" % (comment,
+        expected = "%s\n%s %s %s %s 0 2\n" % (comment,
                                               disk_info['path'],
                                               mount_info['path'],
                                               fs_info['fstype'], options)
@@ -449,7 +449,7 @@ class TestBlockMeta(CiTestCase):
         block_meta.mount_handler(mount_info, self.storage_config)
         options = 'defaults'
         comment = "# /readonly was on /wark/xxx during curtin installation"
-        expected = "#curtin-test\n%s\n%s %s %s %s 0 0\n" % (comment,
+        expected = "#curtin-test\n%s\n%s %s %s %s 0 2\n" % (comment,
                                                             disk_info['path'],
                                                             mount_info['path'],
                                                             fs_info['fstype'],
@@ -610,7 +610,7 @@ class TestFstabData(CiTestCase):
         self.assertEqual(
             block_meta.FstabData(
                 spec="none", fstype="tmpfs", path="/tmpfs",
-                options="defaults", freq="0", passno="0", device=None),
+                options="defaults", freq="0", device=None),
             block_meta.mount_data(info, {'xm1': info}))
 
     @patch('curtin.block.iscsi.volpath_is_iscsi')
@@ -625,7 +625,7 @@ class TestFstabData(CiTestCase):
         self.assertEqual(
             block_meta.FstabData(
                 spec=None, fstype="ext4", path="/",
-                options="noatime", freq="0", passno="0", device="/dev/xda1"),
+                options="noatime", freq="0", device="/dev/xda1"),
             block_meta.mount_data(scfg['m1'], scfg))
 
     @patch('curtin.block.iscsi.volpath_is_iscsi', return_value=False)
@@ -643,7 +643,7 @@ class TestFstabData(CiTestCase):
         self.assertEqual(
             block_meta.FstabData(
                 spec=None, fstype="vfat", path="/boot/efi",
-                options="defaults", freq="0", passno="0", device="/dev/xda1"),
+                options="defaults", freq="0", device="/dev/xda1"),
             block_meta.mount_data(scfg['m1'], scfg))
 
     @patch('curtin.block.iscsi.volpath_is_iscsi')
@@ -657,7 +657,7 @@ class TestFstabData(CiTestCase):
         self.assertEqual(
             block_meta.FstabData(
                 spec=None, fstype="ext4", path="/",
-                options="noatime,_netdev", freq="0", passno="0",
+                options="noatime,_netdev", freq="0",
                 device="/dev/xda1"),
             block_meta.mount_data(scfg['m1'], scfg))
 
@@ -683,7 +683,7 @@ class TestFstabData(CiTestCase):
         self.assertEqual(
             block_meta.FstabData(
                 spec=myspec, fstype="ext3", path="/",
-                options="noatime", freq="0", passno="0",
+                options="noatime", freq="0",
                 device=None),
             block_meta.mount_data(mnt, scfg))
 
@@ -761,15 +761,53 @@ class TestFstabData(CiTestCase):
             spec="/dev/disk2", path="/mnt", fstype='btrfs', options='noatime')
         lines = block_meta.fstab_line_for_data(fdata).splitlines()
         self.assertEqual(
-            ["/dev/disk2", "/mnt", "btrfs", "noatime", "0", "0"],
+            ["/dev/disk2", "/mnt", "btrfs", "noatime", "0", "2"],
+            lines[1].split())
+
+    def test_fstab_line_root_and_no_passno(self):
+        """fstab_line_for_data passno autoselect for /."""
+        fdata = block_meta.FstabData(
+            spec="/dev/disk2", path="/", fstype='btrfs', options='noatime')
+        lines = block_meta.fstab_line_for_data(fdata).splitlines()
+        self.assertEqual(
+            ["/dev/disk2", "/", "btrfs", "noatime", "0", "1"],
+            lines[1].split())
+
+    def test_fstab_line_boot_and_no_passno(self):
+        """fstab_line_for_data passno autoselect for /boot."""
+        fdata = block_meta.FstabData(
+            spec="/dev/disk2", path="/boot", fstype='btrfs', options='noatime')
+        lines = block_meta.fstab_line_for_data(fdata).splitlines()
+        self.assertEqual(
+            ["/dev/disk2", "/boot", "btrfs", "noatime", "0", "1"],
+            lines[1].split())
+
+    def test_fstab_line_boot_efi_and_no_passno(self):
+        """fstab_line_for_data passno autoselect for /boot/efi."""
+        fdata = block_meta.FstabData(
+            spec="/dev/disk2", path="/boot/efi", fstype='btrfs',
+            options='noatime')
+        lines = block_meta.fstab_line_for_data(fdata).splitlines()
+        self.assertEqual(
+            ["/dev/disk2", "/boot/efi", "btrfs", "noatime", "0", "1"],
+            lines[1].split())
+
+    def test_fstab_line_almost_boot(self):
+        """fstab_line_for_data passno that pretends to be /boot."""
+        fdata = block_meta.FstabData(
+            spec="/dev/disk2", path="/boots", fstype='btrfs',
+            options='noatime')
+        lines = block_meta.fstab_line_for_data(fdata).splitlines()
+        self.assertEqual(
+            ["/dev/disk2", "/boots", "btrfs", "noatime", "0", "2"],
             lines[1].split())
 
     def test_fstab_line_for_data_with_passno_and_freq(self):
         """fstab_line_for_data should respect passno and freq."""
         fdata = block_meta.FstabData(
-            spec="/dev/d1", path="/mnt", fstype='ext4', freq="1", passno="2")
+            spec="/dev/d1", path="/mnt", fstype='ext4', freq="1", passno="1")
         lines = block_meta.fstab_line_for_data(fdata).splitlines()
-        self.assertEqual(["1", "2"], lines[1].split()[4:6])
+        self.assertEqual(["1", "1"], lines[1].split()[4:6])
 
     def test_fstab_line_for_data_raises_error_without_spec_or_device(self):
         """fstab_line_for_data should raise ValueError if no spec or device."""
@@ -797,7 +835,7 @@ class TestFstabData(CiTestCase):
             "# /mnt was on /dev/disk2 during curtin installation",
             lines[0])
         self.assertEqual(
-            [by_uuid, "/mnt", "ext4", "defaults", "0", "0"],
+            [by_uuid, "/mnt", "ext4", "defaults", "0", "2"],
             lines[1].split())
         self.assertEqual(1, m_uinfo.call_count)
         self.assertEqual(1, m_vol_type.call_count)
@@ -819,7 +857,7 @@ class TestFstabData(CiTestCase):
             "# /mnt was on /dev/disk2 during curtin installation",
             lines[0])
         self.assertEqual(
-            ["/dev/disk2", "/mnt", "ext4", "defaults", "0", "0"],
+            ["/dev/disk2", "/mnt", "ext4", "defaults", "0", "2"],
             lines[1].split())
         self.assertEqual(1, m_uinfo.call_count)
         self.assertEqual(1, m_vol_type.call_count)
@@ -837,7 +875,7 @@ class TestFstabData(CiTestCase):
             '# /mnt was on /dev/xvda1 during curtin installation',
             lines[0])
         self.assertEqual(
-            ["/dev/xvda1", "/mnt", "ext4", "defaults", "0", "0"],
+            ["/dev/xvda1", "/mnt", "ext4", "defaults", "0", "2"],
             lines[1].split())
         self.assertEqual(0, m_get_uuid.call_count)
 
diff --git a/tests/vmtests/test_fs_battery.py b/tests/vmtests/test_fs_battery.py
index 7d7b494..e5903f1 100644
--- a/tests/vmtests/test_fs_battery.py
+++ b/tests/vmtests/test_fs_battery.py
@@ -159,10 +159,10 @@ class TestFsBattery(VMBaseClass):
     def test_fstab_has_mounts(self):
         """Verify each of the expected "my" mounts got into fstab."""
         expected = [
-            "none /my/tmpfs tmpfs size=4194304 0 0".split(),
+            "none /my/tmpfs tmpfs size=4194304 0 2".split(),
             "none /my/ramfs ramfs defaults 0 0".split(),
-            "/my/bind-over-var-cache /var/cache none bind 0 0".split(),
-            "/etc /my/bind-ro-etc none bind,ro 0 0".split(),
+            "/my/bind-over-var-cache /var/cache none bind 3 2".split(),
+            "/etc /my/bind-ro-etc none bind,ro 1 2".split(),
         ]
         fstab_found = [
             line.split() for line in self.load_collect_file(