From owner-svn-src-head@freebsd.org Thu Nov 16 05:17:29 2017 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 8E27FDBD2DF for ; Thu, 16 Nov 2017 05:17:29 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: from mail-io0-x233.google.com (mail-io0-x233.google.com [IPv6:2607:f8b0:4001:c06::233]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 50ECB6CBE5 for ; Thu, 16 Nov 2017 05:17:29 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: by mail-io0-x233.google.com with SMTP id 189so3978223iow.10 for ; Wed, 15 Nov 2017 21:17:29 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bsdimp-com.20150623.gappssmtp.com; s=20150623; h=mime-version:sender:in-reply-to:references:from:date:message-id :subject:to:cc; bh=tYVGM4sqi0vWPWm1+v1l470wYLzN3Lz3mRnJYcbey/o=; b=ifY+cHEaLeGTq+L9tYSEN7VZ7QPntXA5BUDMt73MxXIjPSwZ7EJsb/zBNPp0LqVFyU 6TyS7MbNQW9xULZPcvzNDjA5jFkchNMDYZdqqn/MLvug59SzHN5ky0SFrH0UG921UDV0 f4fwVuncI56DqZ0JLsLh+PeFnUJW6DyH2iBmirggC49Z1RkdyJz+dVy30CmLrV+JnI8m e5nlEpXssB67mYKJW9ParPeaS/7HV28TVoldAhhkeCapRnAebYsfhHvxLcmpVvZt4BxL vXmGaS5o5kSujNZWVghRYXmBO9hiVSW9tfmPSj4QhlR6CzLqhbFP8kWSCRvmc094s9+a 2AyA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:sender:in-reply-to:references:from :date:message-id:subject:to:cc; bh=tYVGM4sqi0vWPWm1+v1l470wYLzN3Lz3mRnJYcbey/o=; b=URU4Xw21x8z6tUdlY15uya5RvUB04aXNlro2+rj44QaThdOsfr46kLWOgU5mKoK9Wi T9Y8NRh36A3hId7bPwroEXWAM6F2Ht9qvp3Gilybsj1u9f9nmJui8pEAjbe6sfrsBsra 6USSiX7C3yMOjlv+oZQ4BFHm+X/KFlEeO3jdSXedT/roc9mghBSAWrGweWwLxeuyCH3U O9ilFcAjJjNfXO9OOintG0tUgQ/5bn73opnN/zOOqvHleSzXUtwhfgG3dJZhR9OWdZGn YZhUI2jOQq34R/JWpGSU71yq4Kb5icZRn6oc8AcenAgJKyLsRfv1IhJqOgHLzHw1bYl8 JPrA== X-Gm-Message-State: AJaThX7Xsf3xPgK5jYimDc0gFdRjqVtmRDDmhDvASJsYdOBzNjIc/4cj 1FDVJEU0B02cCGW66Kg3Z8vAjq8nklrl9a6FYPr4jw== X-Google-Smtp-Source: AGs4zMbgfFL2zKJ+cF5JHzucOIbj0An7nX+94NgEBVjq+Xcb2s+iE/hElQjMbLw5gkJ762jxkCPEkQ0Gq/D+ywFiMnM= X-Received: by 10.107.81.24 with SMTP id f24mr439920iob.63.1510809448407; Wed, 15 Nov 2017 21:17:28 -0800 (PST) MIME-Version: 1.0 Sender: wlosh@bsdimp.com Received: by 10.79.108.204 with HTTP; Wed, 15 Nov 2017 21:17:27 -0800 (PST) X-Originating-IP: [2603:300b:6:5100:9151:cca:5a07:fd20] In-Reply-To: References: <201711151840.vAFIefKV002185@repo.freebsd.org> <201711151847.vAFIlGD9052509@pdx.rh.CN85.dnsmgr.net> From: Warner Losh Date: Wed, 15 Nov 2017 22:17:27 -0700 X-Google-Sender-Auth: Pue6Oxix7r4GYasxBh0ASjLZraY Message-ID: Subject: Re: svn commit: r325860 - head/sbin/newfs To: Ed Maste Cc: "Rodney W. Grimes" , src-committers , "svn-src-all@freebsd.org" , "svn-src-head@freebsd.org" , Kirk McKusick Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.25 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.25 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 16 Nov 2017 05:17:29 -0000 On Wed, Nov 15, 2017 at 6:46 PM, Ed Maste wrote: > On 15 November 2017 at 19:36, Warner Losh wrote: > > > > On Wed, Nov 15, 2017 at 5:05 PM, Ed Maste wrote: > >> > >> On 15 November 2017 at 13:47, Rodney W. Grimes > >> 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: diff --git a/sbin/fsck_ffs/pass5.c b/sbin/fsck_ffs/pass5.c index 16c46bece00..06e1838a7f1 100644 --- a/sbin/fsck_ffs/pass5.c +++ b/sbin/fsck_ffs/pass5.c @@ -73,6 +73,7 @@ pass5(void) newcg->cg_niblk = fs->fs_ipg; if (preen == 0 && yflag == 0 && fs->fs_magic == FS_UFS2_MAGIC && fswritefd != -1 && (fs->fs_metackhash & CK_CYLGRP) == 0 && + getosreldate() >= 1200046 && reply("ADD CYLINDER GROUP CHECKSUM PROTECTION") != 0) { fs->fs_metackhash |= CK_CYLGRP; rewritecg = 1; diff --git a/sbin/newfs/mkfs.c b/sbin/newfs/mkfs.c index f68c42ec6b3..0e7ee539265 100644 --- a/sbin/newfs/mkfs.c +++ b/sbin/newfs/mkfs.c @@ -495,7 +495,7 @@ restart: /* * Set flags for metadata that is being check-hashed. */ - if (Oflag > 1) + if (Oflag > 1 && getosreldate() >= 1200046) sblock.fs_metackhash = CK_CYLGRP; /* which would avoid setting the flag on a problematical kernel. Here forward compat is easy, and the consequences are scary messages, so I think we should do something more like the above. I don't think we need some kind of "do it anyway" override flag. since that doesn't fit well with the rest of UFS "works by default where we can figure it out" philosophy. I'll cleanup the above with a #define for 1200046. I've cc'd Kirk to see what he thinks of the idea. It generally fits with what we've done in the past for forward compat that's easy but protects the user from harshness. Warner