-
Notifications
You must be signed in to change notification settings - Fork 11
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
Add support for NFS root using unfs3 on server #10
base: nfs
Are you sure you want to change the base?
Conversation
server/novaboot-shell
Outdated
local OLD_IFS=$IFS | ||
IFS=" | ||
=" | ||
|
||
local nfs_tcp | ||
local mount_tcp | ||
|
||
while read -r service port | ||
do | ||
if [ "$service" = "NFS_TCP" ]; then | ||
nfs_tcp=$port | ||
elif [ "$service" = "MOUNT_TCP" ]; then | ||
mount_tcp=$port | ||
fi | ||
done < $port_file | ||
|
||
IFS=$OLD_IFS |
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.
can be replaced with . $port_file
, NFS_TCP
and MOUNT_TCP
will then be variables containing the port number.
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.
Sending few comments to the current state.
server/novaboot-shell
Outdated
@@ -114,6 +146,11 @@ run_console() { | |||
eval "$1" | |||
} | |||
|
|||
# Run UNFSd service if not running for NB_USER | |||
run_unfsd() { | |||
systemctl --user is-active novaboot-unfsd@${NB_USER:?}.service || systemctl --user start novaboot-unfsd@${NB_USER:?}.service |
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.
I think is-active
is not needed here. If the service is already started, start
is a no-op.
server/novaboot-shell
Outdated
get_nfsroot() { | ||
run_unfsd | ||
|
||
local nfsroot_path="$HOME/tftproot/$NB_USER/root" |
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.
Is there any reason, why the nfsroot is under tftproot? This means that all files in the filesystem will be accessible via tftp without any authentication.
If not, I'd prefer to use $HOME/nfsroot/$NB_USER
.
server/novaboot-shell
Outdated
@@ -89,6 +89,38 @@ read_config() { | |||
. "${NOVABOOT_SHELL_CONFIG:-$HOME/.novaboot-shell}" | |||
} | |||
|
|||
get_nfsroot() { | |||
run_unfsd |
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.
If run_unfsd
is not used elsewhere, just run systemctl start ...
here.
server/novaboot-shell
Outdated
|
||
# Use the first ip address returned by the `hostname -I` command | ||
# TODO: change to specific ip or use better method | ||
local nfsroot_ip=`hostname -I | cut -d' ' -f1` |
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.
Use $(...)
rather than `...`
.
And allow the user to override the address in the configuration file. This might be necessary due to NATs etc.
server/novaboot-shell
Outdated
# TODO: change to specific ip or use better method | ||
local nfsroot_ip=`hostname -I | cut -d' ' -f1` | ||
|
||
local port_file="$HOME/.unfsd/$NB_USER.ports" |
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.
Use $HOME/.cache/unfsd/...
server/novaboot-shell
Outdated
mkdir -p "$HOME/tftproot" | ||
cd "$HOME/tftproot" | ||
mkdir -p "$HOME/tftproot/${NB_USER:?}" | ||
cd "$HOME/tftproot/${NB_USER:?}" |
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.
I'm afraid that this will break my existing setups.
server/systemd/novaboot-unfsd-start
Outdated
@@ -0,0 +1,25 @@ | |||
#!/bin/sh |
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.
If possible, I'd prefer not having this file at all and do all things in the .service
file.
server/systemd/novaboot-unfsd-start
Outdated
|
||
# get network from config file | ||
. "${NOVABOOT_SHELL_CONFIG:-$HOME/.novaboot-shell}" | ||
NETWORK=${nfs_network:?} |
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.
If this script is implemented in the .service
, this would have to be configured differently. Perhaps via Environment=
in the .service
file. User will be able to change the value via systemctl --user edit
.
Or it could be configured via EnvironmentFile=
and the same file would be used as ConditionPathExists=
. This would be nice because no service would be started automatically before the administrator configures it.
server/systemd/novaboot-unfsd-start
Outdated
NETWORK=${nfs_network:?} | ||
|
||
# Generate exports file | ||
cat > $EXPORTS_FILE <<EOF |
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.
In .service
, this would be ExecStartPre=sh -c "echo xxx > .../%i.exports"
server/novaboot-shell
Outdated
@@ -188,7 +225,7 @@ main() { | |||
# Commands allowed at any time | |||
"") locked $0 default;; | |||
"console") locked $0 console;; | |||
"get-config") read_config && echo -n "${target_config}"; exit;; | |||
"get-config") read_config && get_nfsroot && echo "--nfsroot=$NB_NFSROOT" && echo -n "${target_config}"; exit;; |
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.
Move this code to a new function get_config
.
If the administrator disables the unfsd service somehow, the --nfsroot
options should not be sent to the client at all.
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.
In addition to the comments below, add some short text to the documentation (novaboot-shell.pod
).
server/novaboot-shell
Outdated
|
||
if is_nfs_enabled; then | ||
get_nfsroot | ||
echo "--nfsroot=$NB_NFSROOT" |
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.
echo "--nfsroot=$NB_NFSROOT" | |
echo "--nfsroot=$(get_nfsroot)" |
server/novaboot-shell
Outdated
|
||
local port_file="$HOME/.cache/unfsd/$NB_USER.ports" | ||
|
||
local nfs_tcp |
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.
Maybe it's not necessary to declare it here, it can be defined local
in the case
, cannot it?
server/novaboot-shell
Outdated
# TODO: change to specific ip or use better method | ||
local nfsroot_ip=${nfs_ip:-$(hostname -I | cut -d' ' -f1)} | ||
|
||
local port_file="$HOME/.cache/unfsd/$NB_USER.ports" |
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.
Rename to unfsd_port_file
.
server/novaboot-shell
Outdated
esac | ||
done < $port_file | ||
|
||
NB_NFSROOT="$nfsroot_ip:$nfsroot_path,v3,tcp,port=${nfs_tcp:?},mountport=${mount_tcp:?}" |
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.
echo ...
server/novaboot-shell
Outdated
fi | ||
mkdir -p "$HOME/nfsroot/${NB_USER:?}" | ||
cd "$HOME/nfsroot/${NB_USER:?}" | ||
exec "$@";; |
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.
Chceck that rsync
cannot overwrite files in subdirectories of $HOME/nfsroot/${NB_USER:?}
.
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.
No, it cannot.
server/systemd/[email protected]
Outdated
# Start unfsd | ||
# -p: do not register with portmapper | ||
# -u: use random ports | ||
ExecStart=sh -c 'unfsd -p -u -P $NB_UNFSD_PORT_FILE -e $NB_UNFSD_EXPORT_FILE' |
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.
Don't use sh
, use variable substitution supported by systemd.
Can you retarget the PR against the |
15c554e
to
fdbfccb
Compare
I've integrated the latest commits to the nfs branch. I still plan to work on the "untar" commit (at least add tests). I've added support for |
Starts the unfsd for user on get-config request and stops it on last user logout.
Yet to be done: