From: Eric Biggers <ebiggers@kernel.org>
To: Harry Austen <harryausten@hotmail.co.uk>
Cc: jaegeuk@kernel.org, linux-kernel@vger.kernel.org,
linux-f2fs-devel@lists.sourceforge.net
Subject: Re: [f2fs-dev] [RFC PATCH] f2fs: disallow setting unsettable file attributes
Date: Mon, 31 Jan 2022 19:34:53 +0000 [thread overview]
Message-ID: <Yfg53VflpMY3eDB4@gmail.com> (raw)
In-Reply-To: <AM6PR10MB2838F71E6117BFFAD3FD5A69FA249@AM6PR10MB2838.EURPRD10.PROD.OUTLOOK.COM>
On Sun, Jan 30, 2022 at 05:21:39PM +0000, Harry Austen wrote:
> After Eric kindly pointed out the reasons why my initial kernel patch
> attempt was incorrect
> (https://lore.kernel.org/lkml/Ye79OLCFLR3H+GnY@gmail.com/), I had a
> rethink as to if the current implementation could be improved in any
> way.
>
> I wondered whether something along the lines of the following patch
> would be more acceptable? It is intentionally verbose in order to
> demonstrate the concept as this is intended purely as an RFC.
>
> What if SETFLAGS returned EOPNOTSUPP if userspace is actually trying to
> *set* one of the unsettable flags (i.e. it isn't set already)? I believe
> this would therefore not break chattr(1), as flags that are retrieved
> from GETFLAGS can still be passed into SETFLAGS without error.
>
> If there is some other ABI compatibility that needs to be maintained
> that is broken by this, then please let me know. Also, I have not yet
> determined whether there are any concerns with calling f2fs_fileattr_get
> from inside f2fs_fileattr_set, e.g. speed/performance? so any thoughts
> would be greatly appreciated.
>
> Signed-off-by: Harry Austen <harryausten@hotmail.co.uk>
> ---
> fs/f2fs/file.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
This makes sense, but this ioctl isn't f2fs-specific; it's implemented by other
filesystems, most notably ext2/ext4 where it originated. f2fs shouldn't have
different behavior for the same ioctl. If you want to change this ioctl, you
need to start a wider discussion (including the linux-fsdevel and linux-ext4
mailing lists) where the change is proposed for all filesystems.
- Eric
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
WARNING: multiple messages have this Message-ID (diff)
From: Eric Biggers <ebiggers@kernel.org>
To: Harry Austen <harryausten@hotmail.co.uk>
Cc: linux-f2fs-devel@lists.sourceforge.net,
linux-kernel@vger.kernel.org, jaegeuk@kernel.org,
chao@kernel.org
Subject: Re: [RFC PATCH] f2fs: disallow setting unsettable file attributes
Date: Mon, 31 Jan 2022 19:34:53 +0000 [thread overview]
Message-ID: <Yfg53VflpMY3eDB4@gmail.com> (raw)
In-Reply-To: <AM6PR10MB2838F71E6117BFFAD3FD5A69FA249@AM6PR10MB2838.EURPRD10.PROD.OUTLOOK.COM>
On Sun, Jan 30, 2022 at 05:21:39PM +0000, Harry Austen wrote:
> After Eric kindly pointed out the reasons why my initial kernel patch
> attempt was incorrect
> (https://lore.kernel.org/lkml/Ye79OLCFLR3H+GnY@gmail.com/), I had a
> rethink as to if the current implementation could be improved in any
> way.
>
> I wondered whether something along the lines of the following patch
> would be more acceptable? It is intentionally verbose in order to
> demonstrate the concept as this is intended purely as an RFC.
>
> What if SETFLAGS returned EOPNOTSUPP if userspace is actually trying to
> *set* one of the unsettable flags (i.e. it isn't set already)? I believe
> this would therefore not break chattr(1), as flags that are retrieved
> from GETFLAGS can still be passed into SETFLAGS without error.
>
> If there is some other ABI compatibility that needs to be maintained
> that is broken by this, then please let me know. Also, I have not yet
> determined whether there are any concerns with calling f2fs_fileattr_get
> from inside f2fs_fileattr_set, e.g. speed/performance? so any thoughts
> would be greatly appreciated.
>
> Signed-off-by: Harry Austen <harryausten@hotmail.co.uk>
> ---
> fs/f2fs/file.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
This makes sense, but this ioctl isn't f2fs-specific; it's implemented by other
filesystems, most notably ext2/ext4 where it originated. f2fs shouldn't have
different behavior for the same ioctl. If you want to change this ioctl, you
need to start a wider discussion (including the linux-fsdevel and linux-ext4
mailing lists) where the change is proposed for all filesystems.
- Eric
next prev parent reply other threads:[~2022-01-31 19:35 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-30 17:21 [f2fs-dev] [RFC PATCH] f2fs: disallow setting unsettable file attributes Harry Austen
2022-01-30 17:21 ` Harry Austen
2022-01-31 19:34 ` Eric Biggers [this message]
2022-01-31 19:34 ` Eric Biggers
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=Yfg53VflpMY3eDB4@gmail.com \
--to=ebiggers@kernel.org \
--cc=harryausten@hotmail.co.uk \
--cc=jaegeuk@kernel.org \
--cc=linux-f2fs-devel@lists.sourceforge.net \
--cc=linux-kernel@vger.kernel.org \
/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.