LKML Archive on lore.kernel.org
 help / color / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: "Theodore Y. Ts'o" <tytso@mit.edu>, Jann Horn <jannh@google.com>,
	Richard Henderson <rth@twiddle.net>,
	Ivan Kokshaysky <ink@jurassic.park.msu.ru>,
	Matt Turner <mattst88@gmail.com>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	linux-fsdevel@vger.kernel.org, Arnd Bergmann <arnd@arndb.de>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	Andreas Dilger <adilger.kernel@dilger.ca>,
	linux-alpha@vger.kernel.org, linux-kernel@vger.kernel.org,
	Pavel Machek <pavel@ucw.cz>,
	linux-arch@vger.kernel.org, linux-api@vger.kernel.org
Subject: Re: [PATCH v4 1/3] fs: hoist EFSCORRUPTED definition into uapi header
Date: Tue, 22 Jan 2019 09:13:57 +1100
Message-ID: <20190121221357.GC4205@dastard> (raw)
In-Reply-To: <20190121215454.GA12996@mit.edu>

On Mon, Jan 21, 2019 at 04:54:54PM -0500, Theodore Y. Ts'o wrote:
> On Fri, Jan 18, 2019 at 05:14:38PM +0100, Jann Horn wrote:
> > Multiple filesystems can already return EFSCORRUPTED errors to userspace;
> > however, so far, definitions of EFSCORRUPTED were in filesystem-private
> > headers.
> > 
> > I wanted to use EUCLEAN to indicate data corruption in the VFS layer;
> > Dave Chinner says that I should instead hoist the definitions of
> > EFSCORRUPTED into the UAPI header and then use EFSCORRUPTED.
> > 
> > This patch is marked for stable backport because it is a prerequisite for
> > the following patch.
> > 
> > Cc: stable@vger.kernel.org
> > Suggested-by: Dave Chinner <david@fromorbit.com>
> > Signed-off-by: Jann Horn <jannh@google.com>
> 
> Before we enshrine the overloading of EUCLEAN and EFSCORRUPTED, I
> wonder if we should at least consider the option of assigning a new
> error code number for EFSCORRUPTED. 

No.

We've exposed filesystem corruption errors to userspace as errno 117
for many years now, so people are already familiar with this error
as indicating a filesystem problem.

> The downside of doing this is
> that for a while, older versions glibc won't have strerror/perror
> translation for the new error code.

And everyone will end up asking "WTF is this undefined error the
application just received?" until glibc, man pages and enough
occurrences of the question have been asked that the search engines
develop enough of a history record that they return useful results.
Not only won't users have a clue, but the app developers the users
first ask "what's this error mean" won't have a clue, either.

> On the other hand, I'm not sure
> it will be that much more confusing to the average user than
> "Structure needs cleaning".  :-)

Go search for "XFS structure needs cleaning" on your preferred
search engine and will you get lots and lots of hits indicating what
you should do when you get that error. It's taken years to build up
that history such that it's extremely useful to the average user....

> The upside of assigning a new error code is that in a year or two,
> we'll actually have an intelligible error message showing up in log
> files and in user's terminals.

The downside is that it will take several years before people will
become familiar with the new error, and we'll have to deal with the
fallout repeatedly from it. Hence, IMO, there's no upside to
changing the errno of EFSCORRUPTED now that it is largely ubiquitous
in userspace.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

  reply index

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-18 16:14 Jann Horn
2019-01-18 16:14 ` [PATCH v4 2/3] fs: don't let getdents return bogus names Jann Horn
2019-01-20 22:17   ` Dave Chinner
2019-01-18 16:14 ` [PATCH v4 3/3] fs: let filldir_t return bool instead of an error code Jann Horn
2019-01-20 22:40   ` Dave Chinner
2019-01-21 15:49     ` Jann Horn
2019-01-21 22:24       ` Dave Chinner
2019-01-23 15:07         ` Jann Horn
2019-01-31 20:39           ` Darrick J. Wong
2019-01-18 16:23 ` [PATCH v4 1/3] fs: hoist EFSCORRUPTED definition into uapi header Arnd Bergmann
2019-01-20 22:13 ` Dave Chinner
2019-01-21 21:54 ` Theodore Y. Ts'o
2019-01-21 22:13   ` Dave Chinner [this message]
2019-01-21 22:14   ` David Sterba
2019-01-21 23:51   ` Darrick J. Wong
2019-01-22  0:38     ` Theodore Y. Ts'o

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=20190121221357.GC4205@dastard \
    --to=david@fromorbit.com \
    --cc=adilger.kernel@dilger.ca \
    --cc=arnd@arndb.de \
    --cc=ebiederm@xmission.com \
    --cc=ink@jurassic.park.msu.ru \
    --cc=jannh@google.com \
    --cc=linux-alpha@vger.kernel.org \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mattst88@gmail.com \
    --cc=pavel@ucw.cz \
    --cc=rth@twiddle.net \
    --cc=tytso@mit.edu \
    --cc=viro@zeniv.linux.org.uk \
    /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

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git
	git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git
	git clone --mirror https://lore.kernel.org/lkml/9 lkml/git/9.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git