Page MenuHomeFreeBSD

Support immutable motd when /var/run is tmpfs
Needs ReviewPublic

Authored by guyyur_gmail.com on Oct 3 2020, 7:10 PM.

Details

Reviewers
imp
cem
delphij
Group Reviewers
manpages
Summary

create /var/run/motd if missing when update_motd="NO"

There are three cases where /var/run/motd won't exist if update_motd="NO"

  1. /var/run is tmpfs.
  2. doing first install and setting update_motd="NO" before rebooting.
  3. upgrading from a version that used /etc/motd.

Copy /etc/motd if it is readable to preserve motd if upgrading
from a version before /var/run/motd was introduced.
Otherwise, copy /etc/motd.template.

PR: 250081

Diff Detail

Repository
R10 FreeBSD src repository
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

bcr added a subscriber: bcr.

Man page changes look good to me.

changes from last diff:
libexec/rc/rc.d/motd

  1. overwrite /etc/motd.template with /etc/motd if /etc/motd is a regular file
  2. always create /etc/motd symbolic link

share/man/man5/motd.5

  1. rebase over latest version of the file
  2. describe removal of /etc/motd and creation of symbolic link
  3. remove mention of update_motd from /etc/motd description since

user can choose to keep it as a symbolic link even if update_motd="NO"

usr.bin/login/Makefile

  1. create /etc/motd symbolic link when installing configuration

usr.sbin/mergemaster/mergemaster.sh

  1. support ignoring symbolic links in IGNORE_FILES

if /var/run/motd doesn't exist (for example, when using update_motd="NO"),
"test -e" in temp root will fail and ${TEMPROOT}/${file} will not be removed
so prompt to handle file will be shown even if /etc/motd is listed in IGNORE_FILES.

gbe added a subscriber: gbe.

Ok from the man page perspective. Adding Warner and Conrad, since they have done the latest changes to the motd.

I'm not super keen on this design. It makes the file type of /etc/motd more confusing, turns _PATH_MOTDFILE into a symlink for the default case, and I'm unclear why you're modifying the motd rc script at all if your proposal involves update_motd="NO".

What about instead making the generated text part of writing the /var/run file optional? I.e. the proposed option would just install the static template directly into /var/run unmodified.

Sounds good to me.
Do you want me to prepare a new patch with your suggestion?

Sure. Warner or Xin Li (CC'd, involved in earlier part of this design IIRC) might prefer something else, so you could also wait to see if they have an opinion.

guyyur_gmail.com edited the summary of this revision. (Show Details)

Previous changes were confusing with /etc/motd being either symlink or file.
As suggested, use simpler design.
motd script will always run and generate /var/run/motd.
update_motd variable will decide if motd script prepends version info or not.

I previously also opened bugzilla bug for it.
Do I need to connect the review to the bug?
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=250081

Code change seems good. The doc change is possibly incomplete (still potentially confusing) but I don't consider that blocking.

This revision is now accepted and ready to land.Nov 22 2020, 10:15 PM

I don't really like the last revision, which materially changed the meaning of update_motd (now on boot /etc/rc.d/motd is always fully executed, and when update_motd is set to NO, the version string is deleted, instead of stay the same as previous).

Looking at the bug report, it appears that what the reporter wanted is to have the file immutable (i.e. it shall stay where it was and not being touched), so it appears like that we should revert the login(1) portion of change and leave /etc/rc.d/motd as-is, so when a /etc/motd file is there, it would stay as-is when update_motd is there, preserving the historical behavior.

The problem with reverting the login(1) portion and leaving /etc/rc.d/motd as-is is that then the default of update_motd=YES will not work.
The motd script only generates /etc/motd symlink if motd.template does not exist when the script runs.
motd.template will exist on new installations so login will look for /etc/motd but script will only generate /var/run/motd.

As I understand the new design, /var/run/motd is now the canonical motd file and /etc/motd.template is the file users are expected to edit.
This means that update_motd should now be in relation to /var/run/motd not /etc/motd.

Do you have a suggestion for a different way to fix it?

jlduran_gmail.com added inline comments.
libexec/rc/rc.d/motd
14

Why was this removed?
This would not allow us to do:

# service motd disable

to set update_motd="NO", for instance.

I was not aware of "service X disable" usage and I thought
when rcvar is set to "NO" it prevented the run of the rc script.
I see that it only prevents run_rc_command from running.
motd_start won't run but motd is loaded and can do stuff before run_rc_command.

Something still needs to create /var/run/motd in the case of update_motd="NO".
If update_motd="NO", /var/run/motd won't exist on first install unless created manually
and if /var/run is tmpfs it will not exist after every boot.

Will it be more acceptable to instead just add copying of /etc/motd.template to /var/run/motd
before rc_run_command (only if update_motd="NO" and the file doesn't exist)?

Something like this:


load_rc_config $name

if ! checkyesno $rcvar && [ ! -e "${TARGET}" ]; then
if [ -f "$COMPAT_MOTD" -a ! -L "$COMPAT_MOTD" ]; then
install -C -o root -g wheel -m "${PERMS}" "$COMPAT_MOTD" "${TARGET}"
else
install -C -o root -g wheel -m "${PERMS}" "$TEMPLATE" "${TARGET}"
fi
fi

run_rc_command "$1"

guyyur_gmail.com edited the summary of this revision. (Show Details)

Updated changes:
When update_motd="NO", don't overwrite /var/run/motd, only copy if missing.
Don't remove rcvar line to allow "service motd disable",
check rcvar before run_rc_command instead.

This revision now requires review to proceed.Mar 30 2021, 9:23 PM