All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Darrick J. Wong" <darrick.wong@oracle.com>
To: Eric Sandeen <sandeen@sandeen.net>
Cc: Brian Foster <bfoster@redhat.com>, linux-xfs@vger.kernel.org
Subject: Re: [PATCH 2/3] xfs: define a new "needrepair" feature
Date: Fri, 4 Dec 2020 13:36:32 -0800	[thread overview]
Message-ID: <20201204213632.GG629293@magnolia> (raw)
In-Reply-To: <2a5ce5a2-9df4-5c19-13d3-f0a16d8030ba@sandeen.net>

On Fri, Dec 04, 2020 at 02:07:49PM -0600, Eric Sandeen wrote:
> On 12/1/20 10:18 AM, Brian Foster wrote:
> > On Mon, Nov 30, 2020 at 07:37:31PM -0800, Darrick J. Wong wrote:
> >> From: Darrick J. Wong <darrick.wong@oracle.com>
> >>
> >> Define an incompat feature flag to indicate that the filesystem needs to
> >> be repaired.  While libxfs will recognize this feature, the kernel will
> >> refuse to mount if the feature flag is set, and only xfs_repair will be
> >> able to clear the flag.  The goal here is to force the admin to run
> >> xfs_repair to completion after upgrading the filesystem, or if we
> >> otherwise detect anomalies.
> >>
> >> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> >> ---
> > IIUC, we're using an incompat bit to intentionally ensure the filesystem
> > cannot mount, even on kernels that predate this particular "needs
> > repair" feature. The only difference is that an older kernel would
> > complain about an unknown feature and return a different error code.
> > Right?
> > 
> > That seems reasonable, but out of curiousity is there a need/reason for
> > using an incompat bit over an ro_compat bit?
> 
> I'm a fan of a straight-up incompat, because we don't really know what
> format changes in the future might require this flag to be set; nothing
> guarantees that future changes will be ro-compat-safe, right?

Correct.  In the case of the inobtcount upgrade, we know that the
inobt/finobt blockcounts in the AGI are zero (and thus wrong) right
after the upgrade.  If we made it a rocompat bit then we'd allow ro
mounts but we'd also have to be careful to prohibit a ro->rw remount,
at which point the admin gets a Big Surprise.

Why not just make the admin repair the system right then and there?
I mean, xfs_admin is already going to run repair anyway, so in practice
there shouldn't be that many people trying to push an "upgraded but
needs repair" fs at the kernel anyway.

--D

> -Eric

  reply	other threads:[~2020-12-04 21:37 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-01  3:37 [PATCH 0/3] xfs: add the ability to flag a fs for repair Darrick J. Wong
2020-12-01  3:37 ` [PATCH 1/3] xfs: move kernel-specific superblock validation out of libxfs Darrick J. Wong
2020-12-01 16:17   ` Brian Foster
2020-12-04 20:35   ` Eric Sandeen
2020-12-04 21:12     ` Darrick J. Wong
2020-12-04 21:46       ` Eric Sandeen
2020-12-04 23:02         ` Darrick J. Wong
2020-12-04 23:29         ` Dave Chinner
2020-12-01  3:37 ` [PATCH 2/3] xfs: define a new "needrepair" feature Darrick J. Wong
2020-12-01 16:18   ` Brian Foster
2020-12-01 16:25     ` Darrick J. Wong
2020-12-01 17:09       ` Brian Foster
2020-12-04 20:07     ` Eric Sandeen
2020-12-04 21:36       ` Darrick J. Wong [this message]
2020-12-01  3:37 ` [PATCH 3/3] xfs: enable the needsrepair feature Darrick J. Wong
2020-12-01 16:18   ` Brian Foster
2020-12-04 20:35   ` Eric Sandeen
2020-12-04  1:13 ` [PATCH 4/3] xfs_db: support the needsrepair feature flag in the version command Darrick J. Wong
2020-12-04 20:32   ` Eric Sandeen
2020-12-04 21:09     ` Darrick J. Wong
2020-12-04 21:16       ` Eric Sandeen
2020-12-04  1:13 ` [PATCH 5/3] xfs_repair: clear the needsrepair flag Darrick J. Wong
2020-12-06 23:09 [PATCH v2 0/3] xfs: add the ability to flag a fs for repair Darrick J. Wong
2020-12-06 23:09 ` [PATCH 2/3] xfs: define a new "needrepair" feature Darrick J. Wong
2020-12-06 23:47   ` Dave Chinner
2020-12-09 17:15   ` Eric Sandeen
2020-12-09 18:04   ` Christoph Hellwig
2020-12-09 18:10     ` Darrick J. Wong
2020-12-09 18:12       ` Christoph Hellwig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201204213632.GG629293@magnolia \
    --to=darrick.wong@oracle.com \
    --cc=bfoster@redhat.com \
    --cc=linux-xfs@vger.kernel.org \
    --cc=sandeen@sandeen.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.