Add variables to BLS entries only if grub is used #705
Conversation
|
This is just idea, doing some more testing now. |
The bootloader plugin needs patched BLS entries with the TuneD variables. It seems variables in the BLS entries is grub extension which is not described in the BLS specification. It can cause boot problems with e.g. systemd-boot, thus add variables to BLS entries only if the BLS entries were generated by grub. Resolves: rhbz#2323514 Signed-off-by: Jaroslav Škarvada <[email protected]>
yarda
force-pushed
the
systemd-boot-workaround
branch
from
7672d77 to
3c3d1b2
Compare
|
Tested on s390x, on f41 with grub and systemd-boot and it worked OK for me. |
I think it might be worth mentioning the low-level issue here, regardless of any specs or extensions (well, not regardless of them, but I believe this is what it comes down to): the variables in question are defined in You can use heuristics which work but are less than ideal to extract these variables anyway (or at least warn when you can't extract them), and make such systems boot without running grub (or at least warn when you can't) - I work on a loader which does this - though I believe sd-boot has no intention of doing any such thing. If you were to take this back to specs, i.e. what spec would help to resolve the above technical issues, I believe you would need a low-processing (i.e. more grubenv style) shared variable file, which grub was aware of, and was all that BLSpec compliant loaders would need to be aware of, to get hold of all variables which can validly be used there. I am not sure how/where/whether to propose this.... (EDIT: FWIW I don't think this file could be grubenv, both because that is in the wrong filesystem location, and because it has very limited space for a shared variable store.) |
|
By the way, this update looks plausible, tyvm, but is there any chance of also looking into the somewhat separate #704, which is that after an upgrade to Fedora 41, the |
|
This is fragile and against the spec. Just drop the whole concept please. |
I wouldn't say it's fragile if it worked without problem for more than a decade and will work without problem till grub is supported. Provide us better alternative and we will happily switch. And no, it isn't against BLS spec, variables aren't defined in the BLS spec. |
Tuned wasn't very widely used and as soon as it was introduced by default in Fedora, it broke the system of anyone using systemd-boot. The fact that it worked without problems in a limited environment does not mean that that approach is good. The approach is fragile because trying to change bootloader entries with
Exactly, they aren't defined so there is no syntax for them and the reference implementation doesn't understand them and fails to boot. If you prefer, we can call it "not compatible" instead of "against". The conclusion is the same. |
I wouldn't say "Tuned wasn't very widely used", TuneD is by default installed and enabled on Red Hat Enterprise Linux since version 6, i.e. for very long time on very large number of different machines. Nobody noticed the problem during F41 test days which means that nobody used systemd-boot. And it wasn't TuneD who broke things, because if you use the default Fedora boot loader - the grub, nothing broke for anybody. It's systemd-boot which isn't drop-in replacement and broke the compatibility. Maybe that's one of the reason why it still isn't the default boot loader in Fedora?
That's your subjective opinion. We used the best what was available. Provide safe polished transactional API interface, get it into the default Fedora boot loader and we will very happily use it.
It seems we are slowly moving somewhere in otherwise non productive discussion. The concept isn't "against" nor "not compatible" because it's supported by grub and you don't blame it. And we use grub not BLS. IMHO the only incompatible thing in the implementation is that initrds are on one line (supported by grub) and not multiline. This can be easily fixed, but we rather disabled the feature for systemd-boot, because most (probably all?) consumers of this feature don't have systemd-boot. If significant consumers have systemd-boot we will have to support is somehow. I am open to suggestions and proposals on it, but advice of type: "don't touch kernel args" are really not helpful here. |
The test days generally test the defaults — which is fine, but it's not going to catch issue which do not affect the installer and the default installation. Most non-standard installation schemes and file systems and software is not tested during test days, but this does not mean that it'd be OK to break them. The situation is that you're modifying config files for a different package. Config files that are defined by a published standard and used by multiple consumers. And your modification is not compatible with the reference implementation in the package that defines the standard. Yes, it works with grub. Grub set you up for failure, by squatting on a name and pushing a misguided and incompatible implementation. So I'm not saying that it's your fault to fall into this trap. But it's still something that should not be done. |
|
At this point, it's probably worth noting, as pointed out here, that grub BootLoaderSpecByDefault files would be in |
Significant bug reports we got from the test days were for non standard situations and configurations, because defaults are usually tested by us and our QA.
No guideline enforced BLS compliance and IMHO the only incompatibility was initrds on single line instead of multiline which shouldn't be hard to fix. |
I'm not sure this is right. grb+blscfg supports variables and mutliple initrds on one line. Pure blspec (e.g. systemd-boot) supports neither of these. Since pure blspec does not support empty initrd lines either (AFAIK) then I am not sure there is much to be gained by splitting initrd onto multiple lines (one of which is from a variable, which may be empty) at this point. |
This is not (fully) correct. The spec says that entries from both locations should be combined. So sd-boot and bootctl would look in both places. New entries would generally be installed to So generally the answer is that when reading those files, both locations have the same rules. When writing, it's … complicated … but generally it's expected that the second location is used if both are available and the ESP is used if only one is. |
Hmm, no. The whole idea of config variables is not supported by the standard. So it's not just a question of one line vs. two. The basic approach is unsupported. |
Correct me if I'm wrong, but doesn't https://uapi-group.org/specifications/specs/boot_loader_specification/ say that compliant EDIT: If you're suggesting that XBOOTLDR is meant to be the |
Of course the TuneD bootloader feature will not work, that's why we disabled it for systemd-boot, but it shouldn't cause possible trouble upon automatic conversion of grub config files etc. We still need some API or mechanism for easy addition/removal of kernel boot args and initrds overlays, because otherwise we would have to find & replace in the bootloader entries, which is really dirty approach, variables made it much cleaner job. |
Yes. XBOOTLDR would be mounted to |
Ah. Blimey. This seems a mess to me. Not least because it makes it hard or impossible to switch back to grub, since /boot normally contains grub-specific (and instance-specific) subfolders, and (what are presumed to be, and may be cleaned up as) instance-specific kernel and initrd files. I guess that should not matter if grub is not running, and any such old files could still be somewhere else.... hmmmmm.... I think I remain unconvinced that this part of the proposal is any good for co-existence and switching back and forth between systemd-boot and grub, but thank you for the clarification and link. |
|
@keszybz - Is there any existing proposal within systemd-boot for how additional components (such as TuneD) should come along and add parameters (kernel options, perhaps even additional initrds) to the system defaults in /loader/entries? |
|
There are generally the following extension mechanism:
This is a big topic. The docs are not very well structured at this point, but the basic interfaces are described. Generally, all this should match what the code actually does. |
|
@yarda @keszybz - Not that it's really my job to carry on commenting here (except as an interested third-party trying to maintain a loader compatible with all these setups), but given that https://uapi-group.org/specifications/specs/boot_loader_specification/#standard-conformance-marker-file
presumably must be referring to BootLoaderSpecByDefault, then it seems that something like the proposed fix here is arguably fine, because it makes TuneD's use of grub variables nothing to do with systemd-boot? There could be an alternative path in TuneD's code to do something different, when pure BLSpec (such as systemd-boot) is in play - but even without it, TuneD can just say that its |
The bootloader plugin needs patched BLS entries with the TuneD variables. It seems variables in the BLS entries is grub extension which is not described in the BLS specification. It can cause boot problems with e.g. systemd-boot, thus add variables to BLS entries only if the BLS entries were generated by grub.
Resolves: rhbz#2323514