All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@infradead.org>
To: "Darrick J. Wong" <darrick.wong@oracle.com>
Cc: Christoph Hellwig <hch@infradead.org>, Chris Mason <clm@fb.com>,
	linux-fsdevel@vger.kernel.org, xfs@oss.sgi.com
Subject: Re: clone ioctl return values
Date: Wed, 18 Nov 2015 07:11:45 -0800	[thread overview]
Message-ID: <20151118151145.GA5365@infradead.org> (raw)
In-Reply-To: <20151118030117.GA26143@birch.djwong.org>

On Tue, Nov 17, 2015 at 07:01:17PM -0800, Darrick J. Wong wrote:
> Current btrfs returns EINVAL if the file isn't open for writing or is
> append-only.  I think EBADF captures the situation here better and
> it's what Anna is pushing for copy_file_range.  We can make the
> change, but generic/157 will fail on old kernels if we do this.

As the old errno was rather wrong compared to similar syscalls I'd
say don't worry about this.

> One other thing I noticed -- prior to Anna's patchset, trying to
> invoke the reflink ioctl with a device, pipe, or socket as the
> destination could return a variety of error codes (-ENOTTY, -EINVAL,
> -ENOIOCTLCMD, etc.) which has all been replaced with -EOPNOTSUPP.
> That seems like a reasonable direction to take the test case.
> 
> Does this seem like a reasonable addition to the plan?

-ENOIOCTLCMD should never reach userspace, this is a clear bug.
-EINVAL also seems wrong, but ENOTTY is perfectly valid for an
ioctl based implementation.  So I'd say we should allow
ENOTTY or -EOPNOTSUPP in the test case by using filters, and ensure
btrfs and nfs only return those in 4.4 and maybe with backports to
stable.

> Should invalid inputs to the dedupe ioctl return the same error codes
> as the same invalid inputs to the reflink ioctl?  I've been working on
> patches to hoist EXTENT_SAME to the VFS.

I think so.  EXTENT_SAME is so similar to clone that it should almost be
a flag for it in the low level API.

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

WARNING: multiple messages have this Message-ID (diff)
From: Christoph Hellwig <hch@infradead.org>
To: "Darrick J. Wong" <darrick.wong@oracle.com>
Cc: Christoph Hellwig <hch@infradead.org>, Chris Mason <clm@fb.com>,
	xfs@oss.sgi.com, linux-fsdevel@vger.kernel.org
Subject: Re: clone ioctl return values
Date: Wed, 18 Nov 2015 07:11:45 -0800	[thread overview]
Message-ID: <20151118151145.GA5365@infradead.org> (raw)
In-Reply-To: <20151118030117.GA26143@birch.djwong.org>

On Tue, Nov 17, 2015 at 07:01:17PM -0800, Darrick J. Wong wrote:
> Current btrfs returns EINVAL if the file isn't open for writing or is
> append-only.  I think EBADF captures the situation here better and
> it's what Anna is pushing for copy_file_range.  We can make the
> change, but generic/157 will fail on old kernels if we do this.

As the old errno was rather wrong compared to similar syscalls I'd
say don't worry about this.

> One other thing I noticed -- prior to Anna's patchset, trying to
> invoke the reflink ioctl with a device, pipe, or socket as the
> destination could return a variety of error codes (-ENOTTY, -EINVAL,
> -ENOIOCTLCMD, etc.) which has all been replaced with -EOPNOTSUPP.
> That seems like a reasonable direction to take the test case.
> 
> Does this seem like a reasonable addition to the plan?

-ENOIOCTLCMD should never reach userspace, this is a clear bug.
-EINVAL also seems wrong, but ENOTTY is perfectly valid for an
ioctl based implementation.  So I'd say we should allow
ENOTTY or -EOPNOTSUPP in the test case by using filters, and ensure
btrfs and nfs only return those in 4.4 and maybe with backports to
stable.

> Should invalid inputs to the dedupe ioctl return the same error codes
> as the same invalid inputs to the reflink ioctl?  I've been working on
> patches to hoist EXTENT_SAME to the VFS.

I think so.  EXTENT_SAME is so similar to clone that it should almost be
a flag for it in the low level API.

  reply	other threads:[~2015-11-18 15:11 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-16 12:04 clone ioctl return values Christoph Hellwig
2015-11-16 12:04 ` Christoph Hellwig
2015-11-17  0:28 ` Darrick J. Wong
2015-11-17  0:28   ` Darrick J. Wong
2015-11-17 10:54   ` Christoph Hellwig
2015-11-17 10:54     ` Christoph Hellwig
2015-11-17 13:57     ` Chris Mason
2015-11-17 13:57       ` Chris Mason
2015-11-17 15:22       ` Christoph Hellwig
2015-11-17 15:22         ` Christoph Hellwig
2015-11-17 15:33         ` Al Viro
2015-11-17 15:33           ` Al Viro
2015-11-17 15:36           ` Christoph Hellwig
2015-11-17 15:36             ` Christoph Hellwig
2015-11-17 18:42         ` Chris Mason
2015-11-17 18:42           ` Chris Mason
2015-11-18  3:01         ` Darrick J. Wong
2015-11-18  3:01           ` Darrick J. Wong
2015-11-18 15:11           ` Christoph Hellwig [this message]
2015-11-18 15:11             ` Christoph Hellwig
2015-11-17 15:12   ` Christoph Hellwig
2015-11-17 15: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=20151118151145.GA5365@infradead.org \
    --to=hch@infradead.org \
    --cc=clm@fb.com \
    --cc=darrick.wong@oracle.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --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 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.