fix filesystem corruption on reboot/shutdown

[HiassofT]

On RPi4 I regularly saw that the journal on the /storage partition was corrupted and needed to be recovered on boot, which indicates that the partition wasn't cleanly unmounted before rebooting/shutting down.

[    4.487545] EXT4-fs (mmcblk0p2): recovery complete

Testing with a simple script executed right before reboot showed that both storage (and also flash if it was remounted rw before) wasn't remounted ro or unmounted before reboot - so the filesystem caches were dirty.

Test commit to add mount info before reboot: HiassofT@175c1df

console output - note rw on /storage

**** /usr/lib/systemd/system-shutdown/debug.sh called with args reboot                               
devtmpfs on /dev type devtmpfs (rw,relatime,size=1735392k,nr_inodes=97649,mode=755)                                                   
proc on /proc type proc (rw,relatime)                                                                                                 
sysfs on /sys type sysfs (rw,relatime)                                                                                                
/dev/mmcblk0p1 on /flash type vfat (ro,noatime,fmask=0022,dmask=0022,codepage=437,iocharset=ascii,shortname=mixed,errors=remount-ro)  
/dev/mmcblk0p2 on /storage type ext4 (rw,noatime)                                                                                     
/dev/loop0 on / type squashfs (ro,relatime)                                                                                           
tmpfs on /dev/shm type tmpfs (rw,nosuid,nodev)                                                                                        
devpts on /dev/pts type devpts (rw,nosuid,noexec,relatime,gid=5,mode=620,ptmxmode=000)                                                
tmpfs on /run type tmpfs (rw,nosuid,nodev,mode=755)                                                                                   
tmpfs on /sys/fs/cgroup type tmpfs (ro,nosuid,nodev,noexec,mode=755)                                                                  
cgroup2 on /sys/fs/cgroup/unified type cgroup2 (rw,nosuid,nodev,noexec,relatime,nsdelegate)                                           
cgroup on /sys/fs/cgroup/systemd type cgroup (rw,nosuid,nodev,noexec,relatime,xattr,name=systemd)                                     
bpf on /sys/fs/bpf type bpf (rw,nosuid,nodev,noexec,relatime,mode=700)                                                                
cgroup on /sys/fs/cgroup/cpu,cpuacct type cgroup (rw,nosuid,nodev,noexec,relatime,cpu,cpuacct)                                        
cgroup on /sys/fs/cgroup/cpuset type cgroup (rw,nosuid,nodev,noexec,relatime,cpuset)                                                  
cgroup on /sys/fs/cgroup/blkio type cgroup (rw,nosuid,nodev,noexec,relatime,blkio)                                                    
cgroup on /sys/fs/cgroup/devices type cgroup (rw,nosuid,nodev,noexec,relatime,devices)                                                
cgroup on /sys/fs/cgroup/freezer type cgroup (rw,nosuid,nodev,noexec,relatime,freezer)                                                
mqueue on /dev/mqueue type mqueue (rw,nosuid,nodev,noexec,relatime)                                                                   
debugfs on /sys/kernel/debug type debugfs (rw,nosuid,nodev,noexec,relatime)                                                           
configfs on /sys/kernel/config type configfs (rw,nosuid,nodev,noexec,relatime)                                                        
[   71.945431] reboot: Restarting system                                          

This is caused by the bad systemd-0005-ignore-storage-flash-mount-points.patch patch which prevents systemd-shutdown to remount ro and then unmount those two filesystems.

Drop this patch, it's highly probable that it has also been causing the unexplainable kodi database or xml file corruptions that we have been seeing in the last couple of years.

With this PR storage will be properly unmounted and flash remounted as ro before reboot (tested on 4GB RPi4 with noram on cmdline.txt to simulate low-ram devices)

**** /usr/lib/systemd/system-shutdown/debug.sh called with args reboot
devtmpfs on /dev type devtmpfs (rw,relatime,size=1735392k,nr_inodes=97649,mode=755)
proc on /proc type proc (rw,relatime)
sysfs on /sys type sysfs (rw,relatime)
/dev/mmcblk0p1 on /flash type vfat (ro,noatime,fmask=0022,dmask=0022,codepage=437,iocharset=ascii,shortname=mixed,errors=remount-ro)
/dev/loop0 on / type squashfs (ro,relatime)
tmpfs on /dev/shm type tmpfs (rw,nosuid,nodev)
devpts on /dev/pts type devpts (rw,nosuid,noexec,relatime,gid=5,mode=620,ptmxmode=000)
tmpfs on /run type tmpfs (rw,nosuid,nodev,mode=755)
tmpfs on /sys/fs/cgroup type tmpfs (ro,nosuid,nodev,noexec,mode=755)
cgroup2 on /sys/fs/cgroup/unified type cgroup2 (rw,nosuid,nodev,noexec,relatime,nsdelegate)
cgroup on /sys/fs/cgroup/systemd type cgroup (rw,nosuid,nodev,noexec,relatime,xattr,name=systemd)
bpf on /sys/fs/bpf type bpf (rw,nosuid,nodev,noexec,relatime,mode=700)
cgroup on /sys/fs/cgroup/cpuset type cgroup (rw,nosuid,nodev,noexec,relatime,cpuset)
cgroup on /sys/fs/cgroup/freezer type cgroup (rw,nosuid,nodev,noexec,relatime,freezer)
cgroup on /sys/fs/cgroup/blkio type cgroup (rw,nosuid,nodev,noexec,relatime,blkio)
cgroup on /sys/fs/cgroup/cpu,cpuacct type cgroup (rw,nosuid,nodev,noexec,relatime,cpu,cpuacct)
cgroup on /sys/fs/cgroup/devices type cgroup (rw,nosuid,nodev,noexec,relatime,devices)
mqueue on /dev/mqueue type mqueue (rw,nosuid,nodev,noexec,relatime)
debugfs on /sys/kernel/debug type debugfs (rw,nosuid,nodev,noexec,relatime)
configfs on /sys/kernel/config type configfs (rw,nosuid,nodev,noexec,relatime)
[   31.478689] reboot: Restarting system

or also flash unmounted if SYSTEM was copied to ram (tested on 4GB RPi4 without noram)

*** /usr/lib/systemd/system-shutdown/debug.sh called with args reboot
devtmpfs on /dev type devtmpfs (rw,relatime,size=1735392k,nr_inodes=97649,mode=755)
proc on /proc type proc (rw,relatime)
sysfs on /sys type sysfs (rw,relatime)
/dev/loop0 on / type squashfs (ro,relatime)
tmpfs on /dev/shm type tmpfs (rw,nosuid,nodev)
devpts on /dev/pts type devpts (rw,nosuid,noexec,relatime,gid=5,mode=620,ptmxmode=000)
tmpfs on /run type tmpfs (rw,nosuid,nodev,mode=755)
tmpfs on /sys/fs/cgroup type tmpfs (ro,nosuid,nodev,noexec,mode=755)
cgroup2 on /sys/fs/cgroup/unified type cgroup2 (rw,nosuid,nodev,noexec,relatime,nsdelegate)
cgroup on /sys/fs/cgroup/systemd type cgroup (rw,nosuid,nodev,noexec,relatime,xattr,name=systemd)
bpf on /sys/fs/bpf type bpf (rw,nosuid,nodev,noexec,relatime,mode=700)
cgroup on /sys/fs/cgroup/devices type cgroup (rw,nosuid,nodev,noexec,relatime,devices)
cgroup on /sys/fs/cgroup/cpu,cpuacct type cgroup (rw,nosuid,nodev,noexec,relatime,cpu,cpuacct)
cgroup on /sys/fs/cgroup/cpuset type cgroup (rw,nosuid,nodev,noexec,relatime,cpuset)
cgroup on /sys/fs/cgroup/freezer type cgroup (rw,nosuid,nodev,noexec,relatime,freezer)
cgroup on /sys/fs/cgroup/blkio type cgroup (rw,nosuid,nodev,noexec,relatime,blkio)
debugfs on /sys/kernel/debug type debugfs (rw,nosuid,nodev,noexec,relatime)
mqueue on /dev/mqueue type mqueue (rw,nosuid,nodev,noexec,relatime)
configfs on /sys/kernel/config type configfs (rw,nosuid,nodev,noexec,relatime)
[   27.909571] reboot: Restarting system

[MilhouseVH]

Thanks, I'll include this in nightly builds starting with #0909

[mglae]

Good catch.

But now /flash and /storage are tried to be unmounted before umount.target too and red error messages are printed on every shutdown.

When defining the mounts as "extrinsic" with this patch they are only re-/unmounted finally in systemd-shutdown after all processes have been stopped.

I've used your debug.sh shutdown script to verify the behavior.

[HiassofT]

@mglae I strongly suspect that if we need to patch systemd then we're doing something utterly wrong

Unmounting /storage via umount.target should be fine, all services accessing that should be stopped before (unless some unit dependencies are wrong, in which case these have to be fixed).

/flash is indeed problematic as it gets an automatic RequiresMountsFor=/ dependency - so we can't just add RequiresMountsFor=/flash to the root mount because that would create a cycle.

Creating a flash.mount drop-in that disables default dependencies (and thus removes the Conflicts=umount.target) should work though.

In local testing I saw /flash unmount failing (you may need "noram" in current master and 9.2 trees to prevent init copying SYSTEM to RAM).

Creating a /storage/.config/system.d/flash.mount.d/no-default-dependencie s.conf drop-in with the following contents seems to prevent that

[Unit]
DefaultDependencies=no

Note: I only did some very quick testing with that so far, didn't see any FAIL error during reboot and systemctl show /flash output looked fine.

[HiassofT]

I've added a drop-in for /flash to remove default dependencies - /flash now won't be unmounted via umount.target

I also noticed that scripts/install didn't install files in PKG_DIR/system.d recursively, added a fix for that so we can easily add drop-ins if needed

[MilhouseVH]

Tested the drop-in on RPi2 and Generic and no longer seeing a failure when unmounting /flash, so LGTM!

[mglae]

I strongly suspect that if we need to patch systemd then we're doing something utterly wrong

We are using an uncommon configuration extending the root FS with /flash and /storage. There may be modification needed to make this work.

But everything that can be configured does not need to be patched. ;-)

The drop-in solution is working fine with systemd 242.

The drop-in for /storage is needed too. When booting with debugging parameter a persistent journal is created on /storage. The systemd-journal process is only stopped and journal flushed in systemd-shutdown. Therefore remounting ro in umount.target is too early even we ignore the error message.

[HiassofT]

The unmount error because of journald has been a long outstanding systemd issue systemd/systemd#867 which has finally beein fixed in systemd 243 via this PR systemd/systemd#12230

systemd-journal-flush.service now calls journalctl --smart-relinquish-var to revert to /run logging during shutdown and close the open journal files on /var.

However, there's another issue in LE: as systemd-journal-flush.service only has a dependency on /var (via RequiresMountsFor=/var/log/journal) and the LE var-log-debug.service symlinks /var/log to /storage/log the dependency on storage.mount is missing

A quick test with a drop-in for systemd-journal-flush.service (adding /storage to RequiresMountsFor) somewhat worked - that added storage.mount to After=, but not Requires=
.config/system.d/systemd-journal-flush.service.d/dependencies.conf:

[Unit]
RequiresMountsFor=/var/log/journal /storage

Though the dependencies now looked correct (checking with systemctl show systemd-journal-flush | egrep 'Requires|After=|Before=' I sometimes got an error unmounting storage during reboot.

Testing with a simple umount wrapper HiassofT@f1a1eb1 I saw that journald had not closed it's files on /storage by the time umount was called

**** /usr/bin/umount-wrapper called with args /storage -c                                                                             
250     /usr/lib/systemd/systemd-journald       /storage/log/journal/0bbf7f2eb51bc21b7f6eeb795caf6b34/system@a946d506b60a4af68b742739d
5bc219c-000000000000116a-000592961e23a569.journal                                                                                     
250     /usr/lib/systemd/systemd-journald       /storage/log/journal/0bbf7f2eb51bc21b7f6eeb795caf6b34/system.journal                  
[   21.997405] systemd[1]: Failed unmounting /storage.                                                                                

Even more puzzling, when I added storage.mount to Requires= of the drop in

[Unit]
Requires=systemd-journald.service storage.mount
RequiresMountsFor=/var/log/journal /storage

which shouldn't change anything in terms of systemd unit ordering the failed message hasn't occurred yet.

The way LE handles persistent logging to /storage should probably be reworked as it's quite hacky. i.e. use tmpfiles.d to create /storage/log, maybe bind-mount /storage/log to /var/mount and/or setup proper dependencies - that'll be stuff for a separate PR though

[HiassofT]

PR with fix for storage unmount failed with debugging enabled is here: #3831