-
Notifications
You must be signed in to change notification settings - Fork 169
Remove zpool altroot when creating datasets.
#1018
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
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
poudrierebreaks if the zpool it's given to use has thealtrootproperty set, as its internal dataset mountpoints are different than the path they were actually mounted on.Specifically, if the zpool has
altrootset, all mounted datasets will havealtrootprepended to their mountpoint. When settingBASEFS, the user would naturally take that into account and includealtrootin the path. The setting would look something like:BASEFS=<altroot>/rest/of/path. However, what happens is that theBASEFSmountpoint is actually<altroot><altroot>/rest/of/path, whilepoudrierethinks it's<altroot>/rest/of/path.To fix the problem, simply check if
altrootis 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 pathpoudriereknows about.