public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
From: David Chinner <dgc@sgi.com>
To: Anton Altaparmakov <aia21@cam.ac.uk>
Cc: David Chinner <dgc@sgi.com>, Nicholas Miell <nmiell@comcast.net>,
	linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	xfs@oss.sgi.com, hch@infradead.org
Subject: Re: [RFC] add FIEMAP ioctl to efficiently map file allocation
Date: Wed, 2 May 2007 19:15:26 +1000	[thread overview]
Message-ID: <20070502091526.GW77450368@melbourne.sgi.com> (raw)
In-Reply-To: <084192A9-D739-44F2-AD21-30BC30486F07@cam.ac.uk>

On Tue, May 01, 2007 at 07:46:53PM +0100, Anton Altaparmakov wrote:
> On 1 May 2007, at 15:20, David Chinner wrote:
> >>
> >>So, either the filesystem will understand the flag or iff the  
> >>unknown flag
> >>is in the incompat set, it will return EINVAL or else the unknown  
> >>flag will
> >>be safely ignored.
> >
> >My point was that there is a difference between specification and
> >implementation - if the specification says something is compulsory,
> >then they must be implemented in the filesystem. This is easy
> >enough to ensure by code review - we don't need additional interface
> >complexity for this....
> 
> You are wrong about this because you are missing the point that you  
> have no code to review.  The users that will use those flags are  
> going to be applications that run in user space.  Chances are you  
> will never see their code.  Heck, they might not even be open source  
> applications...

Ummm - the specification defines what is compulsory for *filesystems*
to implement, not what applications can use. We don't need to see
what the applications do - what we care about is that all filesystems
implement the compulsory part of the specification. That's the code
we review, and that's what I was referring to.

> And all applications will run against a multitude of  
> kernels.  So version X of the application will run on kernel 2.4.*,  
> 2.6.*, a.b.*, etc...  For future expandability of the interface I  
> think it is important to have both compulsory and non-compulsory flags.

Ah, so that's what you want - a mutable interface. i.e. versioning.

So how does compusory flags help here? What happens if a voluntary
flag now becomes compulsory? Or vice versa? How is the application
supposed to deal with this dynamically?

I suggested a version number for this right back at the start of
this discussion and got told that we don't want versioned interfaces
because we should make the effort to get it right the first time.
I don't think this can be called "getting it right".

> For example there is no reason why FIEMAP_HSM_READ needs to be  
> compulsory.  Most filesystems do not support HSM so can safely ignore  
> it.

They might be able to safely ignore it, but in reality it should
be saying "I don't understand this". If the application *needs* to
use a flag like this, then it should be told that the filesystem is
not capable of doing what it was asked!

OTOH if the application does not need to use the flag, then it
shouldn't be using it and we shouldn't be silently ignoring
incorrect usage of the provided API.

What you are effectively saying about these "voluntary" flags
is that their behaviour is _undefined_. That is, if you use
these flags what you get on a successful call is undefined;
it may or may not contain what you asked for but you can't
tell if it really did what you want or returned the information
you asked for.

This is a really bad semantic to encode into an API.

> And vice versa, an application might specify some weird and funky yet  
> to be developed feature that it expects the FS to perform and if the  
> FS cannot do it (either because it does not support it or because it  
> failed to perform the operation) the application expects the FS to  
> return an error and not to ignore the flag.  An example could be the  
> asked for FIEMAP_XATTR_FORK flag.  If that is implemented, and the FS  
> ignores it it will return the extent map for the file data instead of  
> the XATTR_FORK!  Not what the application wanted at all.  Ouch!  So  
> this is definitely a compulsory flag if I ever saw one.

Yes, the correct answer is -EOPNOTSUPP or -EINVAL in this case. But
we don't need a flag defined in the user visible API to tell us
that we need to return an error here.

> So as you see you must support both voluntary and compulsory flags...

No, you've managed to convince me that they are not necessary and
they are in fact a Bad Idea... ;)

> Also consider what I said above about different kernels.  A new  
> feature is implemented in kernel 2.8.13 say that was not there before  
> and an application is updated to use that feature.  There will be  
> lots of instances where that application will still be run on older  
> kernels where this feature does not exist. 

This is *exactly* where silently ignoring flags really falls down.
On 2.8.13, the flag is silently ignored. On 2.8.14, the flag does
something and it returns different structure contents for the same
state. Now how does the application writer know which is correct or
how to tell the difference?  They have to guess or write detection
code which is exactly what we want to avoid.

I objected to the UNKNOWN flag because it wasn't explicit
in it's meaning - I'm doing the same thing here. An interface
needs to be explicitly defined and should not have and undefined
behaviour in it....

Cheers,

Dave.
-- 
Dave Chinner
Principal Engineer
SGI Australian Software Group

  reply	other threads:[~2007-05-02  9:15 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-04-12 11:05 [RFC] add FIEMAP ioctl to efficiently map file allocation Andreas Dilger
2007-04-12 11:22 ` Anton Altaparmakov
2007-04-13  4:01   ` Andreas Dilger
2007-04-13  7:46     ` Anton Altaparmakov
2007-04-13 14:53     ` Jeff Mahoney
2007-04-13  1:33 ` Nicholas Miell
2007-04-13 10:15 ` Christoph Hellwig
2007-04-13 11:38   ` Anton Altaparmakov
2007-04-13 18:55     ` Nicholas Miell
2007-04-16  8:01 ` Timothy Shimmin
2007-04-18 23:03   ` Andreas Dilger
2007-04-16 11:22 ` David Chinner
2007-04-19  0:21   ` Andreas Dilger
2007-04-19  1:54     ` David Chinner
2007-04-30 22:44       ` Andreas Dilger
2007-05-01  4:22         ` David Chinner
2007-05-01  4:39           ` Nicholas Miell
2007-05-01 14:20             ` David Chinner
2007-05-01 18:46               ` Anton Altaparmakov
2007-05-02  9:15                 ` David Chinner [this message]
2007-05-02  9:36                   ` Anton Altaparmakov
2007-05-02 10:57                     ` David Chinner
2007-05-02 11:17                       ` Anton Altaparmakov
2007-05-03  7:49                       ` Andreas Dilger
2007-05-03  8:23                         ` Anton Altaparmakov
2007-05-02  9:45                   ` Anton Altaparmakov
2007-05-01 22:32               ` Andreas Dilger
2007-05-01 18:37           ` Anton Altaparmakov
2007-05-02  0:06             ` David Chinner
2007-05-02  8:16               ` Anton Altaparmakov
2007-10-29 19:45                 ` Andreas Dilger
2007-10-29 20:57                   ` Mark Fasheh
2007-10-29 22:13                     ` Andreas Dilger
2007-10-29 22:29                       ` Andreas Dilger
2007-10-29 22:40                         ` Mark Fasheh
2007-10-30  0:11                       ` Mark Fasheh
2007-10-30  0:25                         ` Andreas Dilger
2007-10-29 22:25                   ` David Chinner
2007-05-01 22:30           ` Andreas Dilger
2007-05-02  2:26             ` David Chinner
2007-05-02  8:23             ` Anton Altaparmakov
2007-05-02  8:30               ` Anton Altaparmakov
2007-05-02  9:48               ` David Chinner
2007-05-02  9:56                 ` Anton Altaparmakov
2007-04-19  6:23     ` Timothy Shimmin

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=20070502091526.GW77450368@melbourne.sgi.com \
    --to=dgc@sgi.com \
    --cc=aia21@cam.ac.uk \
    --cc=hch@infradead.org \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=nmiell@comcast.net \
    --cc=xfs@oss.sgi.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox