-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
initramfs fixes #9089
initramfs fixes #9089
Conversation
0f4bfc5
to
61e8d9a
Compare
Codecov Report
@@ Coverage Diff @@
## master #9089 +/- ##
=========================================
- Coverage 79.04% 78.94% -0.1%
=========================================
Files 401 400 -1
Lines 121648 121640 -8
=========================================
- Hits 96154 96029 -125
- Misses 25494 25611 +117
Continue to review full report at Codecov.
|
1051193
to
2213a72
Compare
This would fix #7904 #8796 is another PR that aims to do multiple initramfs fixes, including this. Either we approve this and remove the zfs-functions from that PR or we just get the other one done. Also, before a PR is committed, ZOL wants commits squashed to a single commit before being accepted. If you need assistance with this, let us know. Thanks for looking at this. This has been a nuisance building deb packages from zfs master. |
@ghfields my changes include some stuff from #8796, while I left out the changes to the initramfs script which are not essential for the fix but are an improvement of code quality - these should go to another PR IMHO.
Thanks, I know that but generally keep PRs as multiple, logically separated commits, to make review easier. Shortly before merging I can squash them |
Sorry can I just quickly ask here: did you change any code? Or its all the same existing PR / commits from earlier? Because I already tested that one last week. And it was absolutely fine for me. It doesnt seem like you added any further commits. I tested ontop of the |
huh?! what about those? https://github.com/zfsonlinux/zfs/pull/9089/commits Edit: I don't think I really got what you were asking (may be my fault). Do you mean this PR by "earlier"? Do you mean #8796? |
Ah sorry @c0d3z3r0 I was the one who had got confused with the other "[RFC] fixes" PR. Which was actually the one i had already tested earlier on. Not this one. Good good... seems like a progress. Getting closer. Yeah I can rebuild pkgs and compare the contents with a |
Aaah, at least we both were confused :D
Well, my PR even does something more than #8796:
The boot fix basically consists of adding the initconf file (/etc/default/zfs) and /etc/zfs-functions to the package. |
1aaca6b
to
c36eade
Compare
Fixed "file included twice" warning from rpmbuild, squashed everything and rebased onto master |
Codecov Report
@@ Coverage Diff @@
## master #9089 +/- ##
==========================================
+ Coverage 79.18% 79.65% +0.47%
==========================================
Files 400 279 -121
Lines 121692 80856 -40836
==========================================
- Hits 96360 64408 -31952
+ Misses 25332 16448 -8884
Continue to review full report at Codecov.
|
* contrib/initramfs: include /etc/default/zfs and /etc/zfs/zfs-functions At least debian needs /etc/default/zfs and /etc/zfs/zfs-functions for its initramfs. Include both in build when initramfs is configured. * contrib/initramfs: include 60-zvol.rules and zvol_id Include 60-zvol.rules and zvol_id and set udev as predependency instead of debians zdev. This makes debians additional zdev hook unneeded. * Correct initconfdir substitution for some distros Not every Linux distro is using @sysconfdir@/default but @initconfdir@ which is already determined by configure. Let's use it. * systemd: prevent possible conflict between systemd and sysvinit Systemd will not load a sysvinit service if a unit exists with the same name. This prevents conflicts between sysvinit and systemd. In ZFS there is one sysvinit service that does not have a systemd service but a target counterpart, zfs-import.target. Usually it does not make any sense to install both but it is possisble. Let's prevent any conflict by masking zfs-import.service by default. This does not harm even if init.d/zfs-import does not exist. Signed-off-by: Michael Niewöhner <foss@mniewoehner.de>
c36eade
to
2bafa28
Compare
Hey there @c0d3z3r0 I tried applying your latest code onto Then all of the compilation steps of zfs executed OK for me. It spat out debs. However when reinstalling zfs, I then got the following error msg: installing zfs_0.8.1-1_amd64.deb ...
Reading package lists... Done
Building dependency tree
Reading state information... Done
Note, selecting 'zfs' instead of './zfs_0.8.1-1_amd64.deb'
The following packages will be DOWNGRADED:
zfs
0 to upgrade, 0 to newly install, 1 to downgrade, 0 to remove and 19 not to upgrade.
Need to get 0 B/1,194 kB of archives.
After this operation, 18.4 kB of additional disk space will be used.
Get:1 /home/id/.dev/zfs/zfs_0.8.1-1_amd64.deb zfs amd64 0.8.1-1 [1,194 kB]
dpkg: warning: downgrading zfs from 0.8.1-6 to 0.8.1-1
(Reading database ... 265906 files and directories currently installed.)
Preparing to unpack .../.dev/zfs/zfs_0.8.1-1_amd64.deb ...
Unpacking zfs (0.8.1-1) over (0.8.1-6) ...
dpkg: error processing archive /home/id/.dev/zfs/zfs_0.8.1-1_amd64.deb (--unpack):
trying to overwrite '/etc/default/zfs', which is also in package zfs-initramfs 0.8.1-6
dpkg-deb: error: paste subprocess was killed by signal (Broken pipe)
Errors were encountered while processing:
/home/id/.dev/zfs/zfs_0.8.1-1_amd64.deb
E: Sub-process /usr/bin/dpkg returned an error code (1)
id@asrock-z170-ocf:~/.dev/zfs$ Not sure what to do now. Personally I don't want to move up to master if there is a risk that it will include other kinds of breaking changes. My generally approach is to use the last stable tagged release as per in the zfs releases page. Please advise further. If there is anything else you want to see / need from me here. |
I guess I did some mistake in the spec which includes /etc/default/zfs in zfs-initramfs*.deb and zfs*.deb. I'll fix that later and you know |
The spec seems correct. According to the log you only installed |
Same error msg. Then the 2nd time i re-run the cmd it's gone. So maybe that is because the version its overwriting was the other previous fix? id@asrock-z170-ocf:~/.dev/zfs$ sudo dpkg -i zfs_0.8.1-1_amd64.deb zfs-initramfs_0.8.1-1_amd64.deb
dpkg: warning: downgrading zfs from 0.8.1-6 to 0.8.1-1
(Reading database ... 265906 files and directories currently installed.)
Preparing to unpack zfs_0.8.1-1_amd64.deb ...
Unpacking zfs (0.8.1-1) over (0.8.1-6) ...
dpkg: error processing archive zfs_0.8.1-1_amd64.deb (--install):
trying to overwrite '/etc/default/zfs', which is also in package zfs-initramfs 0.8.1-6
dpkg-deb: error: paste subprocess was killed by signal (Broken pipe)
dpkg: warning: downgrading zfs-initramfs from 0.8.1-6 to 0.8.1-1
Preparing to unpack zfs-initramfs_0.8.1-1_amd64.deb ...
Unpacking zfs-initramfs (0.8.1-1) over (0.8.1-6) ...
Setting up zfs-initramfs (0.8.1-1) ...
Errors were encountered while processing:
zfs_0.8.1-1_amd64.deb
id@asrock-z170-ocf:~/.dev/zfs$ here is the output of 2nd rerun of same cmd $ sudo dpkg -i zfs_0.8.1-1_amd64.deb zfs-initramfs_0.8.1-1_amd64.deb
(Reading database ... 265908 files and directories currently installed.)
Preparing to unpack zfs_0.8.1-1_amd64.deb ...
Unpacking zfs (0.8.1-1) over (0.8.1-1) ...
Preparing to unpack zfs-initramfs_0.8.1-1_amd64.deb ...
Unpacking zfs-initramfs (0.8.1-1) over (0.8.1-1) ...
Setting up zfs (0.8.1-1) ...
Setting up zfs-initramfs (0.8.1-1) ...
Processing triggers for man-db (2.8.5-2) ...
id@asrock-z170-ocf:~/.dev/zfs$ IDK maybe I should go back and install a completely clean version of |
Yep, good idea. You can check the contents (and post them here, please) with |
It seems that the whole business of me installing zfs on ubuntu here is a bit complicated. So you will have to bear with me on that due to my lack of experience in this area. Getting other errors now involving dkms etc. So whilst I try to tackle that, and get back to reproducing the original boot error / problem. Here is the requested https://gist.github.com/dreamcat4/0e1b8762a130853485c55a6272aaece0 |
OK, got back to the kernel panic now. This reproduced the original problem. Next I will apply the version of your patch which I tweaked slightly. But only to get the patch to apply cleanly, nothing else. As had already explained in one of my earlier comments today #9089 (comment) Then I will rebuild, reinstall, reboot. See what happens. Hopefully there won't be any other unrelated errors anymore to get in our way. |
o.O dpkg complains about replacing
Your patch looks good! |
OK @c0d3z3r0 just finished testing your PR. The failed case was reproduced, as was already described above. Then I built with your codem and reinstalled installed it. In my log it says the same version of zfs is being installed over the top (0.8.1). Which is to be expected since I didn't bother to increment the version number or change anything else except for your patch. Regenerated initramfs & updated grub. Here is the full build log of that run: https://gist.github.com/dreamcat4/8919af50ff5d187f6c44aa98110b8254 Then I shutdown the computer, and pulled out my backup boot disk from the system. To make sure that other one could not possibly be being booted accidentally. So then only the main boot disk which had the new initramfs on it. Rebooted and it came back up. No kernel panic. I think this concludes my testing now. Kind Regards. |
Great!
Could you please do the dpkc -c thing once again, just to be sure? :-)
… Am 08.08.2019 um 23:19 schrieb Dreamcat4 ***@***.***>:
OK @c0d3z3r0 just finished testing your PR. The failed case was reproduced, as was already described above. Then I built with your codem and reinstalled installed it. In my log it says the same version of zfs is being installed over the top (0.8.1). Which is to be expected since I didn't bother to increment the version number or change anything else except for your patch.
Regenerated initramfs & updated grub.
Here is the full build log of that run:
https://gist.github.com/dreamcat4/8919af50ff5d187f6c44aa98110b8254
Then I shutdown the computer, and pulled out my backup boot disk from the system. To make sure that other one could not possibly be being booted accidentally. So then only the main boot disk which had the new initramfs on it.
Rebooted and it came back up. No kernel panic. I think this concludes my testing now. Kind Regards.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
@c0d3z3r0 Here is 2nd https://gist.github.com/dreamcat4/7572fdc600a4d17c1ad603a81d9963fb |
Both files are included now, perfect!
… Am 09.08.2019 um 05:31 schrieb Dreamcat4 ***@***.***>:
@c0d3z3r0 Here is 2nd dpkg -c. Many thanks for doing this PR. Anything else please let us know.
https://gist.github.com/dreamcat4/7572fdc600a4d17c1ad603a81d9963fb
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
The fix has been verified successfully by @dreamcat4 and @ReimuHakurei. The failing tests are unrelated. This should be ready for final review and merging now. @behlendorf |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for sorting this out!
Signed-off-by: Paul Dagnelie <pcd@delphix.com>
Signed-off-by: Paul Dagnelie <pcd@delphix.com>
* contrib/initramfs: include /etc/default/zfs and /etc/zfs/zfs-functions At least debian needs /etc/default/zfs and /etc/zfs/zfs-functions for its initramfs. Include both in build when initramfs is configured. * contrib/initramfs: include 60-zvol.rules and zvol_id Include 60-zvol.rules and zvol_id and set udev as predependency instead of debians zdev. This makes debians additional zdev hook unneeded. * Correct initconfdir substitution for some distros Not every Linux distro is using @sysconfdir@/default but @initconfdir@ which is already determined by configure. Let's use it. * systemd: prevent possible conflict between systemd and sysvinit Systemd will not load a sysvinit service if a unit exists with the same name. This prevents conflicts between sysvinit and systemd. In ZFS there is one sysvinit service that does not have a systemd service but a target counterpart, zfs-import.target. Usually it does not make any sense to install both but it is possisble. Let's prevent any conflict by masking zfs-import.service by default. This does not harm even if init.d/zfs-import does not exist. Reviewed-by: Chris Wedgwood <cw@f00f.org> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Tested-by: Alex Ingram <reimu@reimuhakurei.net> Tested-by: Dreamcat4 <dreamcat4@gmail.com> Signed-off-by: Michael Niewöhner <foss@mniewoehner.de> Closes openzfs#7904 Closes openzfs#9089
Signed-off-by: Paul Dagnelie <pcd@delphix.com>
* contrib/initramfs: include /etc/default/zfs and /etc/zfs/zfs-functions At least debian needs /etc/default/zfs and /etc/zfs/zfs-functions for its initramfs. Include both in build when initramfs is configured. * contrib/initramfs: include 60-zvol.rules and zvol_id Include 60-zvol.rules and zvol_id and set udev as predependency instead of debians zdev. This makes debians additional zdev hook unneeded. * Correct initconfdir substitution for some distros Not every Linux distro is using @sysconfdir@/default but @initconfdir@ which is already determined by configure. Let's use it. * systemd: prevent possible conflict between systemd and sysvinit Systemd will not load a sysvinit service if a unit exists with the same name. This prevents conflicts between sysvinit and systemd. In ZFS there is one sysvinit service that does not have a systemd service but a target counterpart, zfs-import.target. Usually it does not make any sense to install both but it is possisble. Let's prevent any conflict by masking zfs-import.service by default. This does not harm even if init.d/zfs-import does not exist. Reviewed-by: Chris Wedgwood <cw@f00f.org> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Tested-by: Alex Ingram <reimu@reimuhakurei.net> Tested-by: Dreamcat4 <dreamcat4@gmail.com> Signed-off-by: Michael Niewöhner <foss@mniewoehner.de> Closes openzfs#7904 Closes openzfs#9089
Signed-off-by: Paul Dagnelie <pcd@delphix.com>
* contrib/initramfs: include /etc/default/zfs and /etc/zfs/zfs-functions At least debian needs /etc/default/zfs and /etc/zfs/zfs-functions for its initramfs. Include both in build when initramfs is configured. * contrib/initramfs: include 60-zvol.rules and zvol_id Include 60-zvol.rules and zvol_id and set udev as predependency instead of debians zdev. This makes debians additional zdev hook unneeded. * Correct initconfdir substitution for some distros Not every Linux distro is using @sysconfdir@/default but @initconfdir@ which is already determined by configure. Let's use it. * systemd: prevent possible conflict between systemd and sysvinit Systemd will not load a sysvinit service if a unit exists with the same name. This prevents conflicts between sysvinit and systemd. In ZFS there is one sysvinit service that does not have a systemd service but a target counterpart, zfs-import.target. Usually it does not make any sense to install both but it is possisble. Let's prevent any conflict by masking zfs-import.service by default. This does not harm even if init.d/zfs-import does not exist. Reviewed-by: Chris Wedgwood <cw@f00f.org> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Tested-by: Alex Ingram <reimu@reimuhakurei.net> Tested-by: Dreamcat4 <dreamcat4@gmail.com> Signed-off-by: Michael Niewöhner <foss@mniewoehner.de> Closes #7904 Closes #9089
Signed-off-by: Paul Dagnelie <pcd@delphix.com>
Motivation and Context
Fixes #7904
Debian does not boot from zfs when using packages generated from the git repo because /etc/default/zfs and /etc/zfs/zfs-functions are missing, which both are used by the zfs initramfs script.
Description
How Has This Been Tested?
Types of changes
Checklist:
Signed-off-by
.