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 #6897
Closed
Closed
Initramfs fixes #6897
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
For filesystems that are children of the rootfs, when mountpoint=none or mountpoint=legacy, the initrafms script would assume a mountpoint based on the dataset path. Given that the rootfs should have mountpoint=/ and mountpoint inheritance is is the default behavior of ZFS, this behavior seems unnecessary. In any event, it turns mountpoint=none into a no-op. That removes this option from the administrator, and if someone uses it, it does not work as expected. Worse yet, if the mountpoint directory does not exist (which is the typical case for mountpoint=none), the mounting and thus the boot process will fail. For the case of mountpoint=legacy, the assumed mountpoint may not be the correct value set in /etc/fstab. This change makes the initramfs script not mount the filesystem in either case. For mountpoint=none, this means we are correctly honoring the setting. For mountpoint=legacy, there are two scenarios: If canmount=on, the filesystem will be mounted by the normal mechanisms later in the boot process. If canmount=noauto, the filesystem will not be mounted at all, unless the administrator has done something special. If they're not doing something special and they want it mounted by the initramfs, they can simply not set mountpoint=legacy. This is part of the fix for: zfsonlinux/pkg-zfs#221 Signed-off-by: Richard Laager <rlaager@wiktel.com>
The initramfs script was not honoring canmount=off. With this change, it does. If the administrator has asked that a filesystem not be mounted, that should be honored. As an exception, the initramfs script ignores canmount=off on the rootfs. The rootfs should not have canmount=off set either. However, mounting it anyway seems harmless because it is being asked for explicitly. The point of this exception is to avoid the risk of breaking existing systems, just in case someone has canmount=off set on their rootfs. The initramfs still mounts filesystems with canmount=noauto. This is necessary because it is typical to set that on the rootfs so that it can be cloned. Without canmount=noauto, the clones' duplicate mountpoints would conflict. This is the remainder of the fix for: zfsonlinux/pkg-zfs#221 Signed-off-by: Richard Laager <rlaager@wiktel.com>
Codecov Report
@@ Coverage Diff @@
## master #6897 +/- ##
==========================================
+ Coverage 75.1% 75.23% +0.13%
==========================================
Files 298 298
Lines 95270 95270
==========================================
+ Hits 71555 71680 +125
+ Misses 23715 23590 -125
Continue to review full report at Codecov.
|
behlendorf
approved these changes
Nov 28, 2017
gmelikov
approved these changes
Nov 28, 2017
behlendorf
pushed a commit
that referenced
this pull request
Nov 28, 2017
The initramfs script was not honoring canmount=off. With this change, it does. If the administrator has asked that a filesystem not be mounted, that should be honored. As an exception, the initramfs script ignores canmount=off on the rootfs. The rootfs should not have canmount=off set either. However, mounting it anyway seems harmless because it is being asked for explicitly. The point of this exception is to avoid the risk of breaking existing systems, just in case someone has canmount=off set on their rootfs. The initramfs still mounts filesystems with canmount=noauto. This is necessary because it is typical to set that on the rootfs so that it can be cloned. Without canmount=noauto, the clones' duplicate mountpoints would conflict. This is the remainder of the fix for: zfsonlinux/pkg-zfs#221 Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: George Melikov <mail@gmelikov.ru> Signed-off-by: Richard Laager <rlaager@wiktel.com> Closes #6897
behlendorf
pushed a commit
to tonyhutter/zfs
that referenced
this pull request
Nov 28, 2017
For filesystems that are children of the rootfs, when mountpoint=none or mountpoint=legacy, the initrafms script would assume a mountpoint based on the dataset path. Given that the rootfs should have mountpoint=/ and mountpoint inheritance is is the default behavior of ZFS, this behavior seems unnecessary. In any event, it turns mountpoint=none into a no-op. That removes this option from the administrator, and if someone uses it, it does not work as expected. Worse yet, if the mountpoint directory does not exist (which is the typical case for mountpoint=none), the mounting and thus the boot process will fail. For the case of mountpoint=legacy, the assumed mountpoint may not be the correct value set in /etc/fstab. This change makes the initramfs script not mount the filesystem in either case. For mountpoint=none, this means we are correctly honoring the setting. For mountpoint=legacy, there are two scenarios: If canmount=on, the filesystem will be mounted by the normal mechanisms later in the boot process. If canmount=noauto, the filesystem will not be mounted at all, unless the administrator has done something special. If they're not doing something special and they want it mounted by the initramfs, they can simply not set mountpoint=legacy. This is part of the fix for: zfsonlinux/pkg-zfs#221 Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: George Melikov <mail@gmelikov.ru> Signed-off-by: Richard Laager <rlaager@wiktel.com> Closes openzfs#6897
behlendorf
pushed a commit
to tonyhutter/zfs
that referenced
this pull request
Nov 28, 2017
The initramfs script was not honoring canmount=off. With this change, it does. If the administrator has asked that a filesystem not be mounted, that should be honored. As an exception, the initramfs script ignores canmount=off on the rootfs. The rootfs should not have canmount=off set either. However, mounting it anyway seems harmless because it is being asked for explicitly. The point of this exception is to avoid the risk of breaking existing systems, just in case someone has canmount=off set on their rootfs. The initramfs still mounts filesystems with canmount=noauto. This is necessary because it is typical to set that on the rootfs so that it can be cloned. Without canmount=noauto, the clones' duplicate mountpoints would conflict. This is the remainder of the fix for: zfsonlinux/pkg-zfs#221 Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: George Melikov <mail@gmelikov.ru> Signed-off-by: Richard Laager <rlaager@wiktel.com> Closes openzfs#6897
behlendorf
pushed a commit
to tonyhutter/zfs
that referenced
this pull request
Dec 1, 2017
For filesystems that are children of the rootfs, when mountpoint=none or mountpoint=legacy, the initrafms script would assume a mountpoint based on the dataset path. Given that the rootfs should have mountpoint=/ and mountpoint inheritance is is the default behavior of ZFS, this behavior seems unnecessary. In any event, it turns mountpoint=none into a no-op. That removes this option from the administrator, and if someone uses it, it does not work as expected. Worse yet, if the mountpoint directory does not exist (which is the typical case for mountpoint=none), the mounting and thus the boot process will fail. For the case of mountpoint=legacy, the assumed mountpoint may not be the correct value set in /etc/fstab. This change makes the initramfs script not mount the filesystem in either case. For mountpoint=none, this means we are correctly honoring the setting. For mountpoint=legacy, there are two scenarios: If canmount=on, the filesystem will be mounted by the normal mechanisms later in the boot process. If canmount=noauto, the filesystem will not be mounted at all, unless the administrator has done something special. If they're not doing something special and they want it mounted by the initramfs, they can simply not set mountpoint=legacy. This is part of the fix for: zfsonlinux/pkg-zfs#221 Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: George Melikov <mail@gmelikov.ru> Signed-off-by: Richard Laager <rlaager@wiktel.com> Closes openzfs#6897
behlendorf
pushed a commit
to tonyhutter/zfs
that referenced
this pull request
Dec 1, 2017
The initramfs script was not honoring canmount=off. With this change, it does. If the administrator has asked that a filesystem not be mounted, that should be honored. As an exception, the initramfs script ignores canmount=off on the rootfs. The rootfs should not have canmount=off set either. However, mounting it anyway seems harmless because it is being asked for explicitly. The point of this exception is to avoid the risk of breaking existing systems, just in case someone has canmount=off set on their rootfs. The initramfs still mounts filesystems with canmount=noauto. This is necessary because it is typical to set that on the rootfs so that it can be cloned. Without canmount=noauto, the clones' duplicate mountpoints would conflict. This is the remainder of the fix for: zfsonlinux/pkg-zfs#221 Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: George Melikov <mail@gmelikov.ru> Signed-off-by: Richard Laager <rlaager@wiktel.com> Closes openzfs#6897
tonyhutter
pushed a commit
that referenced
this pull request
Dec 5, 2017
For filesystems that are children of the rootfs, when mountpoint=none or mountpoint=legacy, the initrafms script would assume a mountpoint based on the dataset path. Given that the rootfs should have mountpoint=/ and mountpoint inheritance is is the default behavior of ZFS, this behavior seems unnecessary. In any event, it turns mountpoint=none into a no-op. That removes this option from the administrator, and if someone uses it, it does not work as expected. Worse yet, if the mountpoint directory does not exist (which is the typical case for mountpoint=none), the mounting and thus the boot process will fail. For the case of mountpoint=legacy, the assumed mountpoint may not be the correct value set in /etc/fstab. This change makes the initramfs script not mount the filesystem in either case. For mountpoint=none, this means we are correctly honoring the setting. For mountpoint=legacy, there are two scenarios: If canmount=on, the filesystem will be mounted by the normal mechanisms later in the boot process. If canmount=noauto, the filesystem will not be mounted at all, unless the administrator has done something special. If they're not doing something special and they want it mounted by the initramfs, they can simply not set mountpoint=legacy. This is part of the fix for: zfsonlinux/pkg-zfs#221 Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: George Melikov <mail@gmelikov.ru> Signed-off-by: Richard Laager <rlaager@wiktel.com> Closes #6897
tonyhutter
pushed a commit
that referenced
this pull request
Dec 5, 2017
The initramfs script was not honoring canmount=off. With this change, it does. If the administrator has asked that a filesystem not be mounted, that should be honored. As an exception, the initramfs script ignores canmount=off on the rootfs. The rootfs should not have canmount=off set either. However, mounting it anyway seems harmless because it is being asked for explicitly. The point of this exception is to avoid the risk of breaking existing systems, just in case someone has canmount=off set on their rootfs. The initramfs still mounts filesystems with canmount=noauto. This is necessary because it is typical to set that on the rootfs so that it can be cloned. Without canmount=noauto, the clones' duplicate mountpoints would conflict. This is the remainder of the fix for: zfsonlinux/pkg-zfs#221 Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: George Melikov <mail@gmelikov.ru> Signed-off-by: Richard Laager <rlaager@wiktel.com> Closes #6897
tonyhutter
pushed a commit
to tonyhutter/zfs
that referenced
this pull request
Dec 5, 2017
For filesystems that are children of the rootfs, when mountpoint=none or mountpoint=legacy, the initrafms script would assume a mountpoint based on the dataset path. Given that the rootfs should have mountpoint=/ and mountpoint inheritance is is the default behavior of ZFS, this behavior seems unnecessary. In any event, it turns mountpoint=none into a no-op. That removes this option from the administrator, and if someone uses it, it does not work as expected. Worse yet, if the mountpoint directory does not exist (which is the typical case for mountpoint=none), the mounting and thus the boot process will fail. For the case of mountpoint=legacy, the assumed mountpoint may not be the correct value set in /etc/fstab. This change makes the initramfs script not mount the filesystem in either case. For mountpoint=none, this means we are correctly honoring the setting. For mountpoint=legacy, there are two scenarios: If canmount=on, the filesystem will be mounted by the normal mechanisms later in the boot process. If canmount=noauto, the filesystem will not be mounted at all, unless the administrator has done something special. If they're not doing something special and they want it mounted by the initramfs, they can simply not set mountpoint=legacy. This is part of the fix for: zfsonlinux/pkg-zfs#221 Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: George Melikov <mail@gmelikov.ru> Signed-off-by: Richard Laager <rlaager@wiktel.com> Closes openzfs#6897
tonyhutter
pushed a commit
to tonyhutter/zfs
that referenced
this pull request
Dec 5, 2017
The initramfs script was not honoring canmount=off. With this change, it does. If the administrator has asked that a filesystem not be mounted, that should be honored. As an exception, the initramfs script ignores canmount=off on the rootfs. The rootfs should not have canmount=off set either. However, mounting it anyway seems harmless because it is being asked for explicitly. The point of this exception is to avoid the risk of breaking existing systems, just in case someone has canmount=off set on their rootfs. The initramfs still mounts filesystems with canmount=noauto. This is necessary because it is typical to set that on the rootfs so that it can be cloned. Without canmount=noauto, the clones' duplicate mountpoints would conflict. This is the remainder of the fix for: zfsonlinux/pkg-zfs#221 Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: George Melikov <mail@gmelikov.ru> Signed-off-by: Richard Laager <rlaager@wiktel.com> Closes openzfs#6897
Nasf-Fan
pushed a commit
to Nasf-Fan/zfs
that referenced
this pull request
Jan 29, 2018
For filesystems that are children of the rootfs, when mountpoint=none or mountpoint=legacy, the initrafms script would assume a mountpoint based on the dataset path. Given that the rootfs should have mountpoint=/ and mountpoint inheritance is is the default behavior of ZFS, this behavior seems unnecessary. In any event, it turns mountpoint=none into a no-op. That removes this option from the administrator, and if someone uses it, it does not work as expected. Worse yet, if the mountpoint directory does not exist (which is the typical case for mountpoint=none), the mounting and thus the boot process will fail. For the case of mountpoint=legacy, the assumed mountpoint may not be the correct value set in /etc/fstab. This change makes the initramfs script not mount the filesystem in either case. For mountpoint=none, this means we are correctly honoring the setting. For mountpoint=legacy, there are two scenarios: If canmount=on, the filesystem will be mounted by the normal mechanisms later in the boot process. If canmount=noauto, the filesystem will not be mounted at all, unless the administrator has done something special. If they're not doing something special and they want it mounted by the initramfs, they can simply not set mountpoint=legacy. This is part of the fix for: zfsonlinux/pkg-zfs#221 Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: George Melikov <mail@gmelikov.ru> Signed-off-by: Richard Laager <rlaager@wiktel.com> Closes openzfs#6897
Nasf-Fan
pushed a commit
to Nasf-Fan/zfs
that referenced
this pull request
Jan 29, 2018
The initramfs script was not honoring canmount=off. With this change, it does. If the administrator has asked that a filesystem not be mounted, that should be honored. As an exception, the initramfs script ignores canmount=off on the rootfs. The rootfs should not have canmount=off set either. However, mounting it anyway seems harmless because it is being asked for explicitly. The point of this exception is to avoid the risk of breaking existing systems, just in case someone has canmount=off set on their rootfs. The initramfs still mounts filesystems with canmount=noauto. This is necessary because it is typical to set that on the rootfs so that it can be cloned. Without canmount=noauto, the clones' duplicate mountpoints would conflict. This is the remainder of the fix for: zfsonlinux/pkg-zfs#221 Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: George Melikov <mail@gmelikov.ru> Signed-off-by: Richard Laager <rlaager@wiktel.com> Closes openzfs#6897
Nasf-Fan
pushed a commit
to Nasf-Fan/zfs
that referenced
this pull request
Feb 13, 2018
For filesystems that are children of the rootfs, when mountpoint=none or mountpoint=legacy, the initrafms script would assume a mountpoint based on the dataset path. Given that the rootfs should have mountpoint=/ and mountpoint inheritance is is the default behavior of ZFS, this behavior seems unnecessary. In any event, it turns mountpoint=none into a no-op. That removes this option from the administrator, and if someone uses it, it does not work as expected. Worse yet, if the mountpoint directory does not exist (which is the typical case for mountpoint=none), the mounting and thus the boot process will fail. For the case of mountpoint=legacy, the assumed mountpoint may not be the correct value set in /etc/fstab. This change makes the initramfs script not mount the filesystem in either case. For mountpoint=none, this means we are correctly honoring the setting. For mountpoint=legacy, there are two scenarios: If canmount=on, the filesystem will be mounted by the normal mechanisms later in the boot process. If canmount=noauto, the filesystem will not be mounted at all, unless the administrator has done something special. If they're not doing something special and they want it mounted by the initramfs, they can simply not set mountpoint=legacy. This is part of the fix for: zfsonlinux/pkg-zfs#221 Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: George Melikov <mail@gmelikov.ru> Signed-off-by: Richard Laager <rlaager@wiktel.com> Closes openzfs#6897
Nasf-Fan
pushed a commit
to Nasf-Fan/zfs
that referenced
this pull request
Feb 13, 2018
The initramfs script was not honoring canmount=off. With this change, it does. If the administrator has asked that a filesystem not be mounted, that should be honored. As an exception, the initramfs script ignores canmount=off on the rootfs. The rootfs should not have canmount=off set either. However, mounting it anyway seems harmless because it is being asked for explicitly. The point of this exception is to avoid the risk of breaking existing systems, just in case someone has canmount=off set on their rootfs. The initramfs still mounts filesystems with canmount=noauto. This is necessary because it is typical to set that on the rootfs so that it can be cloned. Without canmount=noauto, the clones' duplicate mountpoints would conflict. This is the remainder of the fix for: zfsonlinux/pkg-zfs#221 Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: George Melikov <mail@gmelikov.ru> Signed-off-by: Richard Laager <rlaager@wiktel.com> Closes openzfs#6897
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
mountpoint
For filesystems that are children of the rootfs, when mountpoint=none or mountpoint=legacy, the initrafms script would assume a mountpoint based on the dataset path. Given that the rootfs should have mountpoint=/ and mountpoint inheritance is is the default behavior of ZFS, this behavior seems unnecessary. In any event, it turns mountpoint=none into a no-op. That removes this option from the administrator, and if someone uses it, it does not work as expected. Worse yet, if the mountpoint directory does not exist (which is the typical case for mountpoint=none), the mounting and thus the boot process will fail. For the case of mountpoint=legacy, the assumed mountpoint may not be the correct value set in /etc/fstab.
This change makes the initramfs script not mount the filesystem in either case. For mountpoint=none, this means we are correctly honoring the setting. For mountpoint=legacy, there are two scenarios: If canmount=on, the filesystem will be mounted by the normal mechanisms later in the boot process. If canmount=noauto, the filesystem will not be mounted at all, unless the administrator has done something special. If they're not doing something special and they want it mounted by the initramfs, they can simply not set mountpoint=legacy.
canmount
The initramfs script was not honoring canmount=off. With this change, it does. If the administrator has asked that a filesystem not be mounted, that should be honored.
As an exception, the initramfs script ignores canmount=off on the rootfs. The rootfs should not have canmount=off set either. However, mounting it anyway seems harmless because it is being asked for explicitly. The point of this exception is to avoid the risk of breaking existing systems, just in case someone has canmount=off set on their rootfs.
The initramfs still mounts filesystems with canmount=noauto. This is necessary because it is typical to set that on the rootfs so that it can be cloned. Without canmount=noauto, the clones' duplicate mountpoints would conflict.
Motivation and Context
These changes fix:
zfsonlinux/pkg-zfs#221
How Has This Been Tested?
I tested on Ubuntu 17.10 (artful).
Before the change, mounting and thus booting fails on test2 and test5. Creating the directories fixes the boot. All 5 are mounted. After the change, even without creating the directories, the boot succeeds. Only 1, 3, and 4 are mounted.
The mountpoint=legacy tests:
Before the change, test6 fails to mount (for lack of a directory), and test7 is mounted. I added some logging to the initramfs, and test7 was mounted by the initramfs. After the change, there was no failure, test6 was not mounted (as expected), and test7 was mounted. The same logging indicated that test7 was not mounted by the initramfs, meaning it was mounted later in the boot process (as expected).
Types of changes
I have marked this as a breaking change because it does change the existing behavior. I think I have minimized the potential for breakage as much as possible while fixing the underlying issue.
Checklist:
Signed-off-by.