On Sun, May 13, 2018 at 11:26:39AM -0700, Darrick J. Wong wrote:
> On Sun, May 13, 2018 at 06:21:52PM +0000, Mark Fasheh wrote:
> > On Fri, May 11, 2018 at 05:06:34PM -0700, Darrick J. Wong wrote:
> > > On Fri, May 11, 2018 at 12:26:51PM -0700, Mark Fasheh wrote:
> > > > Right now we return EINVAL if a process does not have permission to
> > > > dedupe a
> > > > file. This was an oversight on my part. EPERM gives a true description
> > > > of
> > > > the nature of our error, and EINVAL is already used for the case that
> > > > the
> > > > filesystem does not support dedupe.
> > > >
> > > > Signed-off-by: Mark Fasheh <mfas...@suse.de>
> > > > ---
> > > > fs/read_write.c | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/fs/read_write.c b/fs/read_write.c
> > > > index 77986a2e2a3b..8edef43a182c 100644
> > > > --- a/fs/read_write.c
> > > > +++ b/fs/read_write.c
> > > > @@ -2038,7 +2038,7 @@ int vfs_dedupe_file_range(struct file *file,
> > > > struct file_dedupe_range *same)
> > > > info->status = -EINVAL;
> > > > } else if (!(is_admin || (dst_file->f_mode &
> > > > FMODE_WRITE) ||
> > > > uid_eq(current_fsuid(), dst->i_uid))) {
> > > > - info->status = -EINVAL;
> > > > + info->status = -EPERM;
> > >
> > > Hmm, are we allowed to change this aspect of the kabi after the fact?
> > >
> > > Granted, we're only trading one error code for another, but will the
> > > existing users of this care? xfs_io won't and I assume duperemove won't
> > > either, but what about bees? :)
> >
> > Yeah if you see my initial e-mail I check bees and also rust-btrfs. I think
> > this is fine as we're simply expanding on an error code return. There's no
> > magic behavior expected with respect to these error codes either.
>
> Ok. No objections from me, then.
>
> Acked-by: Darrick J. Wong <darrick.w...@oracle.com>For what it's worth, no objection from me either. ;) bees runs only with admin privilege and will never hit the modified line. If bees is started without admin privilege, the TREE_SEARCH_V2 ioctl fails. bees uses this ioctl to walk over all the data in the filesystem, so without admin privilege, bees never opens, reads, or dedupes anything. bees relies on having an accurate internal model of btrfs structure and behavior to issue dedup commands that will work and do useful things; however, unexpected kernel behavior or concurrent user data changes will make some dedups fail. When that happens bees just abandons the extent immediately: a user data change will be handled in the next pass over the filesystem, but an unexpected kernel behavior needs bees code changes to correctly predict the new kernel behavior before the dedup can be reattempted. > --D > > > --Mark > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > > the body of a message to majord...@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
signature.asc
Description: PGP signature
