Adapt file name entry for grub2-bls in TW

[aplanas]

On TW with grub2-bls the entry order is only based on the filename.

This PR refactor the logic to compose the entry filename, adding version prefix (snapshot ID) in the filename for the non-ro entries

[aplanas]

cc @TobiPeterG

[TobiPeterG]

This issue sounds more like an issue on the grub-bls side. The bootloader specification specifically mentions that the version specified in the entry file is used for sorting https://uapi-group.org/specifications/specs/boot_loader_specification/#sorting

"version is a human-readable version for this menu item. This is usually the kernel version and is intended for use by OSes to install multiple kernel versions with the same title field. This field is used for sorting entries, so that the boot loader can order entries by age or select the newest one automatically."

Also, we set the sort-key, so it should be sorted by that at least.

I'm not sure if we should "fix" this on our side with a workaround.

[TobiPeterG]

Also, how would we remove this workaround again when it gets fixed? The file name is used in other functions as well, isn't it?

[aplanas]

This issue sounds more like an issue on the grub-bls side.

It is, yes.

The bootloader specification specifically mentions that the version specified in the entry file is used for sorting https://uapi-group.org/specifications/specs/boot_loader_specification/#sorting

Yes, but also the filename:

""" At the end, if necessary, when sort-key is not set or those fields are not set or are all equal, the boot loader should sort using the file name of the entry (decreasing version sort), with the suffix removed. """

I'm not sure if we should "fix" this on our side with a workaround.

I can try to fix it in grub2-bls, but this will take months.

[aplanas]

Also, how would we remove this workaround again when it gets fixed?

I am not sure that it will fixed. But if this happens then a 'remove-all-kernels' / 'add-all-kernels' recreate the entries with the new name.

The file name is used in other functions as well, isn't it?

How this change affect those functions?

[TobiPeterG]

This issue sounds more like an issue on the grub-bls side.

It is, yes.

The bootloader specification specifically mentions that the version specified in the entry file is used for sorting https://uapi-group.org/specifications/specs/boot_loader_specification/#sorting

Yes, but also the filename:

""" At the end, if necessary, when sort-key is not set or those fields are not set or are all equal, the boot loader should sort using the file name of the entry (decreasing version sort), with the suffix removed. """

Yes, after the sort-key at least.

I'm not sure if we should "fix" this on our side with a workaround.

I can try to fix it in grub2-bls, but this will take months.

ouch, that's not great of course. However, I see multiple places in sdbootutil where the config file is used directly. We would have to change all of them. Also, again, how canwe revert this? Having two naming schemes isn't optimal and I would not like to have this permanently. But when we revert it, the affected entries would not be recognized by sdbootutil anymore.

[TobiPeterG]

Also, how would we remove this workaround again when it gets fixed?

I am not sure that it will fixed. But if this happens then a 'remove-all-kernels' / 'add-all-kernels' recreate the entries with the new name.

But then sdbootutil has to always carry the logic to detect these different file names, wouldn't it? How else would sdbootutil find the to-be-removed entries?

The file name is used in other functions as well, isn't it?

How this change affect those functions?

We wouldn't be able to find the entries with sdbootutil anymore when it's not changed at these other places as well, at least in those functions.

[aplanas]

However, I see multiple places in sdbootutil where the config file is used directly. We would have to change all of them.

Maybe I can refactor the code a bit, to compose the entry file name in a single place.

Also, again, how canwe revert this? Having two naming schemes isn't optimal and I would not like to have this permanently. But when we revert it, the affected entries would not be recognized by sdbootutil anymore.

Right. Sadly software out there is full of those imperfections and limitations, and somehow we need to deal with them. There are other bootloaders that support BLS (zipl, and one other for powerpc), and they will have their quirks and bugs too, and we should adapt to those (I do not think that they are opensource)

The BLS does not says that the filename should be formatted in any specific way, and sdbootutil make an early decision that seems that does not cover all the situations.

[TobiPeterG]

However, I see multiple places in sdbootutil where the config file is used directly. We would have to change all of them.

Maybe I can refactor the code a bit, to compose the entry file name in a single place.

Also, again, how canwe revert this? Having two naming schemes isn't optimal and I would not like to have this permanently. But when we revert it, the affected entries would not be recognized by sdbootutil anymore.

Right. Sadly software out there is full of those imperfections and limitations, and somehow we need to deal with them. There are other bootloaders that support BLS (zipl, and one other for powerpc), and they will have their quirks and bugs too, and we should adapt to those (I do not think that they are opensource)

The BLS does not says that the filename should be formatted in any specific way, and sdbootutil make an early decision that seems that does not cover all the situations.

OK, then maybe an alternative solution. I don't like the idea of setting a prefix to 0. How about this: We set snapper- as prefix for tumbleweed, when it's a entry created as ro snapshot and we move the snapshot number right behind it. So we would have e.g. opensuse-tumbleweed-469-6.8.7-lqx2-2-liquorix.conf as regular entry and snapper-opensuse-tumbleweed-470-6.8.7-lqx2-2-liquorix.conf

Would this sort correctly? I wouldn't mind using this naming scheme for systemd-boot as well, it includes the sort-key and version :)

[TobiPeterG]

The refactor is a good idea any way, I'm not a fan of having the magic numbers/strings all over the script, but it also didn't bother me too much yet lol. The path for the snapshtos is also hardcoded in some places, don't like that either.

[aplanas]

I don't like the idea of setting a prefix to 0.

Me neither ...

How about this: We set snapper- as prefix for tumbleweed, when it's a entry created as ro snapshot and we move the snapshot number right behind it. So we would have e.g. opensuse-tumbleweed-469-6.8.7-lqx2-2-liquorix.conf as regular entry and snapper-opensuse-tumbleweed-470-6.8.7-lqx2-2-liquorix.conf

I think this will work. I added 0- to avoid the issue when token-id is is hash that start with a number. Adding only snapper- will fix the issue if for the non-RO we adder ${snapshot-number}- as prefix, as the number will be sorted first.

Would this sort correctly? I wouldn't mind using this naming scheme for systemd-boot as well, it includes the sort-key and version :)

lets start with the refactor and adding the snapper prefix, from there we can iterate to find a good naming schema

[TobiPeterG]

I don't like the idea of setting a prefix to 0.

Me neither ...

How about this: We set snapper- as prefix for tumbleweed, when it's a entry created as ro snapshot and we move the snapshot number right behind it. So we would have e.g. opensuse-tumbleweed-469-6.8.7-lqx2-2-liquorix.conf as regular entry and snapper-opensuse-tumbleweed-470-6.8.7-lqx2-2-liquorix.conf

I think this will work. I added 0- to avoid the issue when token-id is is hash that start with a number. Adding only snapper- will fix the issue if for the non-RO we adder ${snapshot-number}- as prefix, as the number will be sorted first.

Wait, what is the issue? The snapper entries would always be below the regular entries, wouödn't they? If the token-id is part of the name, it starts with a number, and otherwise it starts with opensuse, which is also beofre snapper.

Would this sort correctly? I wouldn't mind using this naming scheme for systemd-boot as well, it includes the sort-key and version :)

lets start with the refactor and adding the snapper prefix, from there we can iterate to find a good naming schema

That's a good idea :)

[aplanas]

Wait, what is the issue? The snapper entries would always be below the regular entries, wouödn't they? If the token-id is part of the name, it starts with a number, and otherwise it starts with opensuse, which is also beofre snapper.

Yes and no. The snapper entries should be listed below, but the token_id can be many things: can be opensuse, something provided by the user, or the machine id (see https://github.com/openSUSE/sdbootutil/blob/main/sdbootutil#L362). So there is a chance that the token contains numbers that alter the order.

[TobiPeterG]

Wait, what is the issue? The snapper entries would always be below the regular entries, wouödn't they? If the token-id is part of the name, it starts with a number, and otherwise it starts with opensuse, which is also beofre snapper.

Yes and no. The snapper entries should be listed below, but the token_id can be many things: can be opensuse, something provided by the user, or the machine id (see https://github.com/openSUSE/sdbootutil/blob/main/sdbootutil#L362). So there is a chance that the token contains numbers that alter the order.

But if "snapper-" is always in front of the filename, how can that happen? Or do you mean the regular entries could have a token that makes them move after the snapper entries?

[aplanas]

But if "snapper-" is always in front of the filename, how can that happen? Or do you mean the regular entries could have a token that makes them move after the snapper entries?

No, that is what I mean. We need to add '0-' or 'snapper-'. I added now 'snapper-', this will fix the ordering.

Like this:

# ls -la /boot/efi/loader/entries/
total 56
drwxr-xr-x 2 root root 8192 Jun 24 19:59 .
drwxr-xr-x 3 root root 8192 Jun 20 14:17 ..
-rwxr-xr-x 1 root root  539 Jun 24 19:58 1-opensuse-tumbleweed-6.9.5-1-default-1.conf
-rwxr-xr-x 1 root root  624 Jun 24 19:59 snapper-opensuse-tumbleweed-6.9.5-1-default-2.conf
-rwxr-xr-x 1 root root  625 Jun 24 19:59 snapper-opensuse-tumbleweed-6.9.5-1-default-3.conf
-rwxr-xr-x 1 root root  624 Jun 24 19:59 snapper-opensuse-tumbleweed-6.9.5-1-default-4.conf
-rwxr-xr-x 1 root root  625 Jun 24 19:59 snapper-opensuse-tumbleweed-6.9.5-1-default-5.conf

[aplanas]

@TobiPeterG should be done. I am still using - as separator

[TobiPeterG]

But if "snapper-" is always in front of the filename, how can that happen? Or do you mean the regular entries could have a token that makes them move after the snapper entries?

No, that is what I mean. We need to add '0-' or 'snapper-'. I added now 'snapper-', this will fix the ordering.

Like this:

# ls -la /boot/efi/loader/entries/
total 56
drwxr-xr-x 2 root root 8192 Jun 24 19:59 .
drwxr-xr-x 3 root root 8192 Jun 20 14:17 ..
-rwxr-xr-x 1 root root  539 Jun 24 19:58 1-opensuse-tumbleweed-6.9.5-1-default-1.conf
-rwxr-xr-x 1 root root  624 Jun 24 19:59 snapper-opensuse-tumbleweed-6.9.5-1-default-2.conf
-rwxr-xr-x 1 root root  625 Jun 24 19:59 snapper-opensuse-tumbleweed-6.9.5-1-default-3.conf
-rwxr-xr-x 1 root root  624 Jun 24 19:59 snapper-opensuse-tumbleweed-6.9.5-1-default-4.conf
-rwxr-xr-x 1 root root  625 Jun 24 19:59 snapper-opensuse-tumbleweed-6.9.5-1-default-5.conf

I might be dumb. Can you provide an example where it would break without adding "1-" as prefix for the regular entry?

[aplanas]

I might be dumb. Can you provide an example where it would break without adding "1-" as prefix for the regular entry?

If token_id="$machine_id", and $machine_id="20fe...", then "1-20fe..." will be shown later than "20fe...-", but adding "snapper-" as a prefix, "1-20fe..." will be shown before than "snapper-20fe..."

[TobiPeterG]

I might be dumb. Can you provide an example where it would break without adding "1-" as prefix for the regular entry?

If token_id="$machine_id", and $machine_id="20fe...", then "1-20fe..." will be shown later than "20fe...-", but adding "snapper-" as a prefix, "1-20fe..." will be shown before than "snapper-20fe..."

In this example, we would have the entries 20fe...-6.9.5-1-default-1.conf snapper-20fe...-6.9.5-1-default-2.conf snapper-20fe...-6.9.5-1-default-3.conf

Won't the 20fe...-6.9.5-1-default-1.conf entry always be sorted in front of the snapper entries?

[aplanas]

In this example, we would have the entries

Ah you are asking without adding the prefix then? In that case if "token_id" is user provided and start by some letter after 's' it will be put after snapper, right?

For example, token_id=$(cat /etc/kernel/entry-token) (see settle_entry_token), and token_id="xyz", then "xyz-.." will be put after "snapper-xyz-.."

[TobiPeterG]

In this example, we would have the entries

Ah you are asking without adding the prefix then? In that case if "token_id" is used provided and start by some letter after 's' it will be put after snapper, right?

For example, token_id=$(cat /etc/kernel/entry-token) (see settle_entry_token), and token_id="xyz", then "xyz-.." will be put after "snapper-xyz-.."

Ah alright, now it makes sense. Thank you :) Didn't think about this scenario. Hmm. The "1-" in front is still not very nice. But I guess checking if the entry_token contains opensuse and if not, adding it as prefix to the file name is a bit too complicated, right?

[aplanas]

The "1-" in front is still not very nice. But I guess checking if the entry_token contains opensuse and if not, adding it as prefix to the file name is a bit too complicated, right?

Pff... we can use rpmdev-vercmp, but is in the rpmdevtools package, and I we should not depend on it. We can use zypper vcmp but some users are using dnf as package manager.

The last option is rpm --eval "%{lua:print(rpm.vercmp('1.2-1', '2.0-1'))}" (saw in stackoverflow). Do you want to use this?

[aplanas]

Also, 'snapper-' is bigger than 'opensuse-', so 'snapper-' will appears always on top : S

[TobiPeterG]

oof I think this is all too complicated. Do I understnad you correctly that the snapper- doesn't work? Then the 0- and 1- is probably the best solution, even though I really don't like it. :D

[aplanas]

oof I think this is all too complicated.

This works:

'1-opensuse-tumbleweed-6.9.5-1-default-1.conf' > '0-opensuse-tumbleweed-6.9.5-1-default-1.conf'

This also works:

'1-opensuse-tumbleweed-6.9.5-1-default-1.conf' > 'snapper-opensuse-tumbleweed-6.9.5-1-default-1.conf'

But this does not:

'opensuse-tumbleweed-6.9.5-1-default-1.conf' < 'snapper-opensuse-tumbleweed-6.9.5-1-default-1.conf'

In the initial code I implemented the first one, in this current code I implemented the second one

[TobiPeterG]

Alright, then let's keep it that way. :) Will have a look at the code later on :)

[aplanas]

This should also work:

'system-opensuse-tumbleweed-6.9.5-1-default-1.conf' > 'snapper-opensuse-tumbleweed-6.9.5-1-default-1.conf'

[TobiPeterG]

This should also work:

'system-opensuse-tumbleweed-6.9.5-1-default-1.conf' > 'snapper-opensuse-tumbleweed-6.9.5-1-default-1.conf'

Uh adding system is better, I like that :)

[aplanas]

Uh adding system is better, I like that :)

Done : )

[TobiPeterG]

code-wise, it's looking good so far. Have you tested the changes thoroughly? I don't want to introduce breaking changes again lol (so UI, install command, remove command etc.)

[TobiPeterG]

Oh one thing: the current entries won't be recognized anymore, right? We should handle this as well, as e.g. Aeon uses systemd-boot by default already with sdbootutil.

[aplanas]

code-wise, it's looking good so far. Have you tested the changes thoroughly?

I tested in the new TW / GRUB2-BLS images and in the TW / sd-boot ones, to see that the entries have a different name schema.

But not the UI, I was planning to remove this feature

I don't want to introduce breaking changes again lol (so UI, install command, remove command etc.)

What breaking change did we introduced?

Oh one thing: the current entries won't be recognized anymore, right? We should handle this as well, as e.g. Aeon uses systemd-boot by default already with sdbootutil.

Not exactly? The current entries are fine for the current systems. Aeon uses sd-boot and this name schema did not change. You can see in the code that only in non transactional with grub2 the name is not different, but there is not a released image with this combination yet