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