On 12/21/2017 11:46 PM, 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@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...
Yes, we were actually bitten by this a while ago. It is a bit tricky to
solve this for _scratch_mkfs_sized given that it has an optional
argument that we do not want to confuse with the extra arguments the
test case specifies. For this reason it can't work like _scratch_mkfs does.
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...
Fair enough. Sent v2 with an amended commit message.
Cheers,
Dave.
Thanks,
Ari Sundholm
ari@xxxxxxxxxx
--
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
