-
Notifications
You must be signed in to change notification settings - Fork 164
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
Remove zpool altroot
when creating datasets.
#1018
base: master
Are you sure you want to change the base?
Conversation
ping? |
src/share/poudriere/include/fs.sh
Outdated
[ $# -ne 1 ] && eargs remove_altroot mountpoint | ||
local mountpoint altroot | ||
mountpoint="$1" | ||
altroot="$([ -z "${NO_ZFS}" ] && zpool get -Ho value altroot "${ZPOOL}")" |
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.
Why the $()
for this? We don't need a subshell for the test; we may avoid a fork if NO_ZFS
is set.
We need to set a default value or unset altroot
too for NO_ZFS
.
altroot=
if [ -z "${NO_ZFS}" ]; then
altroot=$(zpool get -Ho value altroot "${ZPOOL}")
fi
Or
altroot=
[ -n "${NO_ZFS}" ] || altroot=$(zpool get -Ho value altroot "${ZPOOL}")
The test && test
will exit with set -e
, even if in a subshell. Using ||
won't exit.
# sh -c 'set -e; foo=$(false && true); echo here'; echo $?
1
# sh -c 'set -e; foo=$(false && true) || :; echo here'; echo $?
here
0
# sh -c 'set -e; foo=$(false || true); echo here'; echo $?
here
0
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.
Done. Thanks for the detailed example. My shell-fu is not very strong :)
|
969b9b1
to
cfa99e9
Compare
I came across this while trying to use poudriere on TrueNAS, which sets I've also changed the commit message, as it was wrong and maybe a bit convoluted. Let me know what you think. |
If `${ZPOOL}` has `altroot` set, `${ZROOTFS}` can only be mounted under `altroot`. This means `${BASEFS}` _must_ begin with `altroot`. In `poudriere.conf`, the setting would look something like: `BASEFS=<altroot>/rest/of/path`. However, `altroot` is always prepended to whatever path is set to `mountpoint` at the time of dataset creation. What happens is that `${ZROOTFS}` is actually mounted on `<altroot>${BASEFS}`, while `poudriere` thinks it's on `${BASEFS}`. To fix the problem, simply check if `altroot` is set and remove it from `mountpoint` when creating datasets. Their `mountpoint`s will then be prepended with `altroot`, which matches `poudriere`'s internal records.
cfa99e9
to
052090c
Compare
poudriere
breaks if the zpool it's given to use has thealtroot
property set, as its internal dataset mountpoints are different than the path they were actually mounted on.Specifically, if the zpool has
altroot
set, all mounted datasets will havealtroot
prepended to their mountpoint. When settingBASEFS
, the user would naturally take that into account and includealtroot
in the path. The setting would look something like:BASEFS=<altroot>/rest/of/path
. However, what happens is that theBASEFS
mountpoint is actually<altroot><altroot>/rest/of/path
, whilepoudriere
thinks it's<altroot>/rest/of/path
.To fix the problem, simply check if
altroot
is set and remove it from the mount path at the point of the dataset creation. The dataset will be prepended withaltroot
, which will match the pathpoudriere
knows about.