Avoid here-documents in systemd mount generator #9802
Conversation
|
@aerusso would you mind commenting on this. |
Codecov Report
@@ Coverage Diff @@
## master #9802 +/- ##
========================================
- Coverage 79% 79% -<1%
========================================
Files 385 385
Lines 121492 121492
========================================
- Hits 96511 96376 -135
- Misses 24981 25116 +135
Continue to review full report at Codecov.
|
|
Well here's something I didn't know about - bash uses temp files for heredocs?! While the echo solution looks okayish to me, my limited sense of shell script aesthetics tells me to write it 'at once' instead of appending multiple echoes to the file repeatedly, so maybe something like this? ( echo "abc"
...
echo "last line") > "${dest_norm}/${keyloadunit}"saves us a couple syscalls at least. Alternatively we could could escape the newlines to merge it into one long multi-line echo. |
|
Wow, I bumped into this in my first version of this, but somehow wasn't able to reproduce the issue. We can avoid the multiple calls with something like printf "# Automatically generated by zfs-mount-generator\n"\
"\n"\
"[Unit]\n"\
... > ... |
|
I guess one could also do |
I think especially when planning to support more than just Linux, Also, IMHO this whole change might be worth documenting inline to avoid future more elegant refactors using here-docs. 😉 edit: Just realized this is symstemd-specific anyways. So portability is likely not really an issue here. Sorry for the noise. |
This commit makes the following changes: - the systemd mount generator now generates units for datasets marked canmount=noauto too. Unlike canmount=on, these units are NOT WantedBy local-fs.target. - introduces handling of the user property org.openzfs.systemd:noauto, which is now included in the zfs-list.cache files and will cause a canmount=on dataset to be treated like noauto by the generator. - replaces the heredocs with redirected echo statements to avoid problems with creating tempfiles for them. (resolves openzfs#9802) - suppresses shellcheck warnings for an intentionally unquoted variable and some confusion that arises from the nested quoting for the keyload commands. Signed-off-by: InsanePrawn <[email protected]>
|
@lhuedepohl I'd suggest we go with either the |
|
Portability is only important in the sense that /bin/sh is not always bash. So as long as it works in a POSIX she’ll, we should be good. We may need to rewrite this in C anyway, but fixing this now is still valuable. |
|
Rewrote as requested with single echo and multi-line strings. I ran the |
behlendorf
added
Status: Accepted
On some systems - openSUSE, for example - there is not yet a writeable temporary file system available, so bash bails out with an error, 'cannot create temp file for here-document: Read-only file system', on the here documents in zfs-mount-generator. The simple fix is to change these into a multi-line echo statement. Signed-off-by: Lorenz Hüdepohl <[email protected]>
On some systems - openSUSE, for example - there is not yet a writeable temporary file system available, so bash bails out with an error, 'cannot create temp file for here-document: Read-only file system', on the here documents in zfs-mount-generator. The simple fix is to change these into a multi-line echo statement. Reviewed-by: Brian Behlendorf <[email protected]> Reviewed-By: Richard Laager <[email protected]> Reviewed-by: George Melikov <[email protected]> Signed-off-by: Lorenz Hüdepohl <[email protected]> Closes openzfs#9802
On some systems - openSUSE, for example - there is not yet a writeable temporary file system available, so bash bails out with an error, 'cannot create temp file for here-document: Read-only file system', on the here documents in zfs-mount-generator. The simple fix is to change these into a multi-line echo statement. Reviewed-by: Brian Behlendorf <[email protected]> Reviewed-By: Richard Laager <[email protected]> Reviewed-by: George Melikov <[email protected]> Signed-off-by: Lorenz Hüdepohl <[email protected]> Closes openzfs#9802
On some systems - openSUSE, for example - there is not yet a writeable temporary file system available, so bash bails out with an error, 'cannot create temp file for here-document: Read-only file system', on the here documents in zfs-mount-generator. The simple fix is to change these into a multi-line echo statement. Reviewed-by: Brian Behlendorf <[email protected]> Reviewed-By: Richard Laager <[email protected]> Reviewed-by: George Melikov <[email protected]> Signed-off-by: Lorenz Hüdepohl <[email protected]> Closes #9802
Motivation and Context
The systemd zfs-mount-generator script fails on my openSUSE systems, as there is not yet a writeable temporary file system available at the time it runs. Thus bash bails out with an error,
'cannot create temp file for here-document: Read-only file system',
on the here documents in zfs-mount-generator.
Description
The simple fix is to change these into many individual echo statements.
How Has This Been Tested?
I use it myself since a few weeks on several machines. The (backported) patch is also active on my project on the openSUSE Build Service where I build zfs packages for openSUSE and which is used by several other people.
Types of changes
Checklist:
Signed-off-by.