svn commit: r325860 - head/sbin/newfs

[John Baldwin]

On Wednesday, November 15, 2017 10:17:27 PM Warner Losh wrote:
> On Wed, Nov 15, 2017 at 6:46 PM, Ed Maste <emaste at freebsd.org> wrote:
> 
> > On 15 November 2017 at 19:36, Warner Losh <imp at bsdimp.com> wrote:
> > >
> > > On Wed, Nov 15, 2017 at 5:05 PM, Ed Maste <emaste at freebsd.org> wrote:
> > >>
> > >> On 15 November 2017 at 13:47, Rodney W. Grimes
> > >> <freebsd at pdx.rh.cn85.dnsmgr.net> wrote:
> > >> >> Author: emaste
> > >> >> Date: Wed Nov 15 18:40:40 2017
> > >> >> New Revision: 325860
> > >> >> URL: https://svnweb.freebsd.org/changeset/base/325860
> > >> >>
> > >> >> Log:
> > >> >>   newfs: warn if newer than kernel
> > >> >>
> > >> >>   Creating a UFS filesystem with a newfs newer than the running
> > kernel,
> > >> >>   and then mounting that filesystem, can lead to interesting
> > failures.
> > >> >>
> > >> >>   Add a safety belt to explicitly warn when newfs is newer than the
> > >> >>   running kernel.
> > >> >
> > >> > You should probably make the warning if (newer || older) as
> > >> > either is likely to have interesting side effects, as are
> > >> > mounting ufs file systems on different versions.
> > >>
> > >> Why would an older newfs cause trouble? Forward compatibility should be
> > >> fine
> > >
> > > The only scenario that 'old' would cause problems is that if you did a
> > newfs
> > > with a new binary on a new kernel, mounted the file system, wrote files
> > to
> > > it, then rebooted with an old kernel, mounted the filesystem there,
> > writing
> > > new files to it, and then unmounting and running with a new kernel.
> >
> > Right, but that's not older newfs. AFAICT there's no reason at all for
> > a (newer || older) warning.
> 
> 
> I concur.
> 
> > I'm not sure that the new safety belt is reasonable. Today it's fine, but
> > > over time it will start producing false-positive warnings since the real
> > > issue is just before/after the cg change, not old/new in general. I'd be
> > > tempted to make a check against newfs being >= 1200046 while the kernel
> > is <
> > > 1200046. There wasn't a specific bump for this change to sys/param.h, but
> > > this version was bumped within a few hours of Kirk's change.
> >
> > Well, we don't in general support using a userland newer than the
> > running kernel, other than on a best-effort basis to facilitate
> > upgrades and development. This one is only a warning so I don't see
> > much harm in leaving it in place, and it would catch any new cases of
> > a similar nature. If such a warning was already in place we might have
> > avoided the issue where our snapshots produced checksum mismatch
> > messages. But I don't have a strong objection to a hardcoded version
> > check.
> >
> 
> What would have fixed the snapshot isn't a warning that nobody will notice.
> But rather something like the following:

No, what would really fix the snapshot is using makefs or the host newfs
instead of the target newfs during the build.  That part of the release
process is still fundamentally broken and fixing that wouldn't require various
one-off fixes for future changes that happen to introduce compatability but
would fix the entire class of issues.

-- 
John Baldwin