On Tue, Jul 24, 2018 at 10:35:28PM +0200, Adam Borowski wrote:
> On Mon, Jul 23, 2018 at 05:26:24PM +0200, David Sterba wrote:
> > On Wed, Jul 18, 2018 at 12:08:59AM +0200, Adam Borowski wrote:
> (Combined with as-folded)
> 
> | | btrfs: allow defrag on a file opened read-only that has rw permissions
> | |
> > > Requiring a rw descriptor conflicts both ways with exec, returning ETXTBSY
> > > whenever you try to defrag a program that's currently being run, or
> > > causing intermittent exec failures on a live system being defragged.
> > > 
> > > As defrag doesn't change the file's contents in any way, there's no reason
> > > to consider it a rw operation.  Thus, let's check only whether the file
> > > could have been opened rw.  Such access control is still needed as
> > > currently defrag can use extra disk space, and might trigger bugs.
> <-----
> | | We give EINVAL when the request is invalid; here it's ok but merely the
> | | user has insufficient privileges.  Thus, this return value reflects the
> | | error better -- as discussed in the identical case for dedupe.
> | |
> | | According to codesearch.debian.net, no userspace program distinguishes
> | | these values beyond strerror().
> | |
> | | Signed-off-by: Adam Borowski <kilob...@angband.pl>
> | | Reviewed-by: David Sterba <dste...@suse.com>
> | | [ fold the EPERM patch from Adam ]
> | | Signed-off-by: David Sterba <dste...@suse.com>
> 
> [...]
> > So, I'll add the patch to 4.19 queue. It's small and isolated change so
> > a revert would be easy in case we find something bad. The 2nd patch
> > should be IMHO part of this change as it's logical to return the error
> > code in the patch that modifies the user visible behaviour.
> 
> A nitpick: the new commit message has a dangling pointer "this" to the title
> of the commit that was squashed.  It was:
> 
> | btrfs: defrag: return EPERM not EINVAL when only permissions fail
> 
> It'd be nice if it could be inserted in some form in the place I marked with
> an arrow.

Right. I've replaced 'this' with 'EPERM', thanks for catching it.
--
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

Reply via email to