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

Ari Sundholm
2017-12-21T17:40:25Z

On 12/21/2017 03:52 PM, 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.
>
> 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.
>

I looked into implementing ubifs support for _scratch_mkfs_sized and
_scratch_mkfs_sized_encrypted. It doesn't seem there is a very good way
of doing this, apart from removing the existing volume and recreating
it, something like this:
ubirmvol $SCRATCH_DEV -n 0
ubimkvol $SCRATCH_DEV -n 0 -N foobar -s SIZE

This has the problem that we may not really know what the volume ID
should be. Can we trust it to be 0? The other issue is that $SCRATCH_DEV
probably points to a UBI volume instead of a UBI device. I guess ubinfo
(or just sed'ing out the volume suffix) could be used to get the
underlying UBI device.

I think supporting this should be in a separate patch.

Best regards,
Ari Sundholm
ari@tuxera.com

>> Cheers,
>>
>> Dave.
>>
>
> Thanks,
> Ari Sundholm
> ari@tuxera.com
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html