dracutdevs / dracut

dracut the event driven initramfs infrastructure
https://github.com/dracutdevs/dracut/wiki
GNU General Public License v2.0
598 stars 397 forks source link

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

Open aafeijoo-suse opened 1 year ago

aafeijoo-suse commented 1 year ago

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

aafeijoo-suse commented 1 year ago

CC @lnussel

LaszloGombos commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

The output file detection is here https://github.com/dracutdevs/dracut/blob/32f6f364ddeb706bf8741f2895d60022aee264e7/dracut.sh#L1095

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 commented 1 year ago

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 caller[^1] 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.

[^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 commented 1 year ago

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.

lnussel commented 1 year ago

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[bot] commented 11 months ago

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.

aafeijoo-suse commented 5 months ago

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.

aafeijoo-suse commented 5 months ago

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 commented 5 months ago

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

lnussel commented 5 months ago

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

mwilck commented 5 months ago

@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 commented 5 months ago

@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 commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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!