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

Fix dracut. Document it. #13291

Closed
wants to merge 12 commits into from

Conversation

nabijaczleweli
Copy link
Contributor

@nabijaczleweli nabijaczleweli commented Apr 4, 2022

Motivation and Context

@behlendorf: as promised in #13017 (comment)

Description

Uh, well. In order to write documentation I had to actually test whether the magic half of dracut was depending on, well, worked. Turns out it didn't. Which, well, means it didn't. Which explains why it's consistently kinda nondescriptly ass and we keep (kept?) getting people whose basic setups were failing. It's honestly a miracle and an incredible accident this'd worked at all.

Also, the non-systemd path (zfs-mount.sh) needs either removing or fixing; I haven't the slightest how you'd adjust network key loading. I don't even know if it works because I don't have a system or even distribution I could test it with. Well, I'm not building Gentoo I guess?

How Has This Been Tested?

Thoroughly. See commit messages. As far as the new manual goes, here's the dracut.bootup(7)-style flowchart:
image
image

And PDF: job_1958-stdin___jaczleweli_PDF.pdf (I don't usually target postscript in ZFS manuals, but, well, this looks shockingly alright in landscape)

Closes #13291

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist:

  • My code follows the OpenZFS code style requirements.
  • I have updated the documentation accordingly.
  • I have read the contributing document.
  • I have added tests to cover my changes. – none apply
  • I have run the ZFS Test Suite with this change applied. – CI take my hand
  • All commit messages are properly formatted and contain Signed-off-by.

The switch was released in dracut 009 in March 2011,
we can safely get rid of the compatibility hook

Signed-off-by: Ahelenia Ziemiańska <[email protected]>
It was added in the original ae26d04 ("Add dracut support") commit
in 2011, and was then broken a bit later with the advent of
dracut-zfs-generator, or maybe earlier as part of other churn

Either way, it's broken, and has been in 2.0+ as well, and no-one
complained. Stop pretending we support it at all

Signed-off-by: Ahelenia Ziemiańska <[email protected]>
So far, everything parsed root= manually, which meant that while
zfs-parse.sh was updated, and supposedly supported + -> ' ' conversion,
it meant nothing

Instead, centralise parsing, and allow:
  root=
  root=zfs
  root=zfs:
  root=zfs:AUTO

  root=ZFS=data/set
  root=zfs:data/set
  root=zfs:ZFS=data/set (as a side-effect; allowed but undocumented)

  rootfstype=zfs AND root=data/set <=> root=data/set
  rootfstype=zfs AND root=         <=> root=zfs:AUTO

So rootfstype=zfs /also/ behaves as expected, and + decoding works

Signed-off-by: Ahelenia Ziemiańska <[email protected]>
The only user is mount-zfs.sh (non-systemd systems),
so reduce it to what it needs

Signed-off-by: Ahelenia Ziemiańska <[email protected]>
…or_password

Also don't set ROOTFS_MOUNTED; the final mention was removed in dracut
011 from July 2011

Signed-off-by: Ahelenia Ziemiańska <[email protected]>
This fixes at least one race I got with an encrypted root

Signed-off-by: Ahelenia Ziemiańska <[email protected]>
@behlendorf behlendorf self-requested a review April 5, 2022 00:12
@behlendorf behlendorf added Type: Documentation Indicates a requested change to the documentation Component: Dracut dracut integration labels Apr 5, 2022
@behlendorf
Copy link
Contributor

Thanks! The flow chart is great to see. Let's see if we can get some additional eyes on this. cc: @Rudd-O @c0d3z3r0 @prometheanfire

@prometheanfire
Copy link
Contributor

I love this, where was this 2-3 years ago lol

Thorough documentation with a dracut.bootup(7)-style flowchart,
dracut.cmdline(7)-style cmdline listing,
and per-file docs like the old README

Signed-off-by: Ahelenia Ziemiańska <[email protected]>
It was originally released in dracut 008 in February 2011;
we can probably drop it now

Signed-off-by: Ahelenia Ziemiańska <[email protected]>
@nabijaczleweli
Copy link
Contributor Author

1.5-week bump

Copy link
Contributor

@behlendorf behlendorf left a comment

Choose a reason for hiding this comment

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

Going over the commits everything here looks reasonable. I haven't manually tested it myself, but this is a nice step forward and the documentation is much appreciated. I'm going to go ahead and integrate it, if any unexpected issues arise we can investigate them as they're reported.

@behlendorf behlendorf added the Status: Accepted Ready to integrate (reviewed, tested) label Apr 20, 2022
behlendorf pushed a commit that referenced this pull request Apr 20, 2022
It was added in the original ae26d04 ("Add dracut support") commit
in 2011, and was then broken a bit later with the advent of
dracut-zfs-generator, or maybe earlier as part of other churn

Either way, it's broken, and has been in 2.0+ as well, and no-one
complained. Stop pretending we support it at all

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes #13291
behlendorf pushed a commit that referenced this pull request Apr 20, 2022
So far, everything parsed root= manually, which meant that while
zfs-parse.sh was updated, and supposedly supported + -> ' ' conversion,
it meant nothing

Instead, centralise parsing, and allow:
  root=
  root=zfs
  root=zfs:
  root=zfs:AUTO

  root=ZFS=data/set
  root=zfs:data/set
  root=zfs:ZFS=data/set (as a side-effect; allowed but undocumented)

  rootfstype=zfs AND root=data/set <=> root=data/set
  rootfstype=zfs AND root=         <=> root=zfs:AUTO

So rootfstype=zfs /also/ behaves as expected, and + decoding works

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes #13291
behlendorf pushed a commit that referenced this pull request Apr 20, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes #13291
behlendorf pushed a commit that referenced this pull request Apr 20, 2022
The only user is mount-zfs.sh (non-systemd systems),
so reduce it to what it needs

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes #13291
behlendorf pushed a commit that referenced this pull request Apr 20, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes #13291
behlendorf pushed a commit that referenced this pull request Apr 20, 2022
…or_password

Also don't set ROOTFS_MOUNTED; the final mention was removed in dracut
011 from July 2011

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes #13291
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
This fixes at least one race I got with an encrypted root

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#13291
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#13291
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
Thorough documentation with a dracut.bootup(7)-style flowchart,
dracut.cmdline(7)-style cmdline listing,
and per-file docs like the old README

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#13291
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
It was originally released in dracut 008 in February 2011;
we can probably drop it now

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#13291
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
The switch was released in dracut 009 in March 2011,
we can safely get rid of the compatibility hook

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#13291
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
It was added in the original ae26d04 ("Add dracut support") commit
in 2011, and was then broken a bit later with the advent of
dracut-zfs-generator, or maybe earlier as part of other churn

Either way, it's broken, and has been in 2.0+ as well, and no-one
complained. Stop pretending we support it at all

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#13291
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
So far, everything parsed root= manually, which meant that while
zfs-parse.sh was updated, and supposedly supported + -> ' ' conversion,
it meant nothing

Instead, centralise parsing, and allow:
  root=
  root=zfs
  root=zfs:
  root=zfs:AUTO

  root=ZFS=data/set
  root=zfs:data/set
  root=zfs:ZFS=data/set (as a side-effect; allowed but undocumented)

  rootfstype=zfs AND root=data/set <=> root=data/set
  rootfstype=zfs AND root=         <=> root=zfs:AUTO

So rootfstype=zfs /also/ behaves as expected, and + decoding works

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#13291
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#13291
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
The only user is mount-zfs.sh (non-systemd systems),
so reduce it to what it needs

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#13291
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#13291
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
…or_password

Also don't set ROOTFS_MOUNTED; the final mention was removed in dracut
011 from July 2011

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#13291
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
This fixes at least one race I got with an encrypted root

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#13291
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#13291
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
Thorough documentation with a dracut.bootup(7)-style flowchart,
dracut.cmdline(7)-style cmdline listing,
and per-file docs like the old README

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#13291
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
It was originally released in dracut 008 in February 2011;
we can probably drop it now

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#13291
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
The switch was released in dracut 009 in March 2011,
we can safely get rid of the compatibility hook

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#13291
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
It was added in the original ae26d04 ("Add dracut support") commit
in 2011, and was then broken a bit later with the advent of
dracut-zfs-generator, or maybe earlier as part of other churn

Either way, it's broken, and has been in 2.0+ as well, and no-one
complained. Stop pretending we support it at all

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#13291
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
So far, everything parsed root= manually, which meant that while
zfs-parse.sh was updated, and supposedly supported + -> ' ' conversion,
it meant nothing

Instead, centralise parsing, and allow:
  root=
  root=zfs
  root=zfs:
  root=zfs:AUTO

  root=ZFS=data/set
  root=zfs:data/set
  root=zfs:ZFS=data/set (as a side-effect; allowed but undocumented)

  rootfstype=zfs AND root=data/set <=> root=data/set
  rootfstype=zfs AND root=         <=> root=zfs:AUTO

So rootfstype=zfs /also/ behaves as expected, and + decoding works

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#13291
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#13291
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
The only user is mount-zfs.sh (non-systemd systems),
so reduce it to what it needs

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#13291
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#13291
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
…or_password

Also don't set ROOTFS_MOUNTED; the final mention was removed in dracut
011 from July 2011

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#13291
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
This fixes at least one race I got with an encrypted root

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#13291
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#13291
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
Thorough documentation with a dracut.bootup(7)-style flowchart,
dracut.cmdline(7)-style cmdline listing,
and per-file docs like the old README

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#13291
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
It was originally released in dracut 008 in February 2011;
we can probably drop it now

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#13291
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Dracut dracut integration Status: Accepted Ready to integrate (reviewed, tested) Type: Documentation Indicates a requested change to the documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants