Merge ~dbungert/curtin:fs_pass_one into curtin:master

[Dan Bungert]

Use /proc/filesystems to decide passno

The 'nodev' is intended to indicate
"whether the file system is mounted on a block device"
https://red.ht/3toGT5b

Use this info to set nodev items to passno 0, and default to 1 for
non-nodev or if the filesystem isn't present there.
Except that /proc/filesystems doesn't list 'swap' or 'none', so special
case those to passno 0.

LP: #1717584, LP: #1785354

passno 0 or 1 depending on the values found in /proc/filesystems.

[Dan Bungert (dbungert)]

One of the last changes broke unit tests, fixing.

[Ryan Harper (raharper)]

Thanks for submitting the MR.

<rant>
Personally I don't think this is the right choice to make by default. I don't think we install anything but journaled filesystems in which fsck will merely replay the journal just like the kernel already does. Look at fsck.xfs(8) manpage for a laugh.
</rant>

We should at least:

1) Add a curtin feature flag indicating curtin will now write fstabs which will enable fsck by default (curtin/__init__.py)
2) Add a config option to control the default fstab fsck value (block-meta: fstab_passno_default)
3) sync with MAAS/subiquity so they opt-in to fsck by default feature.
4) Consider not enabling the feature by default such that all MAAS/Subiquity users which move up to newer curtin are generating different installs than they did before the update.

I'll note that both MAAS and subiquity *can* fsck by default today if they construct storage config mount entries with the 'spec' field filled out they way they want.

review: Needs Fixing

[Michael Hudson-Doyle (mwhudson)]

In parallel with Ryan writing his comment we found that absolutely nothing in modern Ubuntu cares about passno==1 vs passno==2 so we should definitely drop that logic.

[Michael Hudson-Doyle (mwhudson)]

We discussed this internally a bit and still think we should change the default. (I haven't heard back from the MAAS team yet). The kernel is not the only thing that reads filesystems (UEFI firmware and grub both do), and for the filesystems that do not implement any kind of non-trivial fsck, we're not at the boot optimization point where adding an invocation of essentially /bin/true will have a noticeable impact. So the (small) upsides seem to outweigh the (even smaller) downsides and I don't think this makes enough difference to anyone to warrant a feature flag.

[Michael Hudson-Doyle (mwhudson)]

Also we have changed things so fsck is run in clouds recently after not doing so was causing problems (don't know the details) and we should be consistent about this.

[Ryan Harper (raharper)]

> We discussed this internally a bit and still think we should change the

I do wish this sort of discussion would happen either on discourse or
ubuntu-devel/ubuntu-server.

> default. (I haven't heard back from the MAAS team yet). The kernel is not the
> only thing that reads filesystems (UEFI firmware and grub both do), and for

VFAT is not a journaled filesystem, I'm 100% fine with enabling fsck on this.

As for grub the design of journaled filesystems are such that it's always
readable even on crash. Worst case IIUC is a newer files could not show up
until the journal replays.

Additionally, this journal replay (and fsck) happens after grub has run; any
new writes to the filesystem won't have been checked with fsck unless fsck
happens on shutdown (I don't think this is enabled anywhere in Ubuntu) and
still won't help in the case of a crash.

> the filesystems that do not implement any kind of non-trivial fsck, we're not
> at the boot optimization point where adding an invocation of essentially

This isn't a boot optimization; rather it can only slow boot down and for zero
benefit for journaled filesystems.

> /bin/true will have a noticeable impact. So the (small) upsides seem to
> outweigh the (even smaller) downsides and I don't think this makes enough
> difference to anyone to warrant a feature flag.

Changing behavior always warrants feature flags; they're free just like
version bumps. Let's not hide this "feature".

>
> Also we have changed things so fsck is run in clouds recently after not doing
> so was causing problems (don't know the details) and we should be consistent
> about this.

I've been opposed to this as well. For all of the same reasons here and a few
more (cloud instances not being pets, not having interactive consoles to do
repairs, fsck having a non-zero effect on time it takes to mount the rootfs)

[Dan Bungert (dbungert)]

Thank you for the feedback Ryan.
I've added the FSTAB item to the features list.
The proposal has been redone to not try to be clever about 1 vs 2.
We're planning to take the larger policy discussion to Discourse.
For now, the plan is to address the immediate feedback on the bug reports.
This MP does have an advantage of bringing the various installers into more closely matching behavior.

[Michael Hudson-Doyle (mwhudson)]

I started a discourse post for having a deeper conversation about this: https://discourse.ubuntu.com/t/to-fsck-or-not-to-fsck-at-boot-by-default/21478

[Server Team CI bot (server-team-bot)]

FAILED: Continuous integration, rev:4430c43a185818723b19e2de2f96a735cd5b08aa
https://jenkins.ubuntu.com/server/job/curtin-ci/100/
Executed test runs:
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/100/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/100/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/100/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/100/

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/100//rebuild

review: Needs Fixing (continuous-integration)

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:661d2c9472a727ae564a31c04c419c8ae1054b55
https://jenkins.ubuntu.com/server/job/curtin-ci/102/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/102/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/102/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/102/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/102/

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/102//rebuild

review: Approve (continuous-integration)

[Ryan Harper (raharper)]

Back when I looked at changing this, I also included this in vmtest:

```
diff --git a/tests/vmtests/__init__.py b/tests/vmtests/__init__.py
index 0b19d8f8..21a41aaa 100644
--- a/tests/vmtests/__init__.py
+++ b/tests/vmtests/__init__.py
@@ -556,6 +556,8 @@ DEFAULT_COLLECT_SCRIPTS = {
         [ ! -d /etc/default/grub.d ] ||
             cp -a /etc/default/grub.d etc_default_grub_d
         [ ! -f /etc/default/grub ] || cp /etc/default/grub etc_default_grub
+ command -v journalctl && \
+ journalctl -b -o json -u systemd-fsck* > systemd-fsck.json

         exit 0
         """)],
```

I ran all of vmtest and then sampled this output comparing with fsck enabled by default and without. I'll re-run this test locally but I'd like to have another system or two collect as well.

ISTR that enabling fsck on / by default did have a measurable effect to boot time. I'll update the discussion on discord.

[Michael Hudson-Doyle (mwhudson)]

I know it's a bit of a pain but I think I would really prefer to avoid emitting passno=1 for tmpfses. One approach would be to have passno == 0 for fstypes marked as "nodev" in /proc/filesystems but maybe that's just silly and you can special case "tmpfs" and "ramfs" (which are the filesystems MAAS supports today that should have passno == 0 by my reading).

review: Needs Fixing

[Dan Bungert (dbungert)]

@Michael - I believe I've done what has been requested.
@Ryan - I'll start some local data collection now.

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:b24d7c09beeba0af3e00f9f8947c1f68183e23a2
https://jenkins.ubuntu.com/server/job/curtin-ci/106/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/106/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/106/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/106/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/106/

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/106//rebuild

review: Approve (continuous-integration)

[Ryan Harper (raharper)]

I've been using this:

https://paste.ubuntu.com/p/x2TQ7d2j9v/

which does a better job of collecting boot time (captures systemd-analyze at shutdown time instead of during boot).

I run TestSimple which uses block-meta simple, it defaults to one root partition the size of the entire disk; this allows us to scale the size from (10 (default) to whatever) since fsck is somewhat related to total size of the filesystem it creates.

This loop is what I've used to collect data:

# 1TB, 100GB, 10GB rootfs sizes
for size in 1000 100 10; do
    # 50 tries
    for t in $(seq -w 1 50); do
        # this is where the disks and artifacts will be kept; the IO to/from
        # the virtual disks will happen against this mountpoint
        CURTIN_VMTEST_TOPDIR=/hogshead/blackmetal/curtin/baseline_${size}/${t};
        CURTIN_VMTEST_ROOT_DISK_SIZE_GB=${size}
        # run in parallel for some variance in IO patterns
        # the filter will select just the <Release>TestSimple test cases
        CURTIN_VMTEST_TOPDIR=$CURTIN_VMTEST_TOPDIR \
            ./tools/jenkins-runner -p12 --filter=conf_file=examples/tests/simple.yaml;
    done
done

grep . -r baseline_10/01/XenialTestSimple/collect/systemd-analyze.out
Startup finished in 3.207s (kernel) + 4.459s (userspace) = 7.666s

For now, I've been looking just at total time, and rootfs fsck only, for the
simple test we only have one filesystem; but in other scenarios where we fsck
each fs, that will increase the total boot time.

I've done testing against a high speed (xfs over NVME in raid0), and the
slowest I have access to is zfs over 5-disk raidz2.

I'm still processing the data but should have some results later this week.

[Michael Hudson-Doyle (mwhudson)]

LGTM.

review: Approve

[Server Team CI bot (server-team-bot)]

review: Approve (continuous-integration)

[Server Team CI bot (server-team-bot)]

Commit message lints:
- Line #4 has 39 too many characters. Line starts with: "https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/4/html/reference_guide/s2-proc-filesystem"...

review: Needs Fixing

[Server Team CI bot (server-team-bot)]

review: Approve (continuous-integration)

diff --git a/curtin/__init__.py b/curtin/__init__.py
index 0178a7b..aed0def 100644
--- a/curtin/__init__.py
+++ b/curtin/__init__.py
@@ -36,6 +36,8 @@ FEATURES = [
     'HAS_VERSION_MODULE',
     # uefi_reoder has fallback support if BootCurrent is missing
     'UEFI_REORDER_FALLBACK_SUPPORT',
+    # fstabs by default are output with passno = 1 if not nodev
+    'FSTAB_DEFAULT_FSCK_ON_BLK'
 ]
 
 __version__ = "21.2"
diff --git a/curtin/block/schemas.py b/curtin/block/schemas.py
index 4dc2f0a..7e8e324 100644
--- a/curtin/block/schemas.py
+++ b/curtin/block/schemas.py
@@ -262,6 +262,10 @@ MOUNT = {
             ],
         },
         'spec': {'type': 'string'},  # XXX: Tighten this to fstab fs_spec
+        'freq': {'type': ['integer', 'string'],
+                 'pattern': r'[0-9]'},
+        'passno': {'type': ['integer', 'string'],
+                   'pattern': r'[0-9]'},
     },
 }
 PARTITION = {
diff --git a/curtin/commands/block_meta.py b/curtin/commands/block_meta.py
index cf6bc02..b038dd9 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'):
@@ -1114,6 +1114,28 @@ def get_volume_spec(device_path):
     return devlinks[0] if len(devlinks) else device_path
 
 
+def proc_filesystems_passno(fstype):
+    """Examine /proc/filesystems - is this fstype listed and marked nodev?
+
+    :param fstype: a filesystem name such as ext2 or tmpfs
+    :return passno for fstype - nodev fs get 0, else 1"""
+
+    if fstype in ('swap', 'none'):
+        return "0"
+    with open('/proc/filesystems', 'r') as procfs:
+        for line in procfs.readlines():
+            tokens = line.strip('\n').split('\t')
+            if len(tokens) < 2:
+                continue
+
+            devstatus, curfs = tokens[:2]
+            if curfs != fstype:
+                continue
+
+            return "0" if devstatus == 'nodev' else "1"
+    return "1"
+
+
 def fstab_line_for_data(fdata):
     """Return a string representing fdata in /etc/fstab format.
 
@@ -1151,8 +1173,12 @@ def fstab_line_for_data(fdata):
     else:
         comment = None
 
+    passno = fdata.passno
+    if int(passno) < 0:
+        passno = proc_filesystems_passno(fdata.fstype)
+
     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..b20d5c1 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,14 @@ 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 default to 1 or 0, depending on if that
+filesystem is considered to be a 'nodev' device per /proc/filesystems.
+Note that per systemd-fstab-generator(8), systemd interprets passno as a
+boolean.
+
 **spec**: *<fs_spec>*
 
 The ``spec`` attribute defines the fsspec as defined in fstab(5).
diff --git a/examples/tests/filesystem_battery.yaml b/examples/tests/filesystem_battery.yaml
index 8166360..34657d4 100644
--- a/examples/tests/filesystem_battery.yaml
+++ b/examples/tests/filesystem_battery.yaml
@@ -106,20 +106,24 @@ storage:
       path: "/my/tmpfs"
       options: size=4194304
       fstype: "tmpfs"
+      passno: 1
     - id: ramfs1
       type: mount
       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..f1a658d 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 1\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 1\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 1\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,54 @@ 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", "1"],
+            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', passno='0',
+            options='noatime')
+        lines = block_meta.fstab_line_for_data(fdata).splitlines()
+        self.assertEqual(
+            ["/dev/disk2", "/", "btrfs", "noatime", "0", "0"],
+            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", "1"],
             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 +836,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", "1"],
             lines[1].split())
         self.assertEqual(1, m_uinfo.call_count)
         self.assertEqual(1, m_vol_type.call_count)
@@ -819,7 +858,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", "1"],
             lines[1].split())
         self.assertEqual(1, m_uinfo.call_count)
         self.assertEqual(1, m_vol_type.call_count)
@@ -837,7 +876,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", "1"],
             lines[1].split())
         self.assertEqual(0, m_get_uuid.call_count)
 
diff --git a/tests/unittests/test_feature.py b/tests/unittests/test_feature.py
index 8690ad8..0a7f890 100644
--- a/tests/unittests/test_feature.py
+++ b/tests/unittests/test_feature.py
@@ -30,4 +30,7 @@ class TestExportsFeatures(CiTestCase):
     def test_has_uefi_reorder_fallback_support(self):
         self.assertIn('UEFI_REORDER_FALLBACK_SUPPORT', curtin.FEATURES)
 
+    def test_has_fstab_default_fsck_on(self):
+        self.assertIn('FSTAB_DEFAULT_FSCK_ON_BLK', curtin.FEATURES)
+
 # vi: ts=4 expandtab syntax=python
diff --git a/tests/vmtests/test_fs_battery.py b/tests/vmtests/test_fs_battery.py
index 7d7b494..cc39f4a 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 1".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 0".split(),
+            "/etc /my/bind-ro-etc none bind,ro 1 0".split(),
         ]
         fstab_found = [
             line.split() for line in self.load_collect_file(