Refactor SYSV init and initramfs (Phase 2) #3559
Closed
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
0134c59 to
98af02b
Compare
39c67ce to
2e09031
Compare
1a998ad to
e0d40dc
Compare
14706d8 to
0eb468f
Compare
8e3167b to
1b46c21
Compare
|
I've split up the fixes, which can be considered regressions, into three different commits. At least the last two of those might be considered for the next release tag.. |
|
The "Some function improvenments to the SYSV init and initramfs scripts" commit might be better of as a separate pull request, but I leave it up to @behlendorf to decide that. It's in here for now.. This PR consists of three major parts, subdivided into separate commits:
and the dreaded and super-ugly last "shell check fixes"… I'm not even sure I want that any more, but I leave it up to "the powers that be" to decide it's fate. It's to ugly for me to look at now :) |
d13d852 to
9ebc6e7
Compare
|
@behlendorf Do we have an (approximate) ETA on this? |
|
I'd like to get more eyes on these changes. @ryao can you look these over. |
* Move the SYSV init scripts into the contrib directory. This isn't really part of the ZFS/ZoL code, so it's more appropriate to put it in the contrib directory. * Make sure the zfs-functions library file and the zfs 'defaults' file is in a separate directory (contrib/shell-common), to indicate it's significance for everything that needs generic functions in shell (sh) script code. Signed-off-by: Turbo Fredriksson <turbo@bayour.com>
…s (1/6).
* Move code from the SYSV init and initramfs scripts that is better served
in a common code base library (zfs-functions).
* Because import_pool() and get_pools() are now globaly availible functions,
replace the whole import section in the zfs-import with these functions.
* There is also no need to try with cache file (if one is availible) if
the first import fails. This is automatically taken care of by import_pool().
Signed-off-by: Turbo Fredriksson <turbo@bayour.com>
…s (2/6). * Dracut have the func emergency_shell() which does a little more. For those that don't have it, just run '/bin/sh -i -l'. Signed-off-by: Turbo Fredriksson <turbo@bayour.com>
…s (3/6). * zfs_set_ifs() needs to be a global func. Use this in zfs_run_cmd() before we run the actual command. Signed-off-by: Turbo Fredriksson <turbo@bayour.com>
…s (4/6). * Add some comments to functions that didn't have any. Signed-off-by: Turbo Fredriksson <turbo@bayour.com>
…s (5/6).
* Move disable_plymouth() from the initrd script to the generic zfs-functions.
* If plymouth doesn't exists (like if we're not running from an initrd),
then return false instead.
…s (6/6). * Make find_rootfs() and mount_fs() more generic. * Allow mount_fs() to accept mount point and mount options as arguments. * Create a get_pool_property() which mimics the get_fs_value() - retreive a value for a pool property. Signed-off-by: Turbo Fredriksson <turbo@bayour.com>
* Use check_boolean() where needed in more places.
* Make sure that kernel command line options which are booleans is
all accepting "yes|on|true|1" as option value.
* Put the check for zfs debugging in check_zfs_debug(). Use this in
zfs_action() to check if we should output the message we're called
with.
* Move code snippet that fetches and mounts recursive filesystems into
the new recursive_mount_filesystems() function in ZFS function script.
* Create zfs_run_cmd() which records stderr, exit code etc.
* Instead of using ZFS_CMD+zfs_action()+zfs_log_failure_msg() etc
each and every time, use the more generic zfs_action() that does
most of this for us.
* Downside: Won't accept pools with spaces in them any more :(.
Need to find a way to solve this (can't use arrays, because we're
not running under bash!).
* Upside: Get better control over 'quiet'/'debug' mode output.
* Many functions is and should be generic. That is, they shouldn't
output any information (that is and can be done elsewhere). Instead
they should return true or false depending on status of commands.
* Correct and fix the 'log STDERR to file if debugging'.
* Improve and change the way that get_pools() finds availible pools
for import. If there is a cache file, use (ONLY!) that when retreiving
pools to import. This makes it even easier to choose ONLY using
cache file, OR first try to probe for devices and if that fails,
possibly try using the cache file if one exists.
Signed-off-by: Turbo Fredriksson <turbo@bayour.com>
* Apparently "var > 0" doesn't work. Use 'var -gt 0' instead. But that means that 'var' needs to be set, so set this to '0' if it's not already set at the beginning of the initrd script. Signed-off-by: Turbo Fredriksson <turbo@bayour.com>
* Set and reset IFS in read_{mtab,fstab}() and recursive_mount_filesystems().
* Setup local ALL variables in read_fstab().
* Protect the variable set in read_{fstab,mtab}() in quotes.
Signed-off-by: Turbo Fredriksson <turbo@bayour.com>
* Fixing return variable from a bunch of assist functions. Returned false when they should have returned true! Thanx to markdesouza @ GitHub for help findind this. Closes: openzfs#3869 Signed-off-by: Turbo Fredriksson <turbo@bayour.com>
* var1="$var2".
* "$var" => "${var}"
Signed-off-by: Turbo Fredriksson <turbo@bayour.com>
9ebc6e7 to
89464c3
Compare
|
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. |
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.
In the discussion in FransUrbo@9815562#commitcomment-11919415, we talk about a three stage process. This PR is "Phase 2" and should preferably be applied as one of the first after the tag (because it rearranges code directories etc, which might be somewhat disruptive).