Page MenuHomeFreeBSD

Support immutable motd when /var/run is tmpfs
AcceptedPublic

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

Details

Reviewers
imp
cem
delphij
Group Reviewers
manpages
Summary

When /var/run is a tmpfs and update_motd="NO",
/var/run/motd will not be created and if manually
created, will not persist after reboot.

Change motd rc script to always run.
Use update_motd variable to decide if version info should be prepended or not.

PR: 250081

Diff Detail

Repository
rS 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.Sun, Nov 22, 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.