From: Zorro Lang <zlang@redhat.com>
To: xiubli@redhat.com
Cc: fstests@vger.kernel.org, david@fromorbit.com, djwong@kernel.org,
lhenriques@suse.de, ceph-devel@vger.kernel.org,
jlayton@kernel.org, mchangir@redhat.com
Subject: Re: [PATCH 1/2] encrypt: rename _scratch_mkfs_encrypted to _scratch_check_encrypted
Date: Wed, 26 Oct 2022 22:04:44 +0800 [thread overview]
Message-ID: <20221026140444.6br63mundxivfsnn@zlang-mailbox> (raw)
In-Reply-To: <20221026070418.259351-2-xiubli@redhat.com>
On Wed, Oct 26, 2022 at 03:04:17PM +0800, xiubli@redhat.com wrote:
> From: Xiubo Li <xiubli@redhat.com>
>
> 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@redhat.com>
> ---
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
>
next prev parent reply other threads:[~2022-10-26 14:05 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-26 7:04 [PATCH 0/2] encrypt: add ceph support xiubli
2022-10-26 7:04 ` [PATCH 1/2] encrypt: rename _scratch_mkfs_encrypted to _scratch_check_encrypted xiubli
2022-10-26 14:04 ` Zorro Lang [this message]
2022-10-27 2:19 ` Xiubo Li
2022-10-26 7:04 ` [PATCH 2/2] encrypt: add ceph support xiubli
2022-10-26 14:12 ` Zorro Lang
2022-10-27 2:22 ` Xiubo Li
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20221026140444.6br63mundxivfsnn@zlang-mailbox \
--to=zlang@redhat.com \
--cc=ceph-devel@vger.kernel.org \
--cc=david@fromorbit.com \
--cc=djwong@kernel.org \
--cc=fstests@vger.kernel.org \
--cc=jlayton@kernel.org \
--cc=lhenriques@suse.de \
--cc=mchangir@redhat.com \
--cc=xiubli@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.