V2 of stratis-fstab-setup to order setup before fsck #3437
Conversation
There was a problem hiding this comment.
I've tested v2 with some minor changes to fix errors I saw running the script. Overall I think the fstab syntax is much better with v2 and I think this is preferable to putting the pool UUID into the options string but I still hit boot failures with the updated stratis-fstab-setup.
I resolved these by adding a Before=local-fs-pre.target to the [email protected] unit.
| fi | ||
|
|
||
|
|
||
| if [ "$1" != dev-stratis* ]; then |
There was a problem hiding this comment.
This expression isn't working for me in testing. I had to change it to:
if [[ "$1" != dev-stratis* ]]; then| exit 1 | ||
| fi | ||
|
|
||
| PATH="$1" |
There was a problem hiding this comment.
Needs to be something like DEVPATH to avoid overwriting PATH - otherwise we'll get command not found for the binaries the script runs (stratis-min and sleep).
| exit 1 | ||
| fi | ||
|
|
||
| POOL_NAME="${$SEGMENTS[2]}" |
There was a problem hiding this comment.
To evaluate index 2 of the array this wants to be:
POOL_NAME="${SEGMENTS[2]}"
|
|
||
| IFS='-' read -ra SEGMENTS <<< "$PATH" | ||
|
|
||
| if [ "${#SEGMENTS[@]}" != 4 ]; then |
There was a problem hiding this comment.
This won't work if there are embedded '-' characters in the pool or filesystem name (e.g. p1/fs1-snap). Systemd escapes '-' as \x2d in unit names but by the time the argument is passed to the stratis-fstab-setup script the escaping has been expanded to the literal '-' character.
|
@bmr-cymru I'm considering closing this out and moving the |
|
@jbaublitz yes that seems reasonable to me. I will re-test with a clean set up and the changes from #3436 today. Generally I think the changes you proposed here are an improvement in terms of the fstab syntax if we could get the proper escaping and decoding implemented but we can re-visit that in the future - the change to |
|
@bmr-cymru I agree that it'd be an improvement. See #3438 for hopefully a better version of the pool name syntax that doesn't require the path or filesystem name. |
Related to #3436