Re: consequences of XFS_IOC_FSSETXATTR on non-empty file?

From: Dave Chinner <david@fromorbit.com>
To: Ilya Dryomov <ilya.dryomov@inktank.com>
Cc: "ceph-devel@vger.kernel.org" <ceph-devel@vger.kernel.org>,
	Sage Weil <sage@inktank.com>, Samuel Just <sam.just@inktank.com>,
	xfs@oss.sgi.com
Subject: Re: consequences of XFS_IOC_FSSETXATTR on non-empty file?
Date: Mon, 14 Jul 2014 08:55:32 +1000	[thread overview]
Message-ID: <20140713225532.GD22339@dastard> (raw)
In-Reply-To: <CALFYKtC7J1ZvW40wsSWmVpNS5T3tmAOmY9CP=nsh4r=iUjeznA@mail.gmail.com>

On Sun, Jul 13, 2014 at 09:01:13PM +0400, Ilya Dryomov wrote:
> On Sun, Jul 13, 2014 at 5:48 AM, Samuel Just <sam.just@inktank.com> wrote:
> > Actually, on this ubuntu kernel (3.13.0-24-generic), it doesn't seem
> > to give an error.  I'll attach my test case for that.  We don't yet
> > have a way of reproducing the corruption -- the ext_size change in the
> > osd simply seemed like a promising lead.
> > -Sam
> >
> > On Sat, Jul 12, 2014 at 6:26 PM, Dave Chinner <david@fromorbit.com> wrote:
> >> On Sat, Jul 12, 2014 at 06:16:54PM -0700, Samuel Just wrote:
> >>> Hi,
> >>>
> >>> We are seeing reports of ceph-osd stores on xfs of files with some
> >>> garbage data (possibly misplaced from data elsewhere in the
> >>> filesystem).  There was a bug for a while where the ceph-osd process
> >>> would set a value for fsx_extsize on a non-empty (possibly sparse)
> >>> file using XFS_IOC_FSSETXATTR.  Could that plausibly result in a file
> >>> with garbage data?
> >>
> >> No, setting an extent size on a non-empty file will simply fail
> >> with EINVAL.
> 
> AFAIR it checks whether or not any extents are actually allocated, not
> whether the file is empty or not.

FWIW, that's an implementation detail, not the definition of the
intended behaviour of the ioctl.  Indeed, the man page says:

"The fsx_xflags realtime file bit and the file's extent size may be
changed only when the file is empty, ..."

For most people, "[non-]empty file" is much more easily understood
than "a file without real extents, but might have been written to
and so have dirty, in-memory delayed allocation data whose
asynchronous flushing may or may not affect the behaviour of a call
to XFS_IOC_FSSETXATTR".

i.e. the intended application behaviour is that they should only be
able to change the extent size hint *before* any data is written to
the file.

> I think if you call fsync() or even
> fdatasync() before close(fd), it will fail as expected.

Only if you are trying to change the extent size immediately after
the first write you do to an empty file. Which is, as per the above,
not the recommended or intended use of the ioctl.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs