On Tue, Nov 05, 2024 at 03:56:57PM +0100, Jaroslav Škarvada wrote:
> On Tue, Nov 5, 2024 at 12:04 PM Zbigniew Jędrzejewski-Szmek
> <zbyszek@xxxxxxxxx> wrote:
> >
> > On Mon, Nov 04, 2024 at 05:06:34PM +0100, Jaroslav Škarvada wrote:
> > > On Mon, Nov 4, 2024 at 4:31 PM Fabio Valentini <decathorpe@xxxxxxxxx> wrote:
> > > >
> > > > On Mon, Nov 4, 2024 at 4:20 PM Jaroslav Škarvada <jskarvad@xxxxxxxxxx> wrote:
> > > > >
> > > > > On Mon, Nov 4, 2024 at 3:02 PM Ian Pilcher <arequipeno@xxxxxxxxx> wrote:
> > > > > >
> > > > > > On 11/4/24 7:42 AM, Fabio Valentini wrote:
> > > > > > > Oof ... the fact that tuned mangles bootloader parameters wasn't
> > > > > > > mentioned at all in the Change Proposal to enable it by default. To me
> > > > > > > that would have been a no-go. (I already voted -1 for this change for
> > > > > > > other reasons, but these bootloader shenanigans definitely don't make
> > > > > > > me regret that decision ...)
> > > > > >
> > > > > > Should there be a packaging guideline about this?
I forgot to answer here. I think we actually don't have a clear
guideline about this. There is a general rule that packages shouldn't
touch files owned by other packages. This is common sense, but I don't
think it's actually written down in our guidelines. And of course
there are exceptions… But one should be very very careful when
touching anything releated to boot loader config. And doing it from a
high-level userspace program not owning the config seems very iffy.
I certainly wouldn't expect a package like tuned getting inself
involved in writing of boot loader entries.
> > > > > kernel command line parameters can be changed by the bootloader plugin
> > > > > which is not used in the profiles that were used in the tuned-ppd
> > > > > replacement. So that's probably why it wasn't explicitly mentioned.
> > > > > Patching of bootloader entries is done to clearly mark what can be
> > > > > changed by TuneD, i.e. by the $tuned_* variables. If the bootloader
> > > > > plugin is not used, these variables are empty. It's interesting that
> > > > > this problem wasn't reported during the F41 test days
> > > >
> > > > I wonder if it would be possible to just ... not set bootloader
> > > > parameters at all if the bootloader plugin isn't used?
> > > > The current implementation seems like a very brittle solution to me
> > > > (as witnessed by the systemd-boot breakage).
> > > >
> > > > Fabio
> > >
> > > IMHO this is the first problem with it in a very long time (not
> > > counting the s390x zipl problem). To be honest this feature was in
> > > TuneD years before the Boot Loader Specification (BLS) appeared and
> > > when Fedora switched to BLS we used grub as the reference BLS
> > > implementation (because AFAIK there was nothing else BLS ready at that
> > > time).
> >
> > No, that gets the timeline completely backwards. $tuned_params
> > were introduced in 2014, and the Boot Loader Specification is from
> > before 2013 (the doc was added to the systemd tree in May 2013,
> > but that was a conversion from an earlier moin wiki page).
> >
> TuneD used variables in grub configuration files long before BLS ever
> existed and when grub introduced BLS support we followed it, i.e. to
> support grub configuration files.
Variables in grub configuration files are fine. Variables in BLS are
different story. BLS — the specification — never supported variables,
and as I wrote in the other part of reply, grub initially tried to
introduce support for variables, but this was always against the
spec, never worked properly, and since many years even the grub
developers have dropped support for the idea.
> > Using grub as a reference implementation for BLS is very wrong.
> > For historical reasons, the early implementation in grub completely
> > misunderstood how BLS is supposed to work. Fortunately, that early
> > code has been ripped out and grub now tries to follow the
> > specification.
> >
> > > The bootloader entries are conservatively (excluding rescue entries)
> > > patched in the kernel installation script, which IMHO should run as
> > > the root during kernel update, so probably in a transaction. IMHO
> > > there is a lower chance that something could go wrong than patching it
> > > in the daemon.
> >
> > It's not "conservative" when it goes completely agains the specification
> > and breaks the reference implementation :(
> >
> Otherwise grub would be "completely against
> the specification", because it supports it.
Being "completely against the specification" was about the inserting
of characters at the end of the initrd line. The initrd line contains
a path, so of course appending anything to that means that the loader
tries to access a different path. This breaks boot, and I think it's
fair to say that goes against the specification.
> AFAIK BLS says nothing about variables thus if it supports variables
> it's not against the spec.
Heh, that's a cool idea. But that's not how standards work.
You cannot just insert things at random points in low-level config
files or protols. How do you think this would work?
You insert "$tuned_params", I insert "{tuned_params}", and GNU people
insert "@TUNED_PARAMS@", all because the spec didn't say that's
forbidden?
> > > The problem appeared because systemd-boot doesn't support variable
> > > expansion as grub does, because it is not written in the BLS. I am
> > > going to patch the 92-tuned.install script to disable the bootloader
> > > feature in such case as we already did with the s390x zipl loader
> >
> > Yes, thank you. Please just drop it everywhere, it makes no sense
> > as variable expansion is not going to be added to the specification.
> >
> TuneD is not the only software that uses variables in grub
> configuration files. It's a clean and safe method for customization of
> bootloader args and it's a pity it's not mentioned in the BLS spec
Once again, I don't understand why you think the BLS should work the
same as grub config files. The basic idea of Grub is Turing-complete
configuration language where the complexity is pushed into the config.
The basic idea of gummiboot/sd-boot/BLS is a simple format "that can
be understood by different boot loader implementations, operating
systems, and userspace programs" (quoting the intro to the spec). The
complexity is pushed into the tools creating the boot entries, and the
interpretation must be unambiguous. The underlying philosophies,
syntax, and functionality are all significantly different between
BLS and grub.
Interoperability that is the goal of BLS can only be achieved if the
different implementations follow the specification. If the
specification is extended, this must be done carefully to preserve
backwards compatibility. And obviously if some implementation inserts
additional elements not described by the specification, interoperability
immediately goes out the window.
Zbyszek
--
_______________________________________________
devel mailing list -- devel@xxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxxxxxxxx
Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: https://lists.fedoraproject.org/archives/list/devel@xxxxxxxxxxxxxxxxxxxxxxx
Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue
