| Message ID | 20191106055855.31517-1-amir73il@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | xfs_io/lsattr: expose FS_XFLAG_HASATTR flag | expand |
On Wed, Nov 06, 2019 at 07:58:55AM +0200, Amir Goldstein wrote: > For efficient check if file has xattrs. > > Signed-off-by: Amir Goldstein <amir73il@gmail.com> > --- > io/attr.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/io/attr.c b/io/attr.c > index b713d017..ba88ef16 100644 > --- a/io/attr.c > +++ b/io/attr.c > @@ -37,6 +37,7 @@ static struct xflags { > { FS_XFLAG_FILESTREAM, "S", "filestream" }, > { FS_XFLAG_DAX, "x", "dax" }, > { FS_XFLAG_COWEXTSIZE, "C", "cowextsize" }, > + { FS_XFLAG_HASATTR, "X", "has-xattr" }, > { 0, NULL, NULL } > }; > #define CHATTR_XFLAG_LIST "r"/*p*/"iasAdtPneEfSxC" /me wonders if this should have /*X*/ commented out the same way we do for "p". Otherwise, the patch looks ok to me... /me *also* wonders how many filesystems fail to implement this flag but support xattrs. Oh. All of them. Though I assume overlayfs is being patched... :) --D > @@ -65,6 +66,7 @@ lsattr_help(void) > " S -- enable filestreams allocator for this directory\n" > " x -- Use direct access (DAX) for data in this file\n" > " C -- for files with shared blocks, observe the inode CoW extent size value\n" > +" X -- file has extended attributes (cannot be changed using chattr)\n" > "\n" > " Options:\n" > " -R -- recursively descend (useful when current file is a directory)\n" > -- > 2.17.1 >
On Wed, Nov 6, 2019 at 6:03 PM Darrick J. Wong <darrick.wong@oracle.com> wrote: > > On Wed, Nov 06, 2019 at 07:58:55AM +0200, Amir Goldstein wrote: > > For efficient check if file has xattrs. > > > > Signed-off-by: Amir Goldstein <amir73il@gmail.com> > > --- > > io/attr.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/io/attr.c b/io/attr.c > > index b713d017..ba88ef16 100644 > > --- a/io/attr.c > > +++ b/io/attr.c > > @@ -37,6 +37,7 @@ static struct xflags { > > { FS_XFLAG_FILESTREAM, "S", "filestream" }, > > { FS_XFLAG_DAX, "x", "dax" }, > > { FS_XFLAG_COWEXTSIZE, "C", "cowextsize" }, > > + { FS_XFLAG_HASATTR, "X", "has-xattr" }, > > { 0, NULL, NULL } > > }; > > #define CHATTR_XFLAG_LIST "r"/*p*/"iasAdtPneEfSxC" > > /me wonders if this should have /*X*/ commented out the same way we do > for "p". Sure. Eric, please let me know if you want a re-submit for this. > > Otherwise, the patch looks ok to me... > > /me *also* wonders how many filesystems fail to implement this flag but > support xattrs. > > Oh. All of them. Though I assume overlayfs is being patched... :) > It doesn't need to be patched. It doesn't have xattr storage of its own. The ioctl is passed down to the underlying fs (*). (*) The ioctl is currently blocked on overlayfs directories. Thanks, Amir.
On 11/6/19 12:29 PM, Amir Goldstein wrote: > On Wed, Nov 6, 2019 at 6:03 PM Darrick J. Wong <darrick.wong@oracle.com> wrote: >> >> On Wed, Nov 06, 2019 at 07:58:55AM +0200, Amir Goldstein wrote: >>> For efficient check if file has xattrs. >>> >>> Signed-off-by: Amir Goldstein <amir73il@gmail.com> >>> --- >>> io/attr.c | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/io/attr.c b/io/attr.c >>> index b713d017..ba88ef16 100644 >>> --- a/io/attr.c >>> +++ b/io/attr.c >>> @@ -37,6 +37,7 @@ static struct xflags { >>> { FS_XFLAG_FILESTREAM, "S", "filestream" }, >>> { FS_XFLAG_DAX, "x", "dax" }, >>> { FS_XFLAG_COWEXTSIZE, "C", "cowextsize" }, >>> + { FS_XFLAG_HASATTR, "X", "has-xattr" }, >>> { 0, NULL, NULL } >>> }; >>> #define CHATTR_XFLAG_LIST "r"/*p*/"iasAdtPneEfSxC" >> >> /me wonders if this should have /*X*/ commented out the same way we do >> for "p". > > Sure. Eric, please let me know if you want a re-submit for this. Ummm I'll just stage it now and add it so I don't forget like: #define CHATTR_XFLAG_LIST "r"/*p*/"iasAdtPneEfS"/*X*/"xC" that, right? >> >> Otherwise, the patch looks ok to me... >> >> /me *also* wonders how many filesystems fail to implement this flag but >> support xattrs. >> >> Oh. All of them. Though I assume overlayfs is being patched... :) >> > > It doesn't need to be patched. It doesn't have xattr storage of its own. > The ioctl is passed down to the underlying fs (*). > (*) The ioctl is currently blocked on overlayfs directories. > > Thanks, > Amir. >
On Wed, Nov 06, 2019 at 12:45:55PM -0600, Eric Sandeen wrote: > On 11/6/19 12:29 PM, Amir Goldstein wrote: > > On Wed, Nov 6, 2019 at 6:03 PM Darrick J. Wong <darrick.wong@oracle.com> wrote: > >> > >> On Wed, Nov 06, 2019 at 07:58:55AM +0200, Amir Goldstein wrote: > >>> For efficient check if file has xattrs. > >>> > >>> Signed-off-by: Amir Goldstein <amir73il@gmail.com> > >>> --- > >>> io/attr.c | 2 ++ > >>> 1 file changed, 2 insertions(+) > >>> > >>> diff --git a/io/attr.c b/io/attr.c > >>> index b713d017..ba88ef16 100644 > >>> --- a/io/attr.c > >>> +++ b/io/attr.c > >>> @@ -37,6 +37,7 @@ static struct xflags { > >>> { FS_XFLAG_FILESTREAM, "S", "filestream" }, > >>> { FS_XFLAG_DAX, "x", "dax" }, > >>> { FS_XFLAG_COWEXTSIZE, "C", "cowextsize" }, > >>> + { FS_XFLAG_HASATTR, "X", "has-xattr" }, > >>> { 0, NULL, NULL } > >>> }; > >>> #define CHATTR_XFLAG_LIST "r"/*p*/"iasAdtPneEfSxC" > >> > >> /me wonders if this should have /*X*/ commented out the same way we do > >> for "p". > > > > Sure. Eric, please let me know if you want a re-submit for this. > > Ummm I'll just stage it now and add it so I don't forget > > like: > > #define CHATTR_XFLAG_LIST "r"/*p*/"iasAdtPneEfS"/*X*/"xC" > > that, right? Right. --D > >> > >> Otherwise, the patch looks ok to me... > >> > >> /me *also* wonders how many filesystems fail to implement this flag but > >> support xattrs. > >> > >> Oh. All of them. Though I assume overlayfs is being patched... :) > >> > > > > It doesn't need to be patched. It doesn't have xattr storage of its own. > > The ioctl is passed down to the underlying fs (*). > > (*) The ioctl is currently blocked on overlayfs directories. > > > > Thanks, > > Amir. > >
On 11/6/19 12:52 PM, Darrick J. Wong wrote: > On Wed, Nov 06, 2019 at 12:45:55PM -0600, Eric Sandeen wrote: >> On 11/6/19 12:29 PM, Amir Goldstein wrote: >>>>> #define CHATTR_XFLAG_LIST "r"/*p*/"iasAdtPneEfSxC" >>>> >>>> /me wonders if this should have /*X*/ commented out the same way we do >>>> for "p". >>> >>> Sure. Eric, please let me know if you want a re-submit for this. >> >> Ummm I'll just stage it now and add it so I don't forget >> >> like: >> >> #define CHATTR_XFLAG_LIST "r"/*p*/"iasAdtPneEfS"/*X*/"xC" >> >> that, right? > > Right. Actually for consistent ordering w/ the array, I guess maybe #define CHATTR_XFLAG_LIST "r"/*p*/"iasAdtPneEfSxC"/*X*/ Thanks -Eric
On Wed, Nov 06, 2019 at 12:53:46PM -0600, Eric Sandeen wrote: > > > On 11/6/19 12:52 PM, Darrick J. Wong wrote: > > On Wed, Nov 06, 2019 at 12:45:55PM -0600, Eric Sandeen wrote: > >> On 11/6/19 12:29 PM, Amir Goldstein wrote: > > >>>>> #define CHATTR_XFLAG_LIST "r"/*p*/"iasAdtPneEfSxC" > >>>> > >>>> /me wonders if this should have /*X*/ commented out the same way we do > >>>> for "p". > >>> > >>> Sure. Eric, please let me know if you want a re-submit for this. > >> > >> Ummm I'll just stage it now and add it so I don't forget > >> > >> like: > >> > >> #define CHATTR_XFLAG_LIST "r"/*p*/"iasAdtPneEfS"/*X*/"xC" > >> > >> that, right? > > > > Right. > > > Actually for consistent ordering w/ the array, I guess maybe > > #define CHATTR_XFLAG_LIST "r"/*p*/"iasAdtPneEfSxC"/*X*/ Sure. /me is done beating this hoss. Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> --D > Thanks > > -Eric
On Wed, Nov 06, 2019 at 07:58:55AM +0200, Amir Goldstein wrote: > For efficient check if file has xattrs. > > Signed-off-by: Amir Goldstein <amir73il@gmail.com> > --- > io/attr.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/io/attr.c b/io/attr.c > index b713d017..ba88ef16 100644 > --- a/io/attr.c > +++ b/io/attr.c > @@ -37,6 +37,7 @@ static struct xflags { > { FS_XFLAG_FILESTREAM, "S", "filestream" }, > { FS_XFLAG_DAX, "x", "dax" }, > { FS_XFLAG_COWEXTSIZE, "C", "cowextsize" }, > + { FS_XFLAG_HASATTR, "X", "has-xattr" }, This causes an xfs/207 regression on the extra letter in the output; can you please fix that? --D > { 0, NULL, NULL } > }; > #define CHATTR_XFLAG_LIST "r"/*p*/"iasAdtPneEfSxC" > @@ -65,6 +66,7 @@ lsattr_help(void) > " S -- enable filestreams allocator for this directory\n" > " x -- Use direct access (DAX) for data in this file\n" > " C -- for files with shared blocks, observe the inode CoW extent size value\n" > +" X -- file has extended attributes (cannot be changed using chattr)\n" > "\n" > " Options:\n" > " -R -- recursively descend (useful when current file is a directory)\n" > -- > 2.17.1 >
On 11/8/19 12:50 AM, Darrick J. Wong wrote: > On Wed, Nov 06, 2019 at 07:58:55AM +0200, Amir Goldstein wrote: >> For efficient check if file has xattrs. >> >> Signed-off-by: Amir Goldstein <amir73il@gmail.com> >> --- >> io/attr.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/io/attr.c b/io/attr.c >> index b713d017..ba88ef16 100644 >> --- a/io/attr.c >> +++ b/io/attr.c >> @@ -37,6 +37,7 @@ static struct xflags { >> { FS_XFLAG_FILESTREAM, "S", "filestream" }, >> { FS_XFLAG_DAX, "x", "dax" }, >> { FS_XFLAG_COWEXTSIZE, "C", "cowextsize" }, >> + { FS_XFLAG_HASATTR, "X", "has-xattr" }, > > This causes an xfs/207 regression on the extra letter in the output; can > you please fix that? I sent [PATCH] xfs/207: explicitly test for xflag character but forgot to cc xfs list. it's on the fstests list, sorry! -Eric
diff --git a/io/attr.c b/io/attr.c index b713d017..ba88ef16 100644 --- a/io/attr.c +++ b/io/attr.c @@ -37,6 +37,7 @@ static struct xflags { { FS_XFLAG_FILESTREAM, "S", "filestream" }, { FS_XFLAG_DAX, "x", "dax" }, { FS_XFLAG_COWEXTSIZE, "C", "cowextsize" }, + { FS_XFLAG_HASATTR, "X", "has-xattr" }, { 0, NULL, NULL } }; #define CHATTR_XFLAG_LIST "r"/*p*/"iasAdtPneEfSxC" @@ -65,6 +66,7 @@ lsattr_help(void) " S -- enable filestreams allocator for this directory\n" " x -- Use direct access (DAX) for data in this file\n" " C -- for files with shared blocks, observe the inode CoW extent size value\n" +" X -- file has extended attributes (cannot be changed using chattr)\n" "\n" " Options:\n" " -R -- recursively descend (useful when current file is a directory)\n"
For efficient check if file has xattrs. Signed-off-by: Amir Goldstein <amir73il@gmail.com> --- io/attr.c | 2 ++ 1 file changed, 2 insertions(+)