| Message ID | 20230824-fixes-v2-2-d60c2faf1057@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fstests: add appropriate checks for fs features for some tests | expand |
On Thu, Aug 24, 2023 at 12:44:18PM -0400, Jeff Layton wrote: > This test requires being able to set file capabilities which some > filesystems (namely NFS) do not support. Add a _require_setcap test > and only run it on filesystems that pass it. > > Signed-off-by: Jeff Layton <jlayton@kernel.org> > --- > common/rc | 13 +++++++++++++ > tests/generic/513 | 1 + > 2 files changed, 14 insertions(+) > > diff --git a/common/rc b/common/rc > index 5c4429ed0425..33e74d20c28b 100644 > --- a/common/rc > +++ b/common/rc > @@ -5048,6 +5048,19 @@ _require_mknod() > rm -f $TEST_DIR/$seq.null > } > > +_require_setcap() > +{ > + local testfile=$TEST_DIR/setcaptest.$$ > + > + touch $testfile > + $SETCAP_PROG "cap_sys_module=p" $testfile > $testfile.out 2>&1 Actually we talked about the capabilities checking helper last year, as below: https://lore.kernel.org/fstests/20220323023845.saj5en74km7aibdx@zlang-mailbox/ As you bring this discussion back, how about the _require_capabilities() in above link? Thanks, Zorro > + if grep -q 'Operation not supported' $testfile.out; then > + _notrun "File capabilities are not supported on this filesystem" > + fi > + > + rm -f $testfile $testfile.out > +} > + > _getcap() > { > $GETCAP_PROG "$@" | _filter_getcap > diff --git a/tests/generic/513 b/tests/generic/513 > index dc082787ae4e..52f9eb916b4a 100755 > --- a/tests/generic/513 > +++ b/tests/generic/513 > @@ -18,6 +18,7 @@ _supported_fs generic > _require_scratch_reflink > _require_command "$GETCAP_PROG" getcap > _require_command "$SETCAP_PROG" setcap > +_require_setcap > > _scratch_mkfs >>$seqres.full 2>&1 > _scratch_mount > > -- > 2.41.0 >
On Fri, 2023-08-25 at 22:11 +0800, Zorro Lang wrote: > On Thu, Aug 24, 2023 at 12:44:18PM -0400, Jeff Layton wrote: > > This test requires being able to set file capabilities which some > > filesystems (namely NFS) do not support. Add a _require_setcap test > > and only run it on filesystems that pass it. > > > > Signed-off-by: Jeff Layton <jlayton@kernel.org> > > --- > > common/rc | 13 +++++++++++++ > > tests/generic/513 | 1 + > > 2 files changed, 14 insertions(+) > > > > diff --git a/common/rc b/common/rc > > index 5c4429ed0425..33e74d20c28b 100644 > > --- a/common/rc > > +++ b/common/rc > > @@ -5048,6 +5048,19 @@ _require_mknod() > > rm -f $TEST_DIR/$seq.null > > } > > > > +_require_setcap() > > +{ > > + local testfile=$TEST_DIR/setcaptest.$$ > > + > > + touch $testfile > > + $SETCAP_PROG "cap_sys_module=p" $testfile > $testfile.out 2>&1 > > Actually we talked about the capabilities checking helper last year, as below: > > https://lore.kernel.org/fstests/20220323023845.saj5en74km7aibdx@zlang-mailbox/ > > As you bring this discussion back, how about the _require_capabilities() in > above link? > I was testing a similar patch, but your version looks better. Should I drop mine and you re-post yours? Thanks, -- Jeff Layton <jlayton@kernel.org>
On Fri, Aug 25, 2023 at 11:02:40AM -0400, Jeff Layton wrote: > On Fri, 2023-08-25 at 22:11 +0800, Zorro Lang wrote: > > On Thu, Aug 24, 2023 at 12:44:18PM -0400, Jeff Layton wrote: > > > This test requires being able to set file capabilities which some > > > filesystems (namely NFS) do not support. Add a _require_setcap test > > > and only run it on filesystems that pass it. > > > > > > Signed-off-by: Jeff Layton <jlayton@kernel.org> > > > --- > > > common/rc | 13 +++++++++++++ > > > tests/generic/513 | 1 + > > > 2 files changed, 14 insertions(+) > > > > > > diff --git a/common/rc b/common/rc > > > index 5c4429ed0425..33e74d20c28b 100644 > > > --- a/common/rc > > > +++ b/common/rc > > > @@ -5048,6 +5048,19 @@ _require_mknod() > > > rm -f $TEST_DIR/$seq.null > > > } > > > > > > +_require_setcap() > > > +{ > > > + local testfile=$TEST_DIR/setcaptest.$$ > > > + > > > + touch $testfile > > > + $SETCAP_PROG "cap_sys_module=p" $testfile > $testfile.out 2>&1 > > > > Actually we talked about the capabilities checking helper last year, as below: > > > > https://lore.kernel.org/fstests/20220323023845.saj5en74km7aibdx@zlang-mailbox/ > > > > As you bring this discussion back, how about the _require_capabilities() in > > above link? > > > > I was testing a similar patch, but your version looks better. Should I > drop mine and you re-post yours? Actually we decided to use `_require_attrs security`, rather than a new _require_capabilities() helper. We need a chance/requirement to add that helper (when a test case really need it). So I hope know is `_require_attrs security` enough for you? Or you really need a specific _require_capabilities helper? Thanks, Zorro > > Thanks, > -- > Jeff Layton <jlayton@kernel.org> >
On Sun, 2023-08-27 at 20:45 +0800, Zorro Lang wrote: > On Fri, Aug 25, 2023 at 11:02:40AM -0400, Jeff Layton wrote: > > On Fri, 2023-08-25 at 22:11 +0800, Zorro Lang wrote: > > > On Thu, Aug 24, 2023 at 12:44:18PM -0400, Jeff Layton wrote: > > > > This test requires being able to set file capabilities which some > > > > filesystems (namely NFS) do not support. Add a _require_setcap test > > > > and only run it on filesystems that pass it. > > > > > > > > Signed-off-by: Jeff Layton <jlayton@kernel.org> > > > > --- > > > > common/rc | 13 +++++++++++++ > > > > tests/generic/513 | 1 + > > > > 2 files changed, 14 insertions(+) > > > > > > > > diff --git a/common/rc b/common/rc > > > > index 5c4429ed0425..33e74d20c28b 100644 > > > > --- a/common/rc > > > > +++ b/common/rc > > > > @@ -5048,6 +5048,19 @@ _require_mknod() > > > > rm -f $TEST_DIR/$seq.null > > > > } > > > > > > > > +_require_setcap() > > > > +{ > > > > + local testfile=$TEST_DIR/setcaptest.$$ > > > > + > > > > + touch $testfile > > > > + $SETCAP_PROG "cap_sys_module=p" $testfile > $testfile.out 2>&1 > > > > > > Actually we talked about the capabilities checking helper last year, as below: > > > > > > https://lore.kernel.org/fstests/20220323023845.saj5en74km7aibdx@zlang-mailbox/ > > > > > > As you bring this discussion back, how about the _require_capabilities() in > > > above link? > > > > > > > I was testing a similar patch, but your version looks better. Should I > > drop mine and you re-post yours? > > Actually we decided to use `_require_attrs security`, rather than a new > _require_capabilities() helper. We need a chance/requirement to add > that helper (when a test case really need it). > > So I hope know is `_require_attrs security` enough for you? Or you really > need a specific _require_capabilities helper? > > Thanks, > Zorro > > Yeah, it looks like that should work: generic/513 [not run] attr namespace security not supported by this filesystem type: nfs I'll plan to respin this patch to add that instead. Thanks!
On Sun, Aug 27, 2023 at 09:43:41AM -0400, Jeff Layton wrote: > On Sun, 2023-08-27 at 20:45 +0800, Zorro Lang wrote: > > On Fri, Aug 25, 2023 at 11:02:40AM -0400, Jeff Layton wrote: > > > On Fri, 2023-08-25 at 22:11 +0800, Zorro Lang wrote: > > > > On Thu, Aug 24, 2023 at 12:44:18PM -0400, Jeff Layton wrote: > > > > > This test requires being able to set file capabilities which some > > > > > filesystems (namely NFS) do not support. Add a _require_setcap test > > > > > and only run it on filesystems that pass it. > > > > > > > > > > Signed-off-by: Jeff Layton <jlayton@kernel.org> > > > > > --- > > > > > common/rc | 13 +++++++++++++ > > > > > tests/generic/513 | 1 + > > > > > 2 files changed, 14 insertions(+) > > > > > > > > > > diff --git a/common/rc b/common/rc > > > > > index 5c4429ed0425..33e74d20c28b 100644 > > > > > --- a/common/rc > > > > > +++ b/common/rc > > > > > @@ -5048,6 +5048,19 @@ _require_mknod() > > > > > rm -f $TEST_DIR/$seq.null > > > > > } > > > > > > > > > > +_require_setcap() > > > > > +{ > > > > > + local testfile=$TEST_DIR/setcaptest.$$ > > > > > + > > > > > + touch $testfile > > > > > + $SETCAP_PROG "cap_sys_module=p" $testfile > $testfile.out 2>&1 > > > > > > > > Actually we talked about the capabilities checking helper last year, as below: > > > > > > > > https://lore.kernel.org/fstests/20220323023845.saj5en74km7aibdx@zlang-mailbox/ > > > > > > > > As you bring this discussion back, how about the _require_capabilities() in > > > > above link? > > > > > > > > > > I was testing a similar patch, but your version looks better. Should I > > > drop mine and you re-post yours? > > > > Actually we decided to use `_require_attrs security`, rather than a new > > _require_capabilities() helper. We need a chance/requirement to add > > that helper (when a test case really need it). > > > > So I hope know is `_require_attrs security` enough for you? Or you really > > need a specific _require_capabilities helper? > > > > Thanks, > > Zorro > > > > > > > Yeah, it looks like that should work: > > generic/513 [not run] attr namespace security not supported by this filesystem type: nfs > > I'll plan to respin this patch to add that instead. Thanks:) I saw you've sent a V3: [PATCH fstests v3 0/2] fstests: add appropriate checks for fs features for some tests so will you sent a V4 contains 3 patches? Thanks, Zorro > > Thanks! > -- > Jeff Layton <jlayton@kernel.org> >
diff --git a/common/rc b/common/rc index 5c4429ed0425..33e74d20c28b 100644 --- a/common/rc +++ b/common/rc @@ -5048,6 +5048,19 @@ _require_mknod() rm -f $TEST_DIR/$seq.null } +_require_setcap() +{ + local testfile=$TEST_DIR/setcaptest.$$ + + touch $testfile + $SETCAP_PROG "cap_sys_module=p" $testfile > $testfile.out 2>&1 + if grep -q 'Operation not supported' $testfile.out; then + _notrun "File capabilities are not supported on this filesystem" + fi + + rm -f $testfile $testfile.out +} + _getcap() { $GETCAP_PROG "$@" | _filter_getcap diff --git a/tests/generic/513 b/tests/generic/513 index dc082787ae4e..52f9eb916b4a 100755 --- a/tests/generic/513 +++ b/tests/generic/513 @@ -18,6 +18,7 @@ _supported_fs generic _require_scratch_reflink _require_command "$GETCAP_PROG" getcap _require_command "$SETCAP_PROG" setcap +_require_setcap _scratch_mkfs >>$seqres.full 2>&1 _scratch_mount
This test requires being able to set file capabilities which some filesystems (namely NFS) do not support. Add a _require_setcap test and only run it on filesystems that pass it. Signed-off-by: Jeff Layton <jlayton@kernel.org> --- common/rc | 13 +++++++++++++ tests/generic/513 | 1 + 2 files changed, 14 insertions(+)