-
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 rsync-nfsroot
and unfsd
commands
#6
base: master
Are you sure you want to change the base?
Conversation
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.
See few comments below. Also don't forget to add the new commands to the help message and to the documentation. It's OK if the documentation is written only after all is settled up and working.
@@ -124,6 +126,22 @@ run_subcommand() { | |||
mkdir -p "$HOME/tftproot" | |||
cd "$HOME/tftproot" | |||
exec "$@";; | |||
"rsync-nfsroot "*" --server "*" . .") | |||
shift 1 | |||
mkdir -p "$HOME/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.
I think we want $HOME/nfsroot/$NB_USER
here.
rootdir="$2" | ||
shift 2 | ||
|
||
[ -n "$board_ip" ] || die 'Missing configuration option $board_ip' |
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 it required to be an IP address or hostnames could be supported as well? If possible, I'd prefer that both IP addresses and hostnames can be specified here. Most programs can handle both.
And to be consistent with the rest of the file, I'd prefer checking variables with ${board_ip:?}
, when the variable is first used (the line below). The error message will be generated automatically by the shell.
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.
The unfs3 manual says hostnames are supported as well so it should work.
echo "$HOME/nfsroot/$rootdir $board_ip(rw,no_root_squash)" > "$HOME/export" | ||
|
||
# start or restart unfs3 to reread updated `exports` file | ||
[ -n "$nfs_port" ] || die 'Missing configuration option $nfs_port' |
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.
${nfs_port:?}
|
||
# start or restart unfs3 to reread updated `exports` file | ||
[ -n "$nfs_port" ] || die 'Missing configuration option $nfs_port' | ||
systemctl --user restart "unfsd@$nfs_port.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.
The @
unit names are needed when multiple instances of one service need to execute at the same time. Here, at most one unfsd server will be executed. Maybe, we can distinguish different novaboot users unfsd@$NB_USER
, but even this is not necessary.
I think that the unfsd
subcommand could just create a symlink (e.g. $HOME/nfsroot-link
) pointing to $HOME/nfsroot/$NB_USER
and and the unfsd.service
and even the export
file could just refer to this path.
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 using it for the port variable, what would another option would be to replace the command with a script which sources the .novaboot-shell
config 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.
You can use EnvironmentFile=...
- see systemd.exec(5). Some (randomly found) example is at https://coreos.com/os/docs/latest/using-environment-variables-in-systemd-units.html#environmentfile-directive
ExecStart=unfsd -e %h/export -n %i -m %i -t -p -s | ||
|
||
[Install] | ||
WantedBy=default.target |
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.
This means that the service will be automatically started, when somebody logs in. This is not necessary. We want the service to be started only when somebody runs unfsd subcommand of novaboot-shell.
Adding a draft before I can test it. I'm not sure about the systemd unit file, I couldn't find any way to automatically select a free port using systemd, unfs3 doesn't support it.