* [PATCH v3] common/rc: add filter in _test_inode_flag
@ 2021-02-18 8:57 XiaoLi Feng
2021-02-18 13:41 ` Zorro Lang
0 siblings, 1 reply; 5+ messages in thread
From: XiaoLi Feng @ 2021-02-18 8:57 UTC (permalink / raw)
To: fstests; +Cc: Xiaoli Feng
From: Xiaoli Feng <xifeng@redhat.com>
Add _filter_testdir_and_scratch to avoid _test_inode_flag failed when
mount point contains flag string.
Signed-off-by: Xiaoli Feng <xifeng@redhat.com>
---
common/rc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/rc b/common/rc
index 649b1cfd..473188c1 100644
--- a/common/rc
+++ b/common/rc
@@ -3112,7 +3112,7 @@ _test_inode_flag()
local flag=$1
local file=$2
- if $XFS_IO_PROG -r -c 'lsattr -v' "$file" | grep -q "$flag" ; then
+ if $XFS_IO_PROG -r -c 'lsattr -v' "$file" | _filter_testdir_and_scratch | grep -q "$flag" ; then
return 0
fi
return 1
--
2.18.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v3] common/rc: add filter in _test_inode_flag
2021-02-18 8:57 [PATCH v3] common/rc: add filter in _test_inode_flag XiaoLi Feng
@ 2021-02-18 13:41 ` Zorro Lang
2021-02-18 13:41 ` Zirong Lang
2021-02-19 16:00 ` Xiaoli Feng
0 siblings, 2 replies; 5+ messages in thread
From: Zorro Lang @ 2021-02-18 13:41 UTC (permalink / raw)
To: XiaoLi Feng; +Cc: fstests
On Thu, Feb 18, 2021 at 04:57:52PM +0800, XiaoLi Feng wrote:
> From: Xiaoli Feng <xifeng@redhat.com>
>
> Add _filter_testdir_and_scratch to avoid _test_inode_flag failed when
> mount point contains flag string.
>
> Signed-off-by: Xiaoli Feng <xifeng@redhat.com>
> ---
> common/rc | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/common/rc b/common/rc
> index 649b1cfd..473188c1 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -3112,7 +3112,7 @@ _test_inode_flag()
> local flag=$1
> local file=$2
>
> - if $XFS_IO_PROG -r -c 'lsattr -v' "$file" | grep -q "$flag" ; then
> + if $XFS_IO_PROG -r -c 'lsattr -v' "$file" | _filter_testdir_and_scratch | grep -q "$flag" ; then
I can't understand that. Could you give us an example about why we need this
filter before "quiet grep"? I think this line doesn't have any output
(except xfs_io fails), so why a filter is needed?
Thanks,
Zorro
> return 0
> fi
> return 1
> --
> 2.18.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] common/rc: add filter in _test_inode_flag
2021-02-18 13:41 ` Zorro Lang
@ 2021-02-18 13:41 ` Zirong Lang
2021-02-19 16:00 ` Xiaoli Feng
1 sibling, 0 replies; 5+ messages in thread
From: Zirong Lang @ 2021-02-18 13:41 UTC (permalink / raw)
To: XiaoLi Feng; +Cc: fstests
----- 原始邮件 -----
> 发件人: "Zorro Lang" <zlang@redhat.com>
> 收件人: "XiaoLi Feng" <xifeng@redhat.com>
> 抄送: fstests@vger.kernel.org
> 发送时间: 星期四, 2021年 2 月 18日 下午 9:41:34
> 主题: Re: [PATCH v3] common/rc: add filter in _test_inode_flag
>
> On Thu, Feb 18, 2021 at 04:57:52PM +0800, XiaoLi Feng wrote:
> > From: Xiaoli Feng <xifeng@redhat.com>
> >
> > Add _filter_testdir_and_scratch to avoid _test_inode_flag failed when
> > mount point contains flag string.
> >
> > Signed-off-by: Xiaoli Feng <xifeng@redhat.com>
> > ---
> > common/rc | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/common/rc b/common/rc
> > index 649b1cfd..473188c1 100644
> > --- a/common/rc
> > +++ b/common/rc
> > @@ -3112,7 +3112,7 @@ _test_inode_flag()
> > local flag=$1
> > local file=$2
> >
> > - if $XFS_IO_PROG -r -c 'lsattr -v' "$file" | grep -q "$flag" ; then
> > + if $XFS_IO_PROG -r -c 'lsattr -v' "$file" | _filter_testdir_and_scratch |
> > grep -q "$flag" ; then
>
> I can't understand that. Could you give us an example about why we need this
> filter before "quiet grep"? I think this line doesn't have any output
> (except xfs_io fails), so why a filter is needed?
Oh, I just saw your reply for V1 patch review:
"When test generic/608 for dax on xfs, "_check_xflag $t_file 0" is always
failed if the $f_file has dax string. Yes, here should also include filter
for TEST_DIR."
The _check_xflag() function as below:
_check_xflag()
{
local target=$1
local exp_xflag=$2
if [ $exp_xflag -eq 0 ]; then
_test_inode_flag dax $target && echo "$target has unexpected FS_XFLAG_DAX flag"
else
_test_inode_flag dax $target || echo "$target doesn't have expected FS_XFLAG_DAX flag"
fi
}
Sure, if the $1 is something likes "/mnt/scratch/file", later echo "$target ...." will print "/mnt/scratch".
If you're trying to fix this issue, I think you should filter the "$target" in or behind _check_xflag(),
not in _test_inode_flag() which has no output.
Thanks,
Zorro
>
> Thanks,
> Zorro
>
> > return 0
> > fi
> > return 1
> > --
> > 2.18.1
> >
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] common/rc: add filter in _test_inode_flag
2021-02-18 13:41 ` Zorro Lang
2021-02-18 13:41 ` Zirong Lang
@ 2021-02-19 16:00 ` Xiaoli Feng
2021-02-19 16:54 ` Zorro Lang
1 sibling, 1 reply; 5+ messages in thread
From: Xiaoli Feng @ 2021-02-19 16:00 UTC (permalink / raw)
To: Zorro Lang; +Cc: fstests
Hi,
----- Original Message -----
> From: "Zorro Lang" <zlang@redhat.com>
> To: "XiaoLi Feng" <xifeng@redhat.com>
> Cc: fstests@vger.kernel.org
> Sent: Thursday, February 18, 2021 9:41:35 PM
> Subject: Re: [PATCH v3] common/rc: add filter in _test_inode_flag
>
> On Thu, Feb 18, 2021 at 04:57:52PM +0800, XiaoLi Feng wrote:
> > From: Xiaoli Feng <xifeng@redhat.com>
> >
> > Add _filter_testdir_and_scratch to avoid _test_inode_flag failed when
> > mount point contains flag string.
> >
> > Signed-off-by: Xiaoli Feng <xifeng@redhat.com>
> > ---
> > common/rc | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/common/rc b/common/rc
> > index 649b1cfd..473188c1 100644
> > --- a/common/rc
> > +++ b/common/rc
> > @@ -3112,7 +3112,7 @@ _test_inode_flag()
> > local flag=$1
> > local file=$2
> >
> > - if $XFS_IO_PROG -r -c 'lsattr -v' "$file" | grep -q "$flag" ; then
> > + if $XFS_IO_PROG -r -c 'lsattr -v' "$file" | _filter_testdir_and_scratch |
> > grep -q "$flag" ; then
>
> I can't understand that. Could you give us an example about why we need this
> filter before "quiet grep"? I think this line doesn't have any output
> (except xfs_io fails), so why a filter is needed?
Yes. this line doesn't have any output. But the return value is different. I send
this patch to fix the issue: If testdir or scratch_dir has "$flag". Then it will
always return 0. Even if this file doesn't have this "$flag".
Example:
xfs_io -r -c 'lsattr -v' /daxmnt/fileB
[] /daxmnt/fileB
# xfs_io -r -c 'lsattr -v' /daxmnt/fileB | grep -q "dax"
# echo $?
0
fileB doesn't have dax flag. But _test_inode_flag will always return 0(expect 1).
Thanks.
>
> Thanks,
> Zorro
>
> > return 0
> > fi
> > return 1
> > --
> > 2.18.1
> >
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] common/rc: add filter in _test_inode_flag
2021-02-19 16:00 ` Xiaoli Feng
@ 2021-02-19 16:54 ` Zorro Lang
0 siblings, 0 replies; 5+ messages in thread
From: Zorro Lang @ 2021-02-19 16:54 UTC (permalink / raw)
To: Xiaoli Feng; +Cc: fstests
On Fri, Feb 19, 2021 at 11:00:18AM -0500, Xiaoli Feng wrote:
> Hi,
>
> ----- Original Message -----
> > From: "Zorro Lang" <zlang@redhat.com>
> > To: "XiaoLi Feng" <xifeng@redhat.com>
> > Cc: fstests@vger.kernel.org
> > Sent: Thursday, February 18, 2021 9:41:35 PM
> > Subject: Re: [PATCH v3] common/rc: add filter in _test_inode_flag
> >
> > On Thu, Feb 18, 2021 at 04:57:52PM +0800, XiaoLi Feng wrote:
> > > From: Xiaoli Feng <xifeng@redhat.com>
> > >
> > > Add _filter_testdir_and_scratch to avoid _test_inode_flag failed when
> > > mount point contains flag string.
> > >
> > > Signed-off-by: Xiaoli Feng <xifeng@redhat.com>
> > > ---
> > > common/rc | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/common/rc b/common/rc
> > > index 649b1cfd..473188c1 100644
> > > --- a/common/rc
> > > +++ b/common/rc
> > > @@ -3112,7 +3112,7 @@ _test_inode_flag()
> > > local flag=$1
> > > local file=$2
> > >
> > > - if $XFS_IO_PROG -r -c 'lsattr -v' "$file" | grep -q "$flag" ; then
> > > + if $XFS_IO_PROG -r -c 'lsattr -v' "$file" | _filter_testdir_and_scratch |
> > > grep -q "$flag" ; then
> >
> > I can't understand that. Could you give us an example about why we need this
> > filter before "quiet grep"? I think this line doesn't have any output
> > (except xfs_io fails), so why a filter is needed?
>
> Yes. this line doesn't have any output. But the return value is different. I send
> this patch to fix the issue: If testdir or scratch_dir has "$flag". Then it will
> always return 0. Even if this file doesn't have this "$flag".
>
> Example:
> xfs_io -r -c 'lsattr -v' /daxmnt/fileB
> [] /daxmnt/fileB
> # xfs_io -r -c 'lsattr -v' /daxmnt/fileB | grep -q "dax"
> # echo $?
> 0
>
> fileB doesn't have dax flag. But _test_inode_flag will always return 0(expect 1).
Oh, got it. So the 'grep -q "dax"' is a litte inaccurate. If the mountpoint or
file name, or any sub-string in the $file contains "dax", this function will
return 0.
If that's the issue you're fixing, I think only filter SCRATCH_MNT and TEST_MNT
isn't enough. If we only care about the "attr" part output, how about filter
out junk (path name), only keep the [attr]?
Due to looks like the output format of 'lsattr -v' is: "[$attr] $filename" (correct
me if I'm wrong). So if we only keep the attr part, that can avoid the 'grep' find
"dax" from $filename.
Thanks,
Zorro
>
> Thanks.
>
> >
> > Thanks,
> > Zorro
> >
> > > return 0
> > > fi
> > > return 1
> > > --
> > > 2.18.1
> > >
> >
> >
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-02-19 16:38 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-18 8:57 [PATCH v3] common/rc: add filter in _test_inode_flag XiaoLi Feng
2021-02-18 13:41 ` Zorro Lang
2021-02-18 13:41 ` Zirong Lang
2021-02-19 16:00 ` Xiaoli Feng
2021-02-19 16:54 ` Zorro Lang
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.