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

Refactor SYSV init, initramfs and dracut (Phase 3) #3560

Closed
wants to merge 15 commits into from

Conversation

FransUrbo
Copy link
Contributor

  • Move code from the dracut scripts that is better served in a common code base library (zfs-functions).
  • Add functionality that exists in initramfs to dracut.
    • Support 'zfs:' import and mounting. This was for some reason removed in eda3d4e.
    • Support booting from snapshots.
    • Support mounting recursive filesystems.
    • Support mounting of additional filesystems.
    • Support mounting nativly encrypted filesystems.

This depend on #3559, this PR is "Phase 3". It seriously modifies the dracut code (tested extensively on Fedora 17).

Because #3559 have not been accepted yet, this PR includes those commits as well. But once that PR have been accepted (eventually?), I do a new push and they should vanish.

Even though the intention is to get dracut accepted in dracut upstream, there is no reason why not to do this. The zfs-functions could either be pushed there "as is" or code used could be extracted. Considering the added functionality, which would be next to impossible (and extremely difficult to maintain), giving ALL systems the exact same boot parameters and functionality is, in my opinion, extremely important for a project as ZoL.

Therefor, this should then be tagged 0.6.6 as earliest, but I'm good with 0.6.7. I DO think it needs to be done eventually though.

@FransUrbo FransUrbo force-pushed the initramfs-p3 branch 6 times, most recently from 42b0156 to 5aa94f1 Compare July 9, 2015 12:37
@behlendorf behlendorf added this to the 0.7.0 milestone Jul 10, 2015
@FransUrbo FransUrbo force-pushed the initramfs-p3 branch 9 times, most recently from 8ec6331 to 6172672 Compare July 28, 2015 03:45
@FransUrbo FransUrbo force-pushed the initramfs-p3 branch 12 times, most recently from ac4d557 to eec3394 Compare August 2, 2015 02:13
* Support 'zfs:<rootfs>' import and mounting.
  This was for some reason removed in eda3d4e.
* Support booting from snapshots.
* Support mounting recursive filesystems.
* Support mounting of additional filesystems.
* Support mounting nativly encrypted filesystems.

Signed-off-by: Turbo Fredriksson <[email protected]>
@FransUrbo
Copy link
Contributor Author

@Vaelatern With your fix (which I've just merged and force-pushed), does it work as expected?

Do you want to add your Signed-off-by?

@Vaelatern
Copy link
Contributor

@FransUrbo The patch now looks right. Yes, it works on one of my machines. I'm happy to add my Signed-off-by. I assume you rebase and append it to the end of the commits?

@FransUrbo
Copy link
Contributor Author

No, I merged it to the first commit (Refactoring of common code between the SYSV init, initramfs and dracut scripts.). It made no sense having it in it's separate commit.

@Vaelatern
Copy link
Contributor

I was referring to my Signed-off-by, I'm not sure if I add it or if you.

@FransUrbo
Copy link
Contributor Author

Ah, that one. No, that have not been added yet. It's in here, if @behlendorf ever feels like this might be important or relevant to look at.

@ryao
Copy link
Contributor

ryao commented Feb 6, 2016

Why is this in so many different commits?

@prometheanfire
Copy link
Contributor

someone mind sharing their kernel command line? root=zfs:AUTO didn't work.

@prometheanfire
Copy link
Contributor

here's the output, it also didn't try to import the pool https://gist.github.com/prometheanfire/5dc33fcc55579bc6fd56

@Vaelatern
Copy link
Contributor

@prometheanfire, is "AUTO" your pool name?

@prometheanfire
Copy link
Contributor

nope, I'll try changing that then :D

@prometheanfire
Copy link
Contributor

and it didn't work with zfs:mthode-work (name of the pool)

the drive is decrypted then the mount is called and fails. within the recovery shell I can import the pool and all still.

@prometheanfire
Copy link
Contributor

it seems to me that the mount command doesn't understand zfs:foo or something...

@Vaelatern
Copy link
Contributor

The feature is specifically Support 'zfs:<rootfs>' import and mounting so try that?

@prometheanfire
Copy link
Contributor

zfs:mthode-work/OS did not work

@prometheanfire
Copy link
Contributor

here's a portion of the rdsoreport.txt

kernel cmdline

[    2.911556] localhost dracut-cmdline[2299]: Using kernel command line parameters: ro initrd=\initramfs-4.1.17-gentoo.img rd.luks.uuid=eeec1057-7445-4510-a737-da8fc0085a6c rd.luks.allow-discards=eeec1057-7445-4510-a737-da8fc0085a6c rd.luks.crypttab=0 root=zfs:mthode-work/OS zfs.zfs_arc_max=3221225472 init=/usr/lib/systemd/systemd ro


[   13.006331] localhost systemd[1]: Mounting /sysroot...
[   13.006919] localhost mount[3806]: mount: wrong fs type, bad option, bad superblock on zfs:mthode-work/OS,
[   13.007006] localhost mount[3806]: missing codepage or helper program, or other error
[   13.007088] localhost mount[3806]: (for several filesystems (e.g. nfs, cifs) you might
[   13.007161] localhost mount[3806]: need a /sbin/mount.<type> helper program)
[   13.007234] localhost mount[3806]: In some cases useful info is found in syslog - try
[   13.007317] localhost mount[3806]: dmesg | tail or so.

and the systemctl status

● sysroot.mount - /sysroot
   Loaded: loaded (/proc/cmdline; bad; vendor preset: enabled)
   Active: active (mounted) (Result: exit-code) since Sun 2016-02-07 05:20:36 UTC; 24s ago
    Where: /sysroot
     What: /dev/sda1
     Docs: man:fstab(5)
           man:systemd-fstab-generator(8)
  Process: 3806 ExecMount=/bin/mount zfs:mthode-work/OS /sysroot -o ro (code=exited, status=32)

@prometheanfire
Copy link
Contributor

seems to be trying mount zfs:mthode-work/OS instead of mount -t zfs, not sure if that's intentional.

@prometheanfire
Copy link
Contributor

tried manual mounting in the initram with mount -t zfs, failed because couldn't find the dataset, imported dataset with -N and tried again, failed because dataset was not marked as legacymount, which I don't think I should change.

@Vaelatern
Copy link
Contributor

@prometheanfire, are you sure you are using zfs with the above patches actually applied? At no point should it be trying to mount "zfs:rpool" but rather importing "rpool"

@prometheanfire
Copy link
Contributor

I'm sure...

here is /usr/lib64/dracut/modules.d/90zfs/mount-zfs.sh https://bpaste.net/show/e3067d32ae37

@FransUrbo
Copy link
Contributor Author

@ryao it includes the commits from "Phase 2" as well. Only the last three is relevant to the Dracut changes.

@prometheanfire root=zfs:AUTO is suppose to work! But on the other hand, I've never tested this using systemd!!

@prometheanfire
Copy link
Contributor

@FransUrbo if you have time on I'm on the freenode channel as prometheanfire, we can debug if you want.

@Vaelatern
Copy link
Contributor

Where is the status of this? Were the systemd issues worked out yet?
I have two systems that only boot with versions of zfsonlinux that include this code.

@Vaelatern
Copy link
Contributor

According to the other thread, the systemd issues are elsewhere, and not related to this patch, #3605.

Is this likely to be merged or shall I adopt the last three commits, rewrite them to not require any of the preceding commits, and PR that (with appropiate credit given)?


AC_MSG_CHECKING([whether dracut is available])
if test -d /usr/lib/dracut ; then
DEFINE_DRACUT='--define "_dracut 1"'
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. Is /usr/share/dracut no longer supported?
  2. Is the check to be performed by the host building the package? Doesn't this break building an F23 package in an F22 host, or building the package in a host that has no Dracut? Or are we all already deep into the Mock ideology that this is a lost cause?

Just wondering.

@FransUrbo
Copy link
Contributor Author

If (when!?) this actually attracts some actual interest to be merged, I can spend time on it. But not a second until then!

I've been tricked before.

@behlendorf
Copy link
Contributor

There are some nice improvements in these proposed patches. Unfortunately, they never got the attention they deserved in terms of reviewers and testing. As such I was never comfortable with merging them to master. If someone would like to continue with this work my suggestion would be to start with the most important changes and open new PRs against master. Keeping the PRs small and as functionally independent as possible will make it easier to review and test the changes. Closing.

@behlendorf behlendorf closed this Jan 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants