Re: PSA: tuned breaks boot loader entries for systemd-boot

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> > > > > > > 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.
>
The config files are generated, and technically 92-tuned.install also
generates them (from the template, the previously generated file),
that's how the streamed find & replace works. It could generate it
completely on its own, but why reinvent the wheel here. AFAIK it
wasn't against any guideline more than a decade ago when it was
implemented and IMHO it still isn't.

> > 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.
>
We used the tool, the grub and its features, not wrote the new bootloader.

> > > 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.
>
Again, we used the grub tool, and grub hadn't problem with it for more
than a decade and still hasn't, but I don't see any problem in
changing it to one initrd per one line.

> > 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?
>
That's not how standards work. It's behaviour undefined by the
standard, i.e. undefined behaviour. If it isn't forbidden, or strictly
defined, it's still standard compliant. But again, we are not
implementing a boot loader or writing standard, we are using existing
tool and its features.

On a more constructive note, we need to add/remove kernel args and
initrd overlays, because people, users, and customers want it. Telling
them: do not touch it, will not work. Providing them instructions on
how to do it... well it isn't rocket science, but it also isn't easy
and good luck supporting it. Editing/parsing/generating the BLS
entries: why reinvent the wheel here. Variables are a good compromise,
because you can just set/edit variables through some API/tool and you
don't need to touch the low level stuff. But we don't care about
variables, if there will be some simple API/interface that will work
across implementations we will happily switch to it and it would save
us lot of time and possible troubles

thanks & regards

Jaroslav

-- 
_______________________________________________
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




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Fedora Announce]     [Fedora Users]     [Fedora Kernel]     [Fedora Testing]     [Fedora Formulas]     [Fedora PHP Devel]     [Kernel Development]     [Fedora Legacy]     [Fedora Maintainers]     [Fedora Desktop]     [PAM]     [Red Hat Development]     [Gimp]     [Yosemite News]

  Powered by Linux