Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

init: Silence exFAT errors when mounting iso9660; reorder exfat last #1602

Conversation

JonathonHall-Purism
Copy link
Collaborator

@JonathonHall-Purism JonathonHall-Purism commented Feb 2, 2024

Since exFAT support was enabled, spurious exFAT errors appear in the console in some cases:

  • mounting iso9660 hybrid USB
  • searching for /boot when a bios-grub, LVM, or LUKS partition precedes boot

In both cases, avoid trying to mount things as exFAT that aren't exFAT. exfat is moved below iso9660 in automatic mount detection by creating /etc/filesystems (using /proc/filesystems). Detecting /boot skips some things it knows are not mountable as /boot (bios-grub, LUKS, LVM PV), and also avoids trying the same device twice. Details in commits.

Tested with mount-usb:

  • mount iso9660 hybrid image on USB, verify no more exFAT errors
  • mount exFAT partition
  • format exFAT over iso9660 hybrid image with mkfs.exfat in Heads (without partition table), mount, verify new exFAT is found correctly (not remnant iso9660)
  • format exFAT over iso9660 as above, but with mkfs.exfat from Debian 12
  • mount ext4 partition

Tested with detect_boot_device:

  • GPT disk with bios-grub, ext4 /boot, and LUKS root - tested setting CONFIG_BOOT_DEV to each to verify bios-grub and LUKS do not emit exFAT errors
  • MBR disk with ext4 /boot and LVM root (Qubes default) - tested setting CONFIG_BOOT_DEV to each to verify the LUKS PV does not emit exFAT errors

Since exFAT support was enabled, mounting an iso9660 filesystem prints
spurious exFAT errors to the console.  That is because busybox mount
tries all filesystems in the order listed, and exfat precedes iso9660
(those are the last two in our config).  Most filesystems are silent
when used on the wrong type of filesystem, but exFAT logs errors, which
appear on the console.

Move exFAT after iso9660, so iso9660 filesystems won't show these
errors.  The errors will still appear if the filesystem is actually
exFAT but cannot be mounted.

There's no significant risk of misdetecting a remnant iso9660
superblock here either.  Although an iso9660 superblock could fall in
the unused space between the exFAT boot region and the FAT itself,
mkfs.exfat does zero this space so it is unlikely such a remnant
superblock would exist.

Signed-off-by: Jonathon Hall <[email protected]>
@JonathonHall-Purism
Copy link
Collaborator Author

Testing on more devices I found another situation where spurious exFAT errors appear, and this PR makes them worse 🤦 Setting to draft while I solve that.

On Librem 11, the default OS image has a GPT partition table with /dev/nvme0n1p1 as the bios-grub partition (holds stage 2), the actual /boot is /dev/nvme0n1p2. But CONFIG_BOOT_DEV is defaulted to /dev/nvme0n1p1 and it relies on autodetect to find out that it's actually nvme0n1p2.

detect_boot_device tries to mount /dev/nvme0n1p1 twice (once since it's CONFIG_BOOT_DEV, then again when trying all partitions in order). This used to generate two sets of exFAT errors since we can't mount that partition at all. Now it generates four, since each mount tries exFAT twice (once from /etc/filesystems, once from /proc/filesystems). There's no point trying both lists here, but busybox always uses both (GNU coreutils' mount only uses both lists if /etc/filesystems ends with a '*' line).

@JonathonHall-Purism JonathonHall-Purism marked this pull request as draft February 2, 2024 19:35
@tlaurion
Copy link
Collaborator

tlaurion commented Feb 2, 2024

I tested real quick under debian-12 on qemu and boot messages were still showing this for vda1 (normal since non-supported fs by Heads. Ugly but this is how it is).

Tried to rewrite to /proc/filesystem but changes didnt stick. Now comparing linux configs for cleanup and make differences between configs only as needed per features of the boards.

@JonathonHall-Purism
Copy link
Collaborator Author

Yeah @tlaurion that's the same thing I saw on Librem 11. I think I am going to exclude partitions based on type before trying to automatically mount them as /boot, e.g. there is no point trying a 'grub' partition as /boot. Or LUKS or LVM, currently, etc.

What is your vda1 in that case exactly? (Check the last column of fdisk -l /dev/vda)

When testing a possible boot device, detect its partition type and
skip grub, LUKS, and LVM partitions.  These aren't mountable as /boot,
this silences spurious exFAT errors.

In detect_boot_device, skip testing CONFIG_BOOT_DEV a second time if it
is found as a block device.  This avoids doubling any errors shown from
checking this device, no sense trying it twice.

Refactor some logic to avoid duplication - extract
device_has_partitions and use it in detect_boot_device, extract
mount_possible_boot_device and use it instead of duplicating the logic.

Move find_lvm_vg_name() to /etc/functions.

Avoid mixing up similarly-named devices like 'nvme0n1'/'nvme0n10' or
'sda'/'sdaa' - it's probably unlikely that many devices will appear,
but looking for partitions in '/sys/class/block/<device>/' instead of
'/dev/' would avoid any collisions.

Signed-off-by: Jonathon Hall <[email protected]>
@JonathonHall-Purism
Copy link
Collaborator Author

JonathonHall-Purism commented Feb 2, 2024

@tlaurion Addressed detecting the /boot device and updated OP. If your /dev/vda1 is LUKS or an LVM PV, it shouldn't emit these errors any more. edit: Oh, you said it is an FS unsupported by Heads. If it's a GPT disk we might be able to eliminate it in the same way we detect the 'grub' partition type, but for MBR almost everything just says 'Linux' 🤷

It was a fair amount of code to work around some warnings that I wish the kernel just didn't emit. But it seems preferable to the alternatives:

  • patch kernel...would have to maintain this for every kernel version
  • don't emit kernel errors on console...maybe someday, but not sure I want to make such a change for this specific issue (could discuss though if you lean this way)
  • leave it...I really don't want spurious errors showing up all the time that I have to tell users to ignore, dilutes any real errors that might appear

The refactoring / improved logic seem beneficial anyway to improve detection of /boot and add some tracing.

@JonathonHall-Purism JonathonHall-Purism marked this pull request as ready for review February 2, 2024 22:25
@tlaurion
Copy link
Collaborator

tlaurion commented Feb 3, 2024

I will have to deep check this. I installed debian with nitrokey oem image to unattendedly create installation with oem LUKS default PleaseChangeMe password a while back but never checked their fs deployment, just upgraded it to bookworm internally. Will review and probably open some issues on nitrokey doing so.

Another path would be to check level of Linux reported errors from heads kernel l and check if that error level reported can be dismissed by decreasing verbosity of heads to show only valuable errors outside of debug boards config?

My intuition points to not defaulting boot device anymore and let the logic of autodetecting boot device at factory default do the job properly without boards hardcoding it but I will double check this.

@JonathonHall-Purism
Copy link
Collaborator Author

Another path would be to check level of Linux reported errors from heads kernel l and check if that error level reported can be dismissed by decreasing verbosity of heads to show only valuable errors outside of debug boards config?

I believe this trace occurs at error level, so we would essentially silence all kernel output from the console. Maybe it is time to consider that. We do have configurable debug logging now.

My intuition points to not defaulting boot device anymore and let the logic of autodetecting boot device at factory default do the job properly without boards hardcoding it but I will double check this.

I agree, but that's not what's causing the errors to appear. The errors appear when you try to mount something that isn't an understood filesystem (with the PR promoting iso9660 above exfat, anyway). Automatically detecting /boot does that to find the partition with grub files on it.

Copy link
Collaborator

@tlaurion tlaurion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing against, looks good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants