On Tue, Nov 05, 2024 at 07:32:58PM +0100, Jaroslav Škarvada 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.
> >
> 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.
Let's agree to disagree.
> > > 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.
I think we'll have to agree to disagree. If it's "undefined behaviour"
than it's not "standard compliant".
> But again, we are not implementing a boot loader or writing
> standard, we are using existing tool and its features.
Well, no, that's just part of the situation. You are relying on the
quirks of a specific tool that has a known-buggy implementation of a
generic idea. Relying on those quirks was arguably never a good idea.
But what we're discussing in this thread is the fact that _your_
implementation breaks boot and this is what needs to be fixed.
> 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
Sure, being able to change boot config is a real problem. If there
is an idea how to extend the standard to support new features,
please propose this and let's discuss the options. Any extension
may be accepted or rejected. But the approach of assuming that you
can just inject additional incomatible syntax downstream because
one specific tool supports this is wrong.
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
