Re: [PATCH 1/2] encrypt: rename _scratch_mkfs_encrypted to _scratch_check_encrypted

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Oct 26, 2022 at 03:04:17PM +0800, xiubli@xxxxxxxxxx wrote:
> From: Xiubo Li <xiubli@xxxxxxxxxx>
> 
> For ceph we couldn't check the encryption feature by mkfs, we need
> to mount it first and then check the 'get_encpolicy' ioctl cmd.
> 
> Signed-off-by: Xiubo Li <xiubli@xxxxxxxxxx>
> ---

This patch only does an *incomplete* function rename, without any change
on that function body, that doesn't make sense, and even will bring in
regression, due to lots of cases depend on this common function. If you
really need to change a common function, please "grep" [1] this function
in xfstests-dev/ to find out and check all places use it at least.

The _scratch_mkfs_encrypted is a "mkfs" function, likes _scratch_mkfs. I
think we shouldn't change its name. If you want to check if encryption
feature is supported by ceph, I think you might hope to do that in
_require_* functions, or you even can have a ceph specific function in
common/ceph. Just not the way you did in this patchset.

Thanks,
Zorro

[1]
$ grep -rsn _scratch_mkfs_encrypted .
./common/encrypt:32:    if ! _scratch_mkfs_encrypted &>>$seqres.full; then
./common/encrypt:146:_scratch_mkfs_encrypted()
./common/encrypt:174:# Like _scratch_mkfs_encrypted(), but add -O stable_inodes if applicable for the
./common/encrypt:186:           _scratch_mkfs_encrypted
./common/encrypt:926:           _scratch_mkfs_encrypted &>> $seqres.full
./common/verity:178:_scratch_mkfs_encrypted_verity()
./common/verity:190:            _notrun "$FSTYP not supported in _scratch_mkfs_encrypted_verity"
./tests/ext4/024:36:_scratch_mkfs_encrypted &>>$seqres.full
./tests/generic/395:22:_scratch_mkfs_encrypted &>> $seqres.full
./tests/generic/396:21:_scratch_mkfs_encrypted &>> $seqres.full
./tests/generic/580:23:_scratch_mkfs_encrypted &>> $seqres.full
./tests/generic/581:36:_scratch_mkfs_encrypted &>> $seqres.full
./tests/generic/595:35:_scratch_mkfs_encrypted &>> $seqres.full
./tests/generic/613:29:_scratch_mkfs_encrypted &>> $seqres.full
./tests/generic/621:57:_scratch_mkfs_encrypted &>> $seqres.full
./tests/generic/429:36:_scratch_mkfs_encrypted &>> $seqres.full
./tests/generic/397:28:_scratch_mkfs_encrypted &>> $seqres.full
./tests/generic/398:28:_scratch_mkfs_encrypted &>> $seqres.full
./tests/generic/421:24:_scratch_mkfs_encrypted &>> $seqres.full
./tests/generic/440:29:_scratch_mkfs_encrypted &>> $seqres.full
./tests/generic/419:29:_scratch_mkfs_encrypted &>> $seqres.full
./tests/generic/435:33:_scratch_mkfs_encrypted &>> $seqres.full
./tests/generic/593:24:_scratch_mkfs_encrypted &>> $seqres.full
./tests/generic/576:34:_scratch_mkfs_encrypted_verity &>> $seqres.full



>  common/encrypt | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/common/encrypt b/common/encrypt
> index 45ce0954..fd620c41 100644
> --- a/common/encrypt
> +++ b/common/encrypt
> @@ -29,7 +29,7 @@ _require_scratch_encryption()
>  	# Make a filesystem on the scratch device with the encryption feature
>  	# enabled.  If this fails then probably the userspace tools (e.g.
>  	# e2fsprogs or f2fs-tools) are too old to understand encryption.
> -	if ! _scratch_mkfs_encrypted &>>$seqres.full; then
> +	if ! _scratch_check_encrypted &>>$seqres.full; then
>  		_notrun "$FSTYP userspace tools do not support encryption"
>  	fi
>  
> @@ -143,7 +143,7 @@ _require_encryption_policy_support()
>  	rm -r $dir
>  }
>  
> -_scratch_mkfs_encrypted()
> +_scratch_check_encrypted()
>  {
>  	case $FSTYP in
>  	ext4|f2fs)
> @@ -171,7 +171,7 @@ _scratch_mkfs_sized_encrypted()
>  	esac
>  }
>  
> -# Like _scratch_mkfs_encrypted(), but add -O stable_inodes if applicable for the
> +# Like _scratch_check_encrypted(), but add -O stable_inodes if applicable for the
>  # filesystem type.  This is necessary for using encryption policies that include
>  # the inode number in the IVs, e.g. policies with the IV_INO_LBLK_64 flag set.
>  _scratch_mkfs_stable_inodes_encrypted()
> @@ -183,7 +183,7 @@ _scratch_mkfs_stable_inodes_encrypted()
>  		fi
>  		;;
>  	*)
> -		_scratch_mkfs_encrypted
> +		_scratch_check_encrypted
>  		;;
>  	esac
>  }
> @@ -923,7 +923,7 @@ _verify_ciphertext_for_encryption_policy()
>  			      FSCRYPT_POLICY_FLAG_IV_INO_LBLK_32) )); then
>  		_scratch_mkfs_stable_inodes_encrypted &>> $seqres.full
>  	else
> -		_scratch_mkfs_encrypted &>> $seqres.full
> +		_scratch_check_encrypted &>> $seqres.full
>  	fi
>  	_scratch_mount
>  
> -- 
> 2.31.1
> 




[Index of Archives]     [CEPH Users]     [Ceph Large]     [Ceph Dev]     [Information on CEPH]     [Linux BTRFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux