Dear maintainer,
In some situations, check-missing-firmware fails to find loose firmware
file or package file on partitioned removable media (as most USB drives
are nowadays).
Observed behaviour:
- When looking for a loose firmware file, check-missing-firmware calls
"mountmedia" which returns after mounting the first device which can be
mounted.
- When looking for a firmware package, check-missing-firmware calls
"mountmedia driver" which returns after mounting the first device which
can be mounted and contains {,/firmware}/*.{deb,udeb,ude} files.
mountmedia tries to mount whole drives first, then partitions, in
alphanumerical order.
In either case, the lookup fails if the mounted device is not the one
which contains the requested firmware, next devices are not tried.
The usual workaround is to mount the right device on /media by hand.
Here are tested examples of such situations.
* Example 1
sda = internal drive
sdb = USB flash drive with debian-11.6.0-amd64-i386-netinst.iso
sdc1 = USB flash drive with requested firmware file
-> mountmedia mounts the d-i EFI partition sdb2
The firmware media is usually connected after booting the installer.
If both media are connected before the usb-storage module is loaded,
there is 50% probability of failure.
* Example 2
sda1 = internal EFI/FAT/ISO9660 partition
sdb1 = USB flash drive with requested firmware file
sdc = USB flash drive with debian-11.6.0-amd64-i386-netinst.iso
-> mountmedia mounts the internal partition sda1
Internal drives are usually detected before USB drives and may contain
an EFI partition.
* Example 3
sda3 = internal EFI/FAT/ISO9660 partition with other {,/firmware}/*.deb
sdb1 = USB flash drive with deb package containing requested firmware
sdc = USB flash drive with debian-11.6.0-amd64-i386-netinst.iso
-> mountmedia mounts the internal partition sda3
This is a much less common situation than the previous ones, so loading
firmware from package files is usually more reliable. However all
required firmware packages must be in the same location.
Here is a situation which unexepectedly works:
* Example 4
sda = internal drive
sdb = USB flash drive with firmware-11.6.0-amd64-i386-netinst.iso
without the requested firmware package
sdc1 = USB flash drive with deb package containing requested firmware
mountmedia tries to mount sdb1 which contains /firmare/*.deb, but
"fortunately" busybox mount fails to do so because sdb1 is already
mounted (unlike util-linux mount). IMO we should not rely on this.
IMO the correct behaviour is:
for each drive or partition
if device not mounted
try to mount device
if mount succeeds
look for missing loose firmware files
look for missing firmware in deb package files
But I do not see how this can be achieved by still using mountmedia...
Control: tags -1 patch hw-detect 1.155 implementing a possible solution. The first one adds subcommands into mountmedia to list and mount any media. The second one uses the subcommands in check-missing-firmware to implement the above desired behaviour. The third one is an optimization to reduce the search time: it updates the missing file list after installing a loose file or package and stops scanning remaining media and packages if the missing file list is empty. I chose to add subcommands to mountmedia in order to reuse existing code and keep media management in one place (mountmedia) instead of duplicating some code in hw-detect. Any feedback (comments and tests) appreciated.
Hi Pascal, Pascal Hambourg <pascal@plouf.fr.eu.org> (2023-04-01): Thanks for your work on this. As for the other bug report, that's been on my todo list since you submitted those patches, but time is running out and I'm very much not thrilled with risking anything on the firmware front, even if the existing situation regarding “loose material” isn't ideal. I really looked at those patches, but I'm feeling much too nervous about them, and that's definitely not a feeling I'm going to ignore. At this stage, I'll err on the side of caution, and keep the “suboptimal” support for loose firmware files/packages as it is. Some patches touch the “main code” and that's definitely a red flag, days away from the upcoming releases (RC 3, RC 4, and 12.0). As you noted a while back, the search has been problematic for a very long while, so Bookworm isn't regressing from Bullseye as far as I understand (even if it did at some point, when I fumbled the condition guarding the mountmedia calls…). If anything, Bookworm is expected to be *far better* at firmware support, thanks to official non-free-firmware support, and thanks to improvements like the “address resolution” for USB/MHI. I'm open to the following compromise: look into those patches once the next development cycle opens, test them, get them into some Alpha release, run as many tests as we like given we'll have plenty of time… And once we're absolutely convinced they're helping, and not triggering some nasty regression (breaking support for “official” firmware packages, slowing down the installation because of inefficient searches, or infinite loops, or whatever), then only consider backporting them to stable via a point release. For the time being, I'm adding a reference to this topic to my list of things for 12.1 (even if it's likely to be shifted to some later 12.x point release, since 12.1 is very likely to happen before the D-I Trixie Alpha 1 release). https://salsa.debian.org/installer-team/debian-installer/-/issues/3 I hope this all makes sense and isn't too much of a disappointment. Cheers,
And before I forget… Pascal Hambourg <pascal@plouf.fr.eu.org> (2023-04-01): This should be hw-detect, rather than hw-media. :) Since new subcommands are supposed to have run and exited before, I would expect the default operation to require no changes at all, so the /media → $MNT update seems weird. This part could have been dropped too, even if it's very minor compared to the question above. Cheers,
Thanks for spotting this. As you may have noticed previously, I have a tendency to insert unrelated trivial "cosmetic" fixes in my patches. My purpose here was to consistently make use of $MNT instead of /media like in the rest of the code. Another cosmetic fix...