On Thu, Dec 21, 2017 at 03:52:00PM +0200, Ari Sundholm wrote:
> Hi!
>
> Thank you for your comments. Please see below.
>
> On 12/21/2017 03:38 AM, Dave Chinner wrote:
> >On Wed, Dec 20, 2017 at 07:46:40PM +0200, Ari Sundholm wrote:
> >>Test case generic/399 hardcodes "-O encrypt" in MKFS_OPTIONS when
> >>calling _scratch_mkfs_sized, which only works with the mkfs of certain
> >>filesystems. Create a new helper, _scratch_mkfs_sized_encrypted, for
> >>handling the differences between the mkfs tools of different
> >>filesystems. It also allows those filesystems whose mkfs doesn't accept
> >>"-O encrypt" to skip the test gracefully until proper support is added
> >>for them in the helper.
> >>
> >>Signed-off-by: Ari Sundholm <ari@xxxxxxxxxx>
> >>---
> >> common/encrypt | 12 ++++++++++++
> >> tests/generic/399 | 3 +--
> >> 2 files changed, 13 insertions(+), 2 deletions(-)
> >>
> >>diff --git a/common/encrypt b/common/encrypt
> >>index a6fd89d..189c59e 100644
> >>--- a/common/encrypt
> >>+++ b/common/encrypt
> >>@@ -81,6 +81,18 @@ _scratch_mkfs_encrypted()
> >> esac
> >> }
> >>+_scratch_mkfs_sized_encrypted()
> >>+{
> >>+ case $FSTYP in
> >>+ ext4|f2fs)
> >>+ MKFS_OPTIONS="$MKFS_OPTIONS -O encrypt" _scratch_mkfs_sized $*
> >>+ ;;
> >
> >This does not need to screw around with MKFS_OPTIONS. This:
> >
> > _scratch_mkfs_sized -O encrypt $*
> >
> >Will do just fine.
>
> Hmm, I don't see how that could work. At the moment,
> _scratch_mkfs_sized only takes and uses two arguments, one of which
> is optional. AFAICS, all additional mkfs options need to be passed
> using MKFS_OPTIONS to _scratch_mkfs_sized.
Oh, I was under the impression that got fixed some time ago.
Screwing with MKFS_OPTIONS means defeats some of the test specific
mkfs option conflict resolution that some filesystem have.
i.e. when the options specified by the test cause problems with test
run specified MKFS_OPTIONS, the MKFS_OPTIONS get dropped and just
the test specific options are used. Setting random test options in
MKFS_OPTIONS can cause _scratch_mkfs_sized to not use the options
specified by the test at all...
> As for using $* instead of $1 $2, I am hoping that will avoid having
> to change this new function even if the usage of _scratch_mkfs_sized
> changes.
>
> >Also, _scratch_mkfs_encrypted() supports UBIFS, and this new
> >function doesn't. Seems like it should to me...
>
> _scratch_mkfs_sized does not support ubifs, though, so the operation
> would _notrun anyway. I'm not familiar with ubifs or its tools, but
> I'll see if I can find out how to implement support for ubifs in
> _scratch_mkfs_sized.
Ok, that should be mentioned in the commit message so people know
why it isn't supported despite supporting encryption...
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe fstests" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
