Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: feat(dracut): add option to disable automatic guessing of output file #2448

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

aafeijoo-suse
Copy link
Member

@aafeijoo-suse aafeijoo-suse commented Jul 26, 2023

This patch allows forcing the input of the file path for the generated initramfs image (disabling automatic guessing of the location) using a new configuration option force_output_file=yes.

Checklist

  • I have tested it locally
  • I have reviewed and updated any documentation if relevant
  • I am providing new code and test(s) for it

@aafeijoo-suse
Copy link
Member Author

CC @lnussel

@LaszloGombos
Copy link
Collaborator

As I understand this option makes outfile mandatory (by default it is optional).

@aafeijoo-suse I am sorry, but I do not understand the use case ? Is this a use case for a distro or for a user ?

@lnussel
Copy link

lnussel commented Jul 27, 2023

The use case is for a distro. In some setups it's undesirable that dracut writes a file to a location that may not be relevant. So shipping a preset that requires an explicitly specified location prevents admins or package scriptlets from calling dracut without arguments.

@LaszloGombos
Copy link
Collaborator

LaszloGombos commented Jul 27, 2023

The use case is for a distro.

Thanks @lnussel .

In some setups it's undesirable that dracut writes a file to a location that may not be relevant.

I support this use-case.

So shipping a preset that requires an explicitly specified location prevents admins or package scriptlets from calling dracut without arguments.

I'd like to explore if there are other options for a distro to signal dracut to do so. Introducing a new settings should be a relatively high bar (one could argue last resort) - especially for a feature that is not for end user (but for a distro).

I am not very familiar with the automatic guessing of output file logic.

Perhaps a distro could make sure that those automatic directories that dracut expect do not exists on the distro and dracut could improve the error message in those conditions when automatic guessing of output file logic fails to print out the dracut[F]: so it is required to specify the file path of the generated initramfs image. message.

Can we somehow key/trigger this based on distro filesystem and not from command line/config file ? I am not against changing dracut code, I am questioning the new user argument.

@lnussel
Copy link

lnussel commented Aug 15, 2023

The output file detection is here

dracut/dracut.sh

Line 1095 in 32f6f36

if ! [[ $outfile ]]; then

The directories involved do have to exist.

Is there actually need to add an actual command line parameter? A config file setting would suffice

@mwilck
Copy link
Contributor

mwilck commented Aug 15, 2023

I have to say I find the solution proposed in the PR sort of awkward. If we as distribution architects determine that dracut's guessing logic isn't good enough, we should either improve it to suit our needs, or pass a suitable --outfile option to dracut from the caller.

The distro-specific caller1 has a few advantages over dracut with respect to guessing. The distro architects know the various setups that their distribution supports, and can select the location accordingly. The number of different configuration scenarios is certainly smaller than what the generic dracut logic has to choose from.

Still, simply getting the logic right in upstream dracut seems to be the best option, unless we come to the conclusion that it's too hard to get right.

Footnotes

  1. In suse-module-tools, we used to call dracut with an explicit --outfile option until 4c9dce0 ("Make regenerate-initrd-posttrans compatible with Dracut's UEFI mode (unified kernel image)"). The rationale was that dracut would allegedly know better than distro where to put the image (because the image location depends on dracut options such as --uefi).

@aafeijoo-suse
Copy link
Member Author

Is there actually need to add an actual command line parameter? A config file setting would suffice

There is not much difference I'd say, usually a command line parameter has an implicit config file option.

I have to say I find the solution proposed in the PR sort of awkward. If we as distribution architects determine that dracut's guessing logic isn't good enough, we should either improve it to suit our needs, or pass a suitable --outfile option to dracut from the caller.

I wasn't too convinced about this PR either, it's more of an RFC that anything else.

Still, simply getting the logic right in upstream dracut seems to be the best option, unless we come to the conclusion that it's too hard to get right.

On the other hand, @lnussel use case is very downstream specific (https://github.com/openSUSE/sdbootutil/blob/main/ARCHITECTURE.md), and I'm not sure it can be upstreamed.

@aafeijoo-suse aafeijoo-suse changed the title feat(dracut): add option to disable automatic guessing of output file RFC: feat(dracut): add option to disable automatic guessing of output file Aug 16, 2023
@lnussel
Copy link

lnussel commented Sep 5, 2023

The point of the option is to make sure only a caller that knows where the initrd has to be placed can get dracut to produce an initrd. An admin who naively calls dracut would therefore not accidentally end up with one in a random location.
The guessing logic is convenient in a classic old school system for sure but in other setups not helpful. Doesn't make sense e.g. with UKIs either I guess.

@stale
Copy link

stale bot commented Oct 15, 2023

This issue is being marked as stale because it has not had any recent activity. It will be closed if no further activity occurs. If this is still an issue in the latest release of Dracut and you would like to keep it open please comment on this issue within the next 7 days. Thank you for your contributions.

@stale stale bot added the stale communication is stuck label Oct 15, 2023
This patch allows forcing the input of the file path for the generated initramfs
image (disabling automatic guessing of the location) using a new configuration
option `force_output_file=yes`.
@aafeijoo-suse aafeijoo-suse force-pushed the dracut-disable-outfile-guessing branch from 16c1f5d to 9ca4b52 Compare March 25, 2024 09:38
@stale stale bot removed the stale communication is stuck label Mar 25, 2024
@github-actions github-actions bot added the man label Mar 25, 2024
@aafeijoo-suse
Copy link
Member Author

Is there actually need to add an actual command line parameter? A config file setting would suffice

I agree, let's give it a 2nd try.

CC @tblume, maybe we're interested in applying this only downstream anyway.

Copy link
Collaborator

@tblume tblume left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@aafeijoo-suse
Copy link
Member Author

Still, simply getting the logic right in upstream dracut seems to be the best option, unless we come to the conclusion that it's too hard to get right.

On the other hand, @lnussel use case is very downstream specific (https://github.com/openSUSE/sdbootutil/blob/main/ARCHITECTURE.md), and I'm not sure it can be upstreamed.

@mwilck after re-reading again boo#1213648 and Ludwig's comments, I think this PR makes sense for us when sdbootutil is in charge (e.g. http://download.opensuse.org/tumbleweed/appliances/openSUSE-Tumbleweed-Minimal-VM.x86_64-kvm-and-xen-sdboot.qcow2). Could you share your updated opinion, or is it the same as before?

@aafeijoo-suse
Copy link
Member Author

@lnussel btw, you know that if this option is set and any rpm scriptlet calls dracut with --regenerate-all, it will fail.

@lnussel
Copy link

lnussel commented Mar 26, 2024

we'll find out then. If this happens anywhere right now it would silently generate initrds that are not used

@mwilck
Copy link
Contributor

mwilck commented Apr 5, 2024

@mwilck after re-reading again boo#1213648 and Ludwig's comments, I think this PR makes sense for us when sdbootutil is in charge

I was just expressing my opinion. I'm not in the position to block this PR from being merged.

However: The way I interpret @lnussel's statement above, on openSUSE with sdbootutil, dracut shouldn't be called directly at all any more, at least not by "naïve admins". This seems to be an (open)SUSE-specific situation which arises from the combination of sd-boot and the ability to boot from btrfs snapshots.

We (openSUSE) might as well install dracut in a non-standard location with /usr/bin/dracut just printing an error message, something like this:

dracut should not be invoked directly in this environment.
Please run <whatever sdbootutil incantation is needed> instead.
See <link to openSUSE wiki> for more information.

This would provide more clue to the admin that just uses the command she's been using for a decade than the generic message in this PR.

@aafeijoo-suse
Copy link
Member Author

@mwilck after re-reading again boo#1213648 and Ludwig's comments, I think this PR makes sense for us when sdbootutil is in charge

I was just expressing my opinion. I'm not in the position to block this PR from being merged.

Thanks Martin. Your opinion is always great, that's why I was asking again. And you made reconsider again what's the best workaround for this specific SUSE problem. A complete agreement between everyone involved here would also be great :)

At least I think that we all agree that we have a problem that needs to be fixed, so let's find the best fix.

However: The way I interpret @lnussel's statement above, on openSUSE with sdbootutil, dracut shouldn't be called directly at all any more, at least not by "naïve admins". This seems to be an (open)SUSE-specific situation which arises from the combination of sd-boot and the ability to boot from btrfs snapshots.

We (openSUSE) might as well install dracut in a non-standard location with /usr/bin/dracut just printing an error message, something like this:

dracut should not be invoked directly in this environment.
Please run <whatever sdbootutil incantation is needed> instead.
See <link to openSUSE wiki> for more information.

This would provide more clue to the admin that just uses the command she's been using for a decade than the generic message in this PR.

You made a good point, @lnussel what do you think of this approach? I think this would also require some adjustments on the installation packages which calls /usr/bin/dracut directly, because that would not work anymore, right? But adding the force_output_file=yes option would also invalidate all the --regerate-all calls... so there is no easy solution here.

@mwilck
Copy link
Contributor

mwilck commented Apr 8, 2024

You made a good point, @lnussel what do you think of this approach? I think this would also require some adjustments on the installation packages which calls /usr/bin/dracut directly, because that would not work anymore, right? But adding the force_output_file=yes option would also invalidate all the --regerate-all calls... so there is no easy solution here.

That's right. There are lots of dracut calls in particular in suse-module-tools, and we don't want to break them all. So perhaps (contrary to what I initially said) a command line option is actually the better idea here than a config file option. By checking the option, dracut would be able to detect that it's being called either by a "non-naïve" admin, or from a script that knows about the specifics of sdbootutil.

@mwilck
Copy link
Contributor

mwilck commented Apr 15, 2024

we'll find out then. If this happens anywhere right now it would silently generate initrds that are not used

I don't quite follow. --regenerate-all is called by every package that uses %regenerate_initrd_post. That is, by a lot of packages. How can we just quietly break this?

@lnussel
Copy link

lnussel commented Apr 16, 2024

we'll find out then. If this happens anywhere right now it would silently generate initrds that are not used

I don't quite follow. --regenerate-all is called by every package that uses %regenerate_initrd_post. That is, by a lot of packages. How can we just quietly break this?

We don't. That macro calls an external script if it exists. In the current state we'd ship the global dracut config in sdbootutil-rpm-scriptlets that has a conflict on suse-module-tools-scriptlets so no harm done. Going forward the goal would merge both packages again so suse-module-tools-scriptlets can deal with systemd-boot itself. So only one central place that is in our hands to fix.

@mwilck
Copy link
Contributor

mwilck commented Apr 16, 2024

How can we just quietly break this?

We don't. That macro calls an external script if it exists.

Right, sorry for confusing this.

I'm not happy about the fact that dracut --regenerate-all will be broken, but apparently there isn't much we can do about that, short of implementing the entire sdbootutil functionality inside dracut.

So yeah, I guess this is how we should move forward.

@aafeijoo-suse
Copy link
Member Author

I'm not happy about the fact that dracut --regenerate-all will be broken, but apparently there isn't much we can do about that, short of implementing the entire sdbootutil functionality inside dracut.

So yeah, I guess this is how we should move forward.

Settled then, I'll push this to Factory. Thank you all!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants