| Message ID | 20211207160437.184145-1-jlayton@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ceph: don't run tests if we can't set a custom layout | expand |
On Tue, Dec 07, 2021 at 11:04:37AM -0500, Jeff Layton wrote: > Some of the coming fscrypt patches prohibit non-default layout changes. > Skip running the tests that set custom layouts if setting the layout fails. > > Cc: Luis Henriques <lhenriques@suse.de> > Signed-off-by: Jeff Layton <jlayton@kernel.org> > --- > common/ceph | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/common/ceph b/common/ceph > index ca756dda8dd3..31b169af51b8 100644 > --- a/common/ceph > +++ b/common/ceph > @@ -19,7 +19,7 @@ _ceph_create_file_layout() > touch $fname > $SETFATTR_PROG -n ceph.file.layout \ > -v "stripe_unit=$objsz stripe_count=1 object_size=$objsz" \ > - $fname > + $fname || _notrun "setting ceph.file.layout failed" > } > > # this test requires to access file capabilities through vxattr 'ceph.caps'. > -- > 2.33.1 > Yeah, this is a simple way to skip those tests. Feel free to add my Reviewed-by: Luis Henriques <lhenriques@suse.de> Cheers, -- Luís
On Tue, Dec 07, 2021 at 11:04:37AM -0500, Jeff Layton wrote: > Some of the coming fscrypt patches prohibit non-default layout changes. > Skip running the tests that set custom layouts if setting the layout fails. > > Cc: Luis Henriques <lhenriques@suse.de> > Signed-off-by: Jeff Layton <jlayton@kernel.org> > --- > common/ceph | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/common/ceph b/common/ceph > index ca756dda8dd3..31b169af51b8 100644 > --- a/common/ceph > +++ b/common/ceph > @@ -19,7 +19,7 @@ _ceph_create_file_layout() > touch $fname > $SETFATTR_PROG -n ceph.file.layout \ > -v "stripe_unit=$objsz stripe_count=1 object_size=$objsz" \ > - $fname > + $fname || _notrun "setting ceph.file.layout failed" > } IMO _ceph_create_file_layout() is a helper function that does the real work, i.e. either prepares the test or does the test, not a function to check if current env & setup meets what the test needs. So I think it's better to check the ability to change layout explicitly in a new _require rule. e.g. something like _require_ceph_change_layout? Thanks, Eryu > > # this test requires to access file capabilities through vxattr 'ceph.caps'. > -- > 2.33.1
On Sun, 2021-12-12 at 22:02 +0800, Eryu Guan wrote: > On Tue, Dec 07, 2021 at 11:04:37AM -0500, Jeff Layton wrote: > > Some of the coming fscrypt patches prohibit non-default layout changes. > > Skip running the tests that set custom layouts if setting the layout fails. > > > > Cc: Luis Henriques <lhenriques@suse.de> > > Signed-off-by: Jeff Layton <jlayton@kernel.org> > > --- > > common/ceph | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/common/ceph b/common/ceph > > index ca756dda8dd3..31b169af51b8 100644 > > --- a/common/ceph > > +++ b/common/ceph > > @@ -19,7 +19,7 @@ _ceph_create_file_layout() > > touch $fname > > $SETFATTR_PROG -n ceph.file.layout \ > > -v "stripe_unit=$objsz stripe_count=1 object_size=$objsz" \ > > - $fname > > + $fname || _notrun "setting ceph.file.layout failed" > > } > > IMO _ceph_create_file_layout() is a helper function that does the real > work, i.e. either prepares the test or does the test, not a function to > check if current env & setup meets what the test needs. > > So I think it's better to check the ability to change layout explicitly > in a new _require rule. e.g. something like _require_ceph_change_layout? > > Thanks, > Eryu Good point. OTOH, relying on this program returning an error may be the wrong approach. What might be best is to just have a _require_not_encrypted check. That might be more universally useful anyway. Let me see what I can come up with. We can drop this patch for now. Thanks!
diff --git a/common/ceph b/common/ceph index ca756dda8dd3..31b169af51b8 100644 --- a/common/ceph +++ b/common/ceph @@ -19,7 +19,7 @@ _ceph_create_file_layout() touch $fname $SETFATTR_PROG -n ceph.file.layout \ -v "stripe_unit=$objsz stripe_count=1 object_size=$objsz" \ - $fname + $fname || _notrun "setting ceph.file.layout failed" } # this test requires to access file capabilities through vxattr 'ceph.caps'.
Some of the coming fscrypt patches prohibit non-default layout changes. Skip running the tests that set custom layouts if setting the layout fails. Cc: Luis Henriques <lhenriques@suse.de> Signed-off-by: Jeff Layton <jlayton@kernel.org> --- common/ceph | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)