projg2 / eclean-kernel

Installed kernel cleanup tool
GNU General Public License v2.0
31 stars 10 forks source link

Follow symlinks in unprefixify() #41

Closed bstaletic closed 10 months ago

bstaletic commented 10 months ago

This allows eclean-kernel to correctly identify the kernels used by the bootloader config in the following scenario:

  1. LILO says image=/boot/vmlinuz
  2. /boot/vmlinuz is a symlink to /boot/vmlinuz-X.Y.Z-suffix, which is the actual kernel.

Before this commit, eclean-kernel -ap woulc complain that

Note: strangely named used kernel: /boot/vmlinuz

and then list vmlinuz-X.Y.Z-suffix as one of the kernels to be removed.

Note: This does affect my system, where the actual lilo.conf is something like this:

image=/boot/vmlinuz
        label=default
        read-write
        root=/dev/sda1
        append="net.ifnames=0 init=/sbin/openrc-init sysrq_always_enabled=1"

and new kernels are installed through installkernel which comes from installkernel-gentoo, which produces the following /boot/:

-rw-r--r-- 1 root root     512 Jan  3  2022 boot.0800
lrwxrwxrwx 1 root root      18 Aug 13 11:49 config -> config-6.4.10-test
-rw-r--r-- 1 root root  109325 Aug 13 11:49 config-6.4.10-test
-rw-r--r-- 1 root root  109248 Jul 26 15:07 config-6.4.6-test
-rw-r--r-- 1 root root  109229 Aug  1 13:30 config-6.4.7-test
-rw-r--r-- 1 root root  109229 Aug  4 08:24 config-6.4.8-test
-rw-r--r-- 1 root root  109324 Aug 10 16:45 config-6.4.9-test
lrwxrwxrwx 1 root root      17 Aug 10 16:45 config.old -> config-6.4.9-test
-rw-r--r-- 1 root root       0 Jan  2  2022 .keep
-rw------- 1 root root   23040 Aug 13 11:49 map
lrwxrwxrwx 1 root root      22 Aug 13 11:49 System.map -> System.map-6.4.10-test
-rw-r--r-- 1 root root 4288104 Aug 13 11:49 System.map-6.4.10-test
-rw-r--r-- 1 root root 4286315 Jul 26 15:07 System.map-6.4.6-test
-rw-r--r-- 1 root root 4286495 Aug  1 13:30 System.map-6.4.7-test
-rw-r--r-- 1 root root 4286603 Aug  4 08:24 System.map-6.4.8-test
-rw-r--r-- 1 root root 4287870 Aug 10 16:45 System.map-6.4.9-test
lrwxrwxrwx 1 root root      21 Aug 10 16:45 System.map.old -> System.map-6.4.9-test
lrwxrwxrwx 1 root root      19 Aug 13 11:49 vmlinuz -> vmlinuz-6.4.10-test
-rw-r--r-- 1 root root 7913168 Aug 13 11:49 vmlinuz-6.4.10-test
-rw-r--r-- 1 root root 7908880 Jul 26 15:07 vmlinuz-6.4.6-test
-rw-r--r-- 1 root root 7908176 Aug  1 13:30 vmlinuz-6.4.7-test
-rw-r--r-- 1 root root 7909136 Aug  4 08:24 vmlinuz-6.4.8-test
-rw-r--r-- 1 root root 7911952 Aug 10 16:45 vmlinuz-6.4.9-test
lrwxrwxrwx 1 root root      18 Aug 10 16:45 vmlinuz.old -> vmlinuz-6.4.9-test
bstaletic commented 10 months ago

As for the test, I have written this to prove that it works:

    def test_removal_bootloader_symlink(self) -> None:
        td = Path('/boot')

        write_bzImage(td / 'kernel-0.0.0-suffix', b'kernel with a symlink')
        write_bzImage(td / 'kernel-5.4.3-suffix', b'just a kernel')
        Path( td / 'symlink_to_old' ).symlink_to( td / 'kernel-0.0.0-suffix' )

        kernels = [
            Kernel('0.0.0-suffix'),
            Kernel('5.4.3-suffix')
        ]

        kernels[0].all_files = [ KernelImage( td / 'kernel-0.0.0-suffix' ) ]
        kernels[1].all_files = [ KernelImage( td / 'kernel-5.4.3-suffix' ) ]

        class MockBootloader(Bootloader):
            name = 'mock'

            def __call__(self) -> typing.Iterable[str]:
                yield str(td / 'symlink_to_old')

        self.assertEqual(
            get_removal_list(kernels,
                             sorter=VersionSort(),
                             limit=None,
                             destructive=False,
                             bootloader=MockBootloader()),
             {kernels[1]: ['not referenced by bootloader (mock)'] })

However, since get_removal_list() uses regex that hard-codes the /boot part, this test writes to /boot which is far from ideal. Any suggestions?

bstaletic commented 10 months ago

Thanks for taking a look. Okay, but I am afraid I do not know any other way to resolve a symlink and get a full path, which is all I am attempting to do.

I have tried os.readlink(), but it only produces vmlinuz-6.4.9-test, not /boot/vmlinuz-6.4.9-test. pathlib.Path.resolve() and pathlib.Path.readlink() behave the same as os.readlink().

 

EDIT: Would the following be appropriate?

resolved_path = Path(fn).resolve()
fn = str(resolved_path.parent) + str(resolved_path)
mgorny commented 10 months ago

My point is that we shouldn't be doing that on paths at all. From a quick look, we want to match kernels against bootloader entries, don't we? What the code really should do is use samefile() to compare their equality.

I know it's a major change, so I can take a look at rewriting that part myself when I have some time.

bstaletic commented 10 months ago

Okay, now we are on the same page. I will close this pr and open a bug report. I can also take a closer look at what eclean-kernel is doing and how to achieve what you propose.

mgorny commented 10 months ago

Thanks a lot!