| Message ID | 20210309050124.23797-4-chandanrlinux@gmail.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | xfs: Tests to verify inode fork extent count overflow detection | expand |
On Tue, Mar 09, 2021 at 10:31:14AM +0530, Chandan Babu R wrote: > This commit adds a helper function to obtain the value of a particular field > of an inode's fsxattr fields. > > Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com> Seems reasonable, Reviewed-by: Darrick J. Wong <djwong@kernel.org> --D > --- > common/xfs | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/common/xfs b/common/xfs > index 26ae21b9..130b3232 100644 > --- a/common/xfs > +++ b/common/xfs > @@ -194,6 +194,15 @@ _xfs_get_file_block_size() > $XFS_INFO_PROG "$path" | grep realtime | sed -e 's/^.*extsz=\([0-9]*\).*$/\1/g' > } > > +_xfs_get_fsxattr() > +{ > + local field="$1" > + local path="$2" > + > + local value=$($XFS_IO_PROG -c "stat" "$path" | grep "$field") > + echo ${value##fsxattr.${field} = } > +} > + > # xfs_check script is planned to be deprecated. But, we want to > # be able to invoke "xfs_check" behavior in xfstests in order to > # maintain the current verification levels. > -- > 2.29.2 >
On 3/8/21 10:01 PM, Chandan Babu R wrote: > This commit adds a helper function to obtain the value of a particular field > of an inode's fsxattr fields. > > Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com> > --- > common/xfs | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/common/xfs b/common/xfs > index 26ae21b9..130b3232 100644 > --- a/common/xfs > +++ b/common/xfs > @@ -194,6 +194,15 @@ _xfs_get_file_block_size() > $XFS_INFO_PROG "$path" | grep realtime | sed -e 's/^.*extsz=\([0-9]*\).*$/\1/g' > } > > +_xfs_get_fsxattr() > +{ > + local field="$1" > + local path="$2" > + > + local value=$($XFS_IO_PROG -c "stat" "$path" | grep "$field") > + echo ${value##fsxattr.${field} = } > +} > + In fiddling with the commands here, I think I may have noticed a bug. I think you want to grep whole words only, or you may mistakenly match sub words. Example: root@garnet:/home/achender/work_area# field="extsize " root@garnet:/home/achender/work_area# xfs_io -c "stat" /mnt/scratch/test | grep "$field" fsxattr.extsize = 0 fsxattr.cowextsize = 0 I think if you add the -w to the grep that fixes it: root@garnet:/home/achender/work_area# xfs_io -c "stat" /mnt/scratch/test | grep -w "$field" fsxattr.extsize = 0 I think that's what you meant to do right? Allison > # xfs_check script is planned to be deprecated. But, we want to > # be able to invoke "xfs_check" behavior in xfstests in order to > # maintain the current verification levels. >
On Tue, Mar 09, 2021 at 11:13:17PM -0700, Allison Henderson wrote: > > > On 3/8/21 10:01 PM, Chandan Babu R wrote: > > This commit adds a helper function to obtain the value of a particular field > > of an inode's fsxattr fields. > > > > Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com> > > --- > > common/xfs | 9 +++++++++ > > 1 file changed, 9 insertions(+) > > > > diff --git a/common/xfs b/common/xfs > > index 26ae21b9..130b3232 100644 > > --- a/common/xfs > > +++ b/common/xfs > > @@ -194,6 +194,15 @@ _xfs_get_file_block_size() > > $XFS_INFO_PROG "$path" | grep realtime | sed -e 's/^.*extsz=\([0-9]*\).*$/\1/g' > > } > > +_xfs_get_fsxattr() > > +{ > > + local field="$1" > > + local path="$2" > > + > > + local value=$($XFS_IO_PROG -c "stat" "$path" | grep "$field") > > + echo ${value##fsxattr.${field} = } > > +} > > + > In fiddling with the commands here, I think I may have noticed a bug. I > think you want to grep whole words only, or you may mistakenly match sub > words. Example: > > root@garnet:/home/achender/work_area# field="extsize " > root@garnet:/home/achender/work_area# xfs_io -c "stat" /mnt/scratch/test | > grep "$field" > fsxattr.extsize = 0 > fsxattr.cowextsize = 0 > > I think if you add the -w to the grep that fixes it: > root@garnet:/home/achender/work_area# xfs_io -c "stat" /mnt/scratch/test | > grep -w "$field" Hey, that's a neat trick I didn't know about! Oooh and it even exists in both busybox /and/ gnu grep! :) /me madly goes digging through his shell scripts <and that was the last we heard from djwong> --D > fsxattr.extsize = 0 > > I think that's what you meant to do right? > > Allison > > > # xfs_check script is planned to be deprecated. But, we want to > > # be able to invoke "xfs_check" behavior in xfstests in order to > > # maintain the current verification levels. > > >
On 10 Mar 2021 at 11:43, Allison Henderson wrote: > On 3/8/21 10:01 PM, Chandan Babu R wrote: >> This commit adds a helper function to obtain the value of a particular field >> of an inode's fsxattr fields. >> >> Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com> >> --- >> common/xfs | 9 +++++++++ >> 1 file changed, 9 insertions(+) >> >> diff --git a/common/xfs b/common/xfs >> index 26ae21b9..130b3232 100644 >> --- a/common/xfs >> +++ b/common/xfs >> @@ -194,6 +194,15 @@ _xfs_get_file_block_size() >> $XFS_INFO_PROG "$path" | grep realtime | sed -e 's/^.*extsz=\([0-9]*\).*$/\1/g' >> } >> +_xfs_get_fsxattr() >> +{ >> + local field="$1" >> + local path="$2" >> + >> + local value=$($XFS_IO_PROG -c "stat" "$path" | grep "$field") >> + echo ${value##fsxattr.${field} = } >> +} >> + > In fiddling with the commands here, I think I may have noticed a bug. > I think you want to grep whole words only, or you may mistakenly match > sub words. Example: > > root@garnet:/home/achender/work_area# field="extsize " > root@garnet:/home/achender/work_area# xfs_io -c "stat" > /mnt/scratch/test | grep "$field" > fsxattr.extsize = 0 > fsxattr.cowextsize = 0 > > I think if you add the -w to the grep that fixes it: > root@garnet:/home/achender/work_area# xfs_io -c "stat" > /mnt/scratch/test | grep -w "$field" > fsxattr.extsize = 0 > > I think that's what you meant to do right? > Yes, that was the intended behaviour. Thanks for catching the bug and suggesting the appropriate solution. I will fix this. Also, Thanks for reviewing the entire patchset. -- chandan
diff --git a/common/xfs b/common/xfs index 26ae21b9..130b3232 100644 --- a/common/xfs +++ b/common/xfs @@ -194,6 +194,15 @@ _xfs_get_file_block_size() $XFS_INFO_PROG "$path" | grep realtime | sed -e 's/^.*extsz=\([0-9]*\).*$/\1/g' } +_xfs_get_fsxattr() +{ + local field="$1" + local path="$2" + + local value=$($XFS_IO_PROG -c "stat" "$path" | grep "$field") + echo ${value##fsxattr.${field} = } +} + # xfs_check script is planned to be deprecated. But, we want to # be able to invoke "xfs_check" behavior in xfstests in order to # maintain the current verification levels.
This commit adds a helper function to obtain the value of a particular field of an inode's fsxattr fields. Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com> --- common/xfs | 9 +++++++++ 1 file changed, 9 insertions(+)