diff mbox

vfs: return EOPNOTSUPP when ioctl_fsthaw() is not supported

Message ID 20161218185727.2209-1-kerolasa@iki.fi (mailing list archive)
State New, archived
Headers show

Commit Message

Sami Kerola Dec. 18, 2016, 6:57 p.m. UTC
An attempt to freeze a filesystem that does not support such operation is
reported as EOPNOTSUPP to user.  Running unfreeze to the same filesystem
returns EINVAL.  Later is a little misleading, users can mix that message
with return value from unfreezing when filesystem is not frozen.

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 fs/ioctl.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Christoph Hellwig Dec. 19, 2016, 10:24 a.m. UTC | #1
On Sun, Dec 18, 2016 at 06:57:27PM +0000, Sami Kerola wrote:
> An attempt to freeze a filesystem that does not support such operation is
> reported as EOPNOTSUPP to user.  Running unfreeze to the same filesystem
> returns EINVAL.  Later is a little misleading, users can mix that message
> with return value from unfreezing when filesystem is not frozen.

This is going to break gfs2 with it's odd ->that_super method.
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sami Kerola Dec. 19, 2016, 10:33 p.m. UTC | #2
On 19 December 2016 at 10:24, Christoph Hellwig <hch@infradead.org> wrote:
> On Sun, Dec 18, 2016 at 06:57:27PM +0000, Sami Kerola wrote:
>> An attempt to freeze a filesystem that does not support such operation is
>> reported as EOPNOTSUPP to user.  Running unfreeze to the same filesystem
>> returns EINVAL.  Later is a little misleading, users can mix that message
>> with return value from unfreezing when filesystem is not frozen.
>
> This is going to break gfs2 with it's odd ->that_super method.

Thank you for feedback, there's just one small problem with it.  I don't
quite understand the breakage.

Proposed change uses similar logic as ioctl_fsfreeze()[1] and it works fine.
If I am not entirely mistaking freeze and thaw hooks where added[2] to avoid
issues with gfs2.  And looking gfs2 super_operations init[3] does not make
me any wiser what might be wrong.

Maybe feedback had typo s/that_super/thaw_super/[4].  But even then it is
unclear to me how a null could cause breakage.

[1] https://github.com/torvalds/linux/blob/master/fs/ioctl.c#L553
[2] https://github.com/torvalds/linux/commit/48b6bca6b7b8309697fc8a101793befe92d249d9
[3] https://github.com/torvalds/linux/blob/master/fs/gfs2/super.c#L1683
[4] https://github.com/torvalds/linux/blob/master/fs/super.c#L1446
diff mbox

Patch

diff --git a/fs/ioctl.c b/fs/ioctl.c
index cb9b02940805..1d6372ef4008 100644
--- a/fs/ioctl.c
+++ b/fs/ioctl.c
@@ -566,6 +566,10 @@  static int ioctl_fsthaw(struct file *filp)
 	if (!capable(CAP_SYS_ADMIN))
 		return -EPERM;
 
+	/* If filesystem doesn't support thaw feature, return. */
+	if (!sb->s_op->unfreeze_fs)
+		return -EOPNOTSUPP;
+
 	/* Thaw */
 	if (sb->s_op->thaw_super)
 		return sb->s_op->thaw_super(sb);