factory: do not mount /lib/{modules,firmware} if there is a drivers tree

snapcore / core-initrd

initrd implementation in ubuntu core

https://ubuntu.com/core/docs

GNU General Public License v3.0

9 stars 27 forks source link

factory: do not mount /lib/{modules,firmware} if there is a drivers tree #238

Closed alfonsosanchezbeato closed 2 months ago

alfonsosanchezbeato commented 5 months ago

Do not mount /lib/{modules,firmware} in case snapd created a drivers tree for our kernel snap, as in that case these mounts will be created by snapd-generator from the base or from the snapd deb package for hybrid systems. This situation is expected to happen only for UC24+.

alfonsosanchezbeato commented 5 months ago

Draft for the moment, please do not merge - This will need to wait for snapd 2.63 to be in stable

alfonsosanchezbeato commented 3 months ago

@valentindavid @Meulengracht this is ready for review now.

valentindavid commented 2 months ago

@alfonsosanchezbeato Could we have a reference of what PR from snapd will expect this?

alfonsosanchezbeato commented 2 months ago

@alfonsosanchezbeato Could we have a reference of what PR from snapd will expect this?

This change enables using (mounting) a kernel drivers tree when created by snapd. All the needed changes in snapd were already merged. The ways this works is:

snapd will generate the drivers tree only for core24/hybrid 24.04 or more modern, as in these cases the systemd generator that creates the mounts for /var/lib/snapd/kernel/... exists
With this change, the initramfs will not mount kernel in /lib/{firmware,modules} if the drivers tree exists
Instead, the systemd generator from the rootfs will generate the right mounts from the drivers tree to /lib/{firmware,modules}

Hopefully this answers your question.

valentindavid commented 2 months ago

I suppose this was this one: https://github.com/snapcore/snapd/pull/13923

alfonsosanchezbeato commented 2 months ago

I suppose this was this one: snapcore/snapd#13923

And https://github.com/snapcore/snapd/pull/13563 and possibly others.

alfonsosanchezbeato commented 2 months ago

@valentindavid I now check for kernel snap name / revision, I think that should handle most corner case situations. And hopefully the change guarantees that you will have a drivers tree compatible with the loaded kernel, and any core24/hybrid 24.10 system has in theory the right generator for these mounts.

I have tested the change for UC24 and hybrid in different scenarios, and seems to work as expected.

alfonsosanchezbeato commented 2 months ago

The tests-snapd tests seem to fail because no eth device is created in the VM. This looks as the same as https://github.com/snapcore/core-initrd/pull/246 , so the issue does not seem directly related to the PR.

alfonsosanchezbeato commented 2 months ago

I'd like to set snapd.debug=1 in the kernel command line, but we need https://github.com/snapcore/snapd/pull/14077 to land first. Otherwise the tests should pass now.