Re: [PATCH] common/encrypt: Create an encrypted equivalent of _scratch_mkfs_sized

[Eryu Guan <eguan@redhat.com>]

On Fri, Dec 22, 2017 at 08:46:57AM +1100, Dave Chinner wrote:
> 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@tuxera.com>
> > >>---
> > >>  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...

JFYI, that was fixed in the _scratch_mkfs helper, but not the _sized
one, the fs size handling makes it special and not work well with the
generic mkfs helpers.

Thanks,
Eryu