[PATCH 00/12] fstests: fscrypt test updates

[PATCH 00/12] fstests: fscrypt test updates

[Josef Bacik]

Hello,

Btrfs is adding fscrypt support, and thus requires a variety of changes to the
current fscrypt tests and infrastructure, as well as adding a few extra tests.

The bulk of the changes to the existing tests is simply breaking the v1 and v2
policy tests into two different tests, as btrfs will only support v2 policies.
The infrastructure related work is around pulling the nonce's out of the file
system in order to support the different nonce/decryption related checks.

And finally there are 3 new tests, two around reflinks and snapshots and then a
generic fsstress test.

I've tested these with ext4 and btrfs (with our patches) to make sure everything
works properly.  Thanks,

Josef

Josef Bacik (5):
  fstests: properly test for v1 encryption policies in encrypt tests
  fstests: split generic/580 into two tests
  fstests: split generic/581 into two tests
  fstests: split generic/613 into two tests
  fstest: add a fsstress+fscrypt test

Sweet Tea Dorminy (7):
  common/encrypt: separate data and inode nonces
  common/encrypt: add btrfs to get_encryption_*nonce
  common/encrypt: add btrfs to get_ciphertext_filename
  common/encrypt: enable making a encrypted btrfs filesystem
  common/verity: explicitly don't allow btrfs encryption
  btrfs: add simple test of reflink of encrypted data
  btrfs: test snapshotting encrypted subvol

 common/encrypt        |  88 ++++++++++++++++++++++++---
 common/verity         |   4 ++
 tests/btrfs/613       |  59 ++++++++++++++++++
 tests/btrfs/613.out   |  13 ++++
 tests/btrfs/614       |  76 ++++++++++++++++++++++++
 tests/btrfs/614.out   | 111 ++++++++++++++++++++++++++++++++++
 tests/f2fs/002        |   2 +-
 tests/generic/580     | 118 ++++++++++++++++--------------------
 tests/generic/580.out |  40 -------------
 tests/generic/581     |  89 +---------------------------
 tests/generic/581.out |  50 ----------------
 tests/generic/593     |   1 +
 tests/generic/613     |  24 +++-----
 tests/generic/613.out |   5 +-
 tests/generic/733     |  79 ++++++++++++++++++++++++
 tests/generic/733.out |  44 ++++++++++++++
 tests/generic/734     | 135 ++++++++++++++++++++++++++++++++++++++++++
 tests/generic/734.out |  51 ++++++++++++++++
 tests/generic/735     | 117 ++++++++++++++++++++++++++++++++++++
 tests/generic/735.out |  14 +++++
 tests/generic/736     |  38 ++++++++++++
 tests/generic/736.out |   3 +
 22 files changed, 888 insertions(+), 273 deletions(-)
 create mode 100755 tests/btrfs/613
 create mode 100644 tests/btrfs/613.out
 create mode 100755 tests/btrfs/614
 create mode 100644 tests/btrfs/614.out
 create mode 100644 tests/generic/733
 create mode 100644 tests/generic/733.out
 create mode 100644 tests/generic/734
 create mode 100644 tests/generic/734.out
 create mode 100644 tests/generic/735
 create mode 100644 tests/generic/735.out
 create mode 100644 tests/generic/736
 create mode 100644 tests/generic/736.out

-- 
2.41.0

[PATCH 01/12] common/encrypt: separate data and inode nonces

[Josef Bacik]

From: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>

btrfs will have different inode and data nonces, so we need to be
specific about which nonce each use needs. For now, there is no
difference in the two functions.

Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
 common/encrypt    | 33 ++++++++++++++++++++++++++-------
 tests/f2fs/002    |  2 +-
 tests/generic/613 |  4 ++--
 3 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/common/encrypt b/common/encrypt
index 1a77e23b..04b6e5ac 100644
--- a/common/encrypt
+++ b/common/encrypt
@@ -488,7 +488,7 @@ _add_fscrypt_provisioning_key()
 # Retrieve the encryption nonce of the given inode as a hex string.  The nonce
 # was randomly generated by the filesystem and isn't exposed directly to
 # userspace.  But it can be read using the filesystem's debugging tools.
-_get_encryption_nonce()
+_get_encryption_file_nonce()
 {
 	local device=$1
 	local inode=$2
@@ -532,15 +532,34 @@ _get_encryption_nonce()
 			}'
 		;;
 	*)
-		_fail "_get_encryption_nonce() isn't implemented on $FSTYP"
+		_fail "_get_encryption_file_nonce() isn't implemented on $FSTYP"
 		;;
 	esac
 }
 
-# Require support for _get_encryption_nonce()
+# Retrieve the encryption nonce used to encrypt the data of the given inode as
+# a hex string.  The nonce was randomly generated by the filesystem and isn't
+# exposed directly to userspace.  But it can be read using the filesystem's
+# debugging tools.
+_get_encryption_data_nonce()
+{
+	local device=$1
+	local inode=$2
+
+	case $FSTYP in
+	ext4|f2fs)
+		_get_encryption_file_nonce $device $inode
+		;;
+	*)
+		_fail "_get_encryption_data_nonce() isn't implemented on $FSTYP"
+		;;
+	esac
+}
+
+# Require support for _get_encryption_*nonce()
 _require_get_encryption_nonce_support()
 {
-	echo "Checking for _get_encryption_nonce() support for $FSTYP" >> $seqres.full
+	echo "Checking for _get_encryption_*nonce() support for $FSTYP" >> $seqres.full
 	case $FSTYP in
 	ext4)
 		_require_command "$DEBUGFS_PROG" debugfs
@@ -554,7 +573,7 @@ _require_get_encryption_nonce_support()
 		# the test fail in that case, as it was an f2fs-tools bug...
 		;;
 	*)
-		_notrun "_get_encryption_nonce() isn't implemented on $FSTYP"
+		_notrun "_get_encryption_*nonce() isn't implemented on $FSTYP"
 		;;
 	esac
 }
@@ -760,7 +779,7 @@ _do_verify_ciphertext_for_encryption_policy()
 	echo "Verifying encrypted file contents" >> $seqres.full
 	for f in "${test_contents_files[@]}"; do
 		read -r src inode blocklist <<< "$f"
-		nonce=$(_get_encryption_nonce $SCRATCH_DEV $inode)
+		nonce=$(_get_encryption_data_nonce $SCRATCH_DEV $inode)
 		_dump_ciphertext_blocks $SCRATCH_DEV $blocklist > $tmp.actual_contents
 		$crypt_contents_cmd $contents_encryption_mode $raw_key_hex \
 			--file-nonce=$nonce --block-size=$blocksize \
@@ -780,7 +799,7 @@ _do_verify_ciphertext_for_encryption_policy()
 	echo "Verifying encrypted file names" >> $seqres.full
 	for f in "${test_filenames_files[@]}"; do
 		read -r name inode dir_inode padding <<< "$f"
-		nonce=$(_get_encryption_nonce $SCRATCH_DEV $dir_inode)
+		nonce=$(_get_encryption_file_nonce $SCRATCH_DEV $dir_inode)
 		_get_ciphertext_filename $SCRATCH_DEV $inode $dir_inode \
 			> $tmp.actual_name
 		echo -n "$name" | \
diff --git a/tests/f2fs/002 b/tests/f2fs/002
index 8235d88a..a51ddf22 100755
--- a/tests/f2fs/002
+++ b/tests/f2fs/002
@@ -129,7 +129,7 @@ blocklist=$(_get_ciphertext_block_list $file)
 _scratch_unmount
 
 echo -e "\n# Getting file's encryption nonce"
-nonce=$(_get_encryption_nonce $SCRATCH_DEV $inode)
+nonce=$(_get_encryption_data_nonce $SCRATCH_DEV $inode)
 
 echo -e "\n# Dumping the file's raw data"
 _dump_ciphertext_blocks $SCRATCH_DEV $blocklist > $tmp.raw
diff --git a/tests/generic/613 b/tests/generic/613
index 4cf5ccc6..47c60e9c 100755
--- a/tests/generic/613
+++ b/tests/generic/613
@@ -68,10 +68,10 @@ echo -e "\n# Getting encryption nonces from inodes"
 echo -n > $tmp.nonces_hex
 echo -n > $tmp.nonces_bin
 for inode in "${inodes[@]}"; do
-	nonce=$(_get_encryption_nonce $SCRATCH_DEV $inode)
+	nonce=$(_get_encryption_data_nonce $SCRATCH_DEV $inode)
 	if (( ${#nonce} != 32 )) || [ -n "$(echo "$nonce" | tr -d 0-9a-fA-F)" ]
 	then
-		_fail "Expected nonce to be 16 bytes (32 hex characters), but got \"$nonce\""
+		_fail "Expected nonce for inode $inode to be 16 bytes (32 hex characters), but got \"$nonce\""
 	fi
 	echo $nonce >> $tmp.nonces_hex
 	echo -ne "$(echo $nonce | sed 's/[0-9a-fA-F]\{2\}/\\x\0/g')" \
-- 
2.41.0

[PATCH 02/12] common/encrypt: add btrfs to get_encryption_*nonce

[Josef Bacik]

From: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>

Add the modes of getting the encryption nonces, either inode or extent,
to the various get_encryption_nonce functions. For now, no encrypt test
makes a file with more than one extent, so we can just grab the first
extent's nonce for the data nonce; when we write a bigger file test,
we'll need to change that.

Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
 common/encrypt | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/common/encrypt b/common/encrypt
index 04b6e5ac..fc1c8cc7 100644
--- a/common/encrypt
+++ b/common/encrypt
@@ -531,6 +531,17 @@ _get_encryption_file_nonce()
 				found = 0;
 			}'
 		;;
+	btrfs)
+		# Retrieve the fscrypt context for an inode as a hex string.
+		# btrfs prints these like:
+		#        item 14 key ($inode FSCRYPT_CTXT_ITEM 0) itemoff 15491 itemsize 40
+		#                value: 02010400000000008fabf3dd745d41856e812458cd765bf0140f41d62853f4c0351837daff4dcc8f
+
+		$BTRFS_UTIL_PROG inspect-internal dump-tree $device | \
+			grep -A 1 "key ($inode FSCRYPT_CTXT_ITEM 0)" | \
+			grep --only-matching 'value: [[:xdigit:]]\+' | \
+			tr -d ' \n' | tail -c 32
+		;;
 	*)
 		_fail "_get_encryption_file_nonce() isn't implemented on $FSTYP"
 		;;
@@ -550,6 +561,23 @@ _get_encryption_data_nonce()
 	ext4|f2fs)
 		_get_encryption_file_nonce $device $inode
 		;;
+	btrfs)
+		# Retrieve the encryption IV of the first file extent in an inode as a hex
+		# string. btrfs prints the file extents (for simple unshared
+		# inodes) like:
+		#         item 21 key ($inode EXTENT_DATA 0) itemoff 2534 itemsize 69
+		#                generation 7 type 1 (regular)
+                #		 extent data disk byte 5304320 nr 1048576
+                #		 extent data offset 0 nr 1048576 ram 1048576
+                #		 extent compression 0 (none)
+                #		 extent encryption 161 ((1, 40: context 0201040200000000116a77667261d7422a4b1ed8c427e685edb7a0d370d0c9d40030333033333330))
+
+
+		$BTRFS_UTIL_PROG inspect-internal dump-tree $device | \
+			grep -A 5 "key ($inode EXTENT_DATA 0)" | \
+			grep --only-matching 'context [[:xdigit:]]\+' | \
+			tr -d ' \n' | tail -c 32
+		;;
 	*)
 		_fail "_get_encryption_data_nonce() isn't implemented on $FSTYP"
 		;;
@@ -572,6 +600,9 @@ _require_get_encryption_nonce_support()
 		# Otherwise the xattr is incorrectly parsed as v1.  But just let
 		# the test fail in that case, as it was an f2fs-tools bug...
 		;;
+	btrfs)
+		_require_command "$BTRFS_UTIL_PROG" btrfs
+		;;
 	*)
 		_notrun "_get_encryption_*nonce() isn't implemented on $FSTYP"
 		;;
-- 
2.41.0

[PATCH 03/12] common/encrypt: add btrfs to get_ciphertext_filename

[Josef Bacik]

From: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>

Add the relevant call to get an encrypted filename from btrfs.

Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
 common/encrypt | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/common/encrypt b/common/encrypt
index fc1c8cc7..2c1925da 100644
--- a/common/encrypt
+++ b/common/encrypt
@@ -618,6 +618,19 @@ _get_ciphertext_filename()
 	local dir_inode=$3
 
 	case $FSTYP in
+	btrfs)
+		# Extract the filename from the inode_ref object, similar to:
+		# item 24 key (259 INODE_REF 257) itemoff 14826 itemsize 26
+		# 	index 3 namelen 16 name: J\xf7\x15tD\x8eL\xae/\x98\x9f\x09\xc1\xb6\x09>
+		#
+		$BTRFS_UTIL_PROG inspect-internal dump-tree $device | \
+			grep -A 1 "key ($inode INODE_REF " | tail -n 1 | \
+			perl -ne '
+				s/.*?name: //;
+				chomp;
+				s/\\x([[:xdigit:]]{2})/chr hex $1/eg;
+				print;'
+		;;
 	ext4)
 		# Extract the filename from the debugfs output line like:
 		#
@@ -715,6 +728,9 @@ _require_get_ciphertext_filename_support()
 			_notrun "dump.f2fs (f2fs-tools) is too old; doesn't support showing unambiguous on-disk filenames"
 		fi
 		;;
+	btrfs)
+		_require_command "$BTRFS_UTIL_PROG" btrfs
+		;;
 	*)
 		_notrun "_get_ciphertext_filename() isn't implemented on $FSTYP"
 		;;
-- 
2.41.0

[PATCH 04/12] common/encrypt: enable making a encrypted btrfs filesystem

[Josef Bacik]

From: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>

Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
 common/encrypt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/common/encrypt b/common/encrypt
index 2c1925da..1372af66 100644
--- a/common/encrypt
+++ b/common/encrypt
@@ -153,6 +153,9 @@ _scratch_mkfs_encrypted()
 		# erase the UBI volume; reformated automatically on next mount
 		$UBIUPDATEVOL_PROG ${SCRATCH_DEV} -t
 		;;
+	btrfs)
+		_scratch_mkfs
+		;;
 	ceph)
 		_scratch_cleanup_files
 		;;
@@ -168,6 +171,9 @@ _scratch_mkfs_sized_encrypted()
 	ext4|f2fs)
 		MKFS_OPTIONS="$MKFS_OPTIONS -O encrypt" _scratch_mkfs_sized $*
 		;;
+	btrfs)
+		_scratch_mkfs_sized $*
+		;;
 	*)
 		_notrun "Filesystem $FSTYP not supported in _scratch_mkfs_sized_encrypted"
 		;;
-- 
2.41.0

[PATCH 05/12] common/verity: explicitly don't allow btrfs encryption

[Josef Bacik]

From: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>

Currently btrfs encryption doesn't support verity, but it is planned to
one day. To be explicit about the lack of support, add a custom error
message to the combination.

Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
 common/verity | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/common/verity b/common/verity
index 03d175ce..4e601a81 100644
--- a/common/verity
+++ b/common/verity
@@ -224,6 +224,10 @@ _scratch_mkfs_encrypted_verity()
 		# features with -O.  Instead -O must be supplied multiple times.
 		_scratch_mkfs -O encrypt -O verity
 		;;
+	btrfs)
+		# currently verity + encryption is not supported
+		_notrun "btrfs doesn't currently support verity + encryption"
+		;;
 	*)
 		_notrun "$FSTYP not supported in _scratch_mkfs_encrypted_verity"
 		;;
-- 
2.41.0

[PATCH 06/12] btrfs: add simple test of reflink of encrypted data

[Josef Bacik]

From: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>

Make sure that we succeed at reflinking encrypted data.

Test deliberately numbered with a high number so it won't conflict with
tests between now and merge.
---
 tests/btrfs/613     | 59 +++++++++++++++++++++++++++++++++++++++++++++
 tests/btrfs/613.out | 13 ++++++++++
 2 files changed, 72 insertions(+)
 create mode 100755 tests/btrfs/613
 create mode 100644 tests/btrfs/613.out

diff --git a/tests/btrfs/613 b/tests/btrfs/613
new file mode 100755
index 00000000..0288016e
--- /dev/null
+++ b/tests/btrfs/613
@@ -0,0 +1,59 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2023 Meta Platforms, Inc.  All Rights Reserved.
+#
+# FS QA Test 613
+#
+# Check if reflinking one encrypted file on btrfs succeeds.
+#
+. ./common/preamble
+_begin_fstest auto encrypt
+
+# Import common functions.
+. ./common/encrypt
+. ./common/filter
+. ./common/reflink
+
+# real QA test starts here
+
+# Modify as appropriate.
+_supported_fs btrfs
+
+_require_test
+_require_scratch
+_require_cp_reflink
+_require_scratch_encryption -v 2
+_require_command "$KEYCTL_PROG" keyctl
+
+_scratch_mkfs_encrypted &>> $seqres.full
+_scratch_mount
+
+dir=$SCRATCH_MNT/dir
+mkdir $dir
+_set_encpolicy $dir $TEST_KEY_IDENTIFIER
+_add_enckey $SCRATCH_MNT "$TEST_RAW_KEY"
+echo "Creating and reflinking a file"
+$XFS_IO_PROG -t -f -c "pwrite 0 33k" $dir/test > /dev/null
+cp --reflink=always $dir/test $dir/test2
+
+echo "Can't reflink encrypted and unencrypted"
+cp --reflink=always $dir/test $SCRATCH_MNT/fail |& _filter_scratch
+
+echo "Diffing the file and its copy"
+diff $dir/test $dir/test2
+
+echo "Verifying the files are reflinked"
+_verify_reflink $dir/test $dir/test2
+
+echo "Diffing the files after remount"
+_scratch_cycle_mount
+_add_enckey $SCRATCH_MNT "$TEST_RAW_KEY"
+diff $dir/test $dir/test2
+
+echo "Diffing the files after key remove"
+_rm_enckey $SCRATCH_MNT $TEST_KEY_IDENTIFIER
+diff $dir/test $dir/test2 |& _filter_scratch
+
+# success, all done
+status=0
+exit
diff --git a/tests/btrfs/613.out b/tests/btrfs/613.out
new file mode 100644
index 00000000..4895d6dd
--- /dev/null
+++ b/tests/btrfs/613.out
@@ -0,0 +1,13 @@
+QA output created by 613
+Added encryption key with identifier 69b2f6edeee720cce0577937eb8a6751
+Creating and reflinking a file
+Can't reflink encrypted and unencrypted
+cp: failed to clone 'SCRATCH_MNT/fail' from 'SCRATCH_MNT/dir/test': Invalid argument
+Diffing the file and its copy
+Verifying the files are reflinked
+Diffing the files after remount
+Added encryption key with identifier 69b2f6edeee720cce0577937eb8a6751
+Diffing the files after key remove
+Removed encryption key with identifier 69b2f6edeee720cce0577937eb8a6751
+diff: SCRATCH_MNT/dir/test: No such file or directory
+diff: SCRATCH_MNT/dir/test2: No such file or directory
-- 
2.41.0

[PATCH 07/12] btrfs: test snapshotting encrypted subvol

[Josef Bacik]

From: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>

Make sure that snapshots of encrypted data are readable and writeable.

Test deliberately high-numbered to not conflict.

Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
 tests/btrfs/614     |  76 ++++++++++++++++++++++++++++++
 tests/btrfs/614.out | 111 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 187 insertions(+)
 create mode 100755 tests/btrfs/614
 create mode 100644 tests/btrfs/614.out

diff --git a/tests/btrfs/614 b/tests/btrfs/614
new file mode 100755
index 00000000..87dd27f9
--- /dev/null
+++ b/tests/btrfs/614
@@ -0,0 +1,76 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2023 Meta Platforms, Inc.  All Rights Reserved.
+#
+# FS QA Test 614
+#
+# Try taking a snapshot of an encrypted subvolume. Make sure the snapshot is
+# still readable. Rewrite part of the subvol with the same data; make sure it's
+# still readable.
+#
+. ./common/preamble
+_begin_fstest auto encrypt
+
+# Import common functions.
+. ./common/encrypt
+. ./common/filter
+
+# real QA test starts here
+_supported_fs btrfs
+
+_require_test
+_require_scratch
+_require_scratch_encryption -v 2
+_require_command "$KEYCTL_PROG" keyctl
+
+_scratch_mkfs_encrypted &>> $seqres.full
+_scratch_mount
+
+udir=$SCRATCH_MNT/reference
+dir=$SCRATCH_MNT/subvol
+dir2=$SCRATCH_MNT/subvol2
+$BTRFS_UTIL_PROG subvolume create $dir >> $seqres.full
+mkdir $udir
+
+_set_encpolicy $dir $TEST_KEY_IDENTIFIER
+_add_enckey $SCRATCH_MNT "$TEST_RAW_KEY"
+
+# get files with lots of extents by using backwards writes.
+for j in `seq 0 50`; do
+	for i in `seq 20 -1 1`; do
+		$XFS_IO_PROG -f -d -c "pwrite $(($i * 4096)) 4096" \
+		$dir/foo-$j >> $seqres.full | _filter_xfs_io
+		$XFS_IO_PROG -f -d -c "pwrite $(($i * 4096)) 4096" \
+		$udir/foo-$j >> $seqres.full | _filter_xfs_io
+	done
+done
+
+$BTRFS_UTIL_PROG subvolume snapshot $dir $dir2 | _filter_scratch
+
+_scratch_remount
+_add_enckey $SCRATCH_MNT "$TEST_RAW_KEY"
+sleep 30
+echo "Diffing $dir and $dir2"
+diff $dir $dir2
+
+echo "Rewriting $dir2 partly"
+# rewrite half of each file in the snapshot
+for j in `seq 0 50`; do
+	for i in `seq 10 -1 1`; do
+		$XFS_IO_PROG -f -d -c "pwrite $(($i * 4096)) 4096" \
+		$dir2/foo-$j >> $seqres.full | _filter_xfs_io
+	done
+done
+
+echo "Diffing $dir and $dir2"
+diff $dir $dir2
+
+echo "Dropping key and diffing"
+_rm_enckey $SCRATCH_MNT $TEST_KEY_IDENTIFIER
+diff $dir $dir2 |& _filter_scratch | _filter_nokey_filenames
+
+$BTRFS_UTIL_PROG subvolume delete $dir > /dev/null 2>&1
+
+# success, all done
+status=0
+exit
diff --git a/tests/btrfs/614.out b/tests/btrfs/614.out
new file mode 100644
index 00000000..390807e8
--- /dev/null
+++ b/tests/btrfs/614.out
@@ -0,0 +1,111 @@
+QA output created by 614
+Added encryption key with identifier 69b2f6edeee720cce0577937eb8a6751
+Create a snapshot of 'SCRATCH_MNT/subvol' in 'SCRATCH_MNT/subvol2'
+Added encryption key with identifier 69b2f6edeee720cce0577937eb8a6751
+Diffing /mnt/scratch/subvol and /mnt/scratch/subvol2
+Rewriting /mnt/scratch/subvol2 partly
+Diffing /mnt/scratch/subvol and /mnt/scratch/subvol2
+Dropping key and diffing
+Removed encryption key with identifier 69b2f6edeee720cce0577937eb8a6751
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
+NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
-- 
2.41.0

[PATCH 08/12] fstests: properly test for v1 encryption policies in encrypt tests

[Josef Bacik]

With btrfs adding fscrypt support we're limiting the usage to plain v2
policies only.  This means we need to update the _require's for
generic/593 that tests both v1 and v2 policies.  The other sort of tests
will be split into two tests in later patches.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 common/encrypt    | 2 ++
 tests/generic/593 | 1 +
 2 files changed, 3 insertions(+)

diff --git a/common/encrypt b/common/encrypt
index 1372af66..120ca612 100644
--- a/common/encrypt
+++ b/common/encrypt
@@ -59,6 +59,8 @@ _require_scratch_encryption()
 	# policy required by the test.
 	if [ $# -ne 0 ]; then
 		_require_encryption_policy_support $SCRATCH_MNT "$@"
+	else
+		_require_encryption_policy_support $SCRATCH_MNT -v 1
 	fi
 
 	_scratch_unmount
diff --git a/tests/generic/593 b/tests/generic/593
index 2dda5d76..7907236c 100755
--- a/tests/generic/593
+++ b/tests/generic/593
@@ -17,6 +17,7 @@ _begin_fstest auto quick encrypt
 
 # real QA test starts here
 _supported_fs generic
+_require_scratch_encryption -v 1
 _require_scratch_encryption -v 2
 _require_command "$KEYCTL_PROG" keyctl
 
-- 
2.41.0

[PATCH 09/12] fstests: split generic/580 into two tests

[Josef Bacik]

generic/580 tests both v1 and v2 encryption policies, however btrfs only
supports v2 policies.  Split this into two tests so that we can get the
v2 coverage for btrfs.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 tests/generic/580     | 118 ++++++++++++++++++------------------------
 tests/generic/580.out |  40 --------------
 tests/generic/733     |  79 ++++++++++++++++++++++++++++
 tests/generic/733.out |  44 ++++++++++++++++
 4 files changed, 173 insertions(+), 108 deletions(-)
 create mode 100644 tests/generic/733
 create mode 100644 tests/generic/733.out

diff --git a/tests/generic/580 b/tests/generic/580
index 73f32ff9..63ab9712 100755
--- a/tests/generic/580
+++ b/tests/generic/580
@@ -5,7 +5,7 @@
 # FS QA Test generic/580
 #
 # Basic test of the fscrypt filesystem-level encryption keyring
-# and v2 encryption policies.
+# policy.
 #
 
 . ./common/preamble
@@ -18,80 +18,62 @@ echo
 
 # real QA test starts here
 _supported_fs generic
-_require_scratch_encryption -v 2
+_require_scratch_encryption 
 
 _scratch_mkfs_encrypted &>> $seqres.full
 _scratch_mount
 
-test_with_policy_version()
-{
-	local vers=$1
-
-	if (( vers == 1 )); then
-		local keyspec=$TEST_KEY_DESCRIPTOR
-		local add_enckey_args="-d $keyspec"
-	else
-		local keyspec=$TEST_KEY_IDENTIFIER
-		local add_enckey_args=""
-	fi
-
-	mkdir $dir
-	echo "# Setting v$vers encryption policy"
-	_set_encpolicy $dir $keyspec
-	echo "# Getting v$vers encryption policy"
-	_get_encpolicy $dir | _filter_scratch
-	if (( vers == 1 )); then
-		echo "# Getting v1 encryption policy using old ioctl"
-		_get_encpolicy $dir -1 | _filter_scratch
-	fi
-	echo "# Trying to create file without key added yet"
-	$XFS_IO_PROG -f $dir/file |& _filter_scratch
-	echo "# Getting encryption key status"
-	_enckey_status $SCRATCH_MNT $keyspec
-	echo "# Adding encryption key"
-	_add_enckey $SCRATCH_MNT "$TEST_RAW_KEY" $add_enckey_args
-	echo "# Creating encrypted file"
-	echo contents > $dir/file
-	echo "# Getting encryption key status"
-	_enckey_status $SCRATCH_MNT $keyspec
-	echo "# Removing encryption key"
-	_rm_enckey $SCRATCH_MNT $keyspec
-	echo "# Getting encryption key status"
-	_enckey_status $SCRATCH_MNT $keyspec
-	echo "# Verifying that the encrypted directory was \"locked\""
-	cat $dir/file |& _filter_scratch
-	cat "$(find $dir -type f)" |& _filter_scratch | cut -d ' ' -f3-
-
-	# Test removing key with a file open.
-	echo "# Re-adding encryption key"
-	_add_enckey $SCRATCH_MNT "$TEST_RAW_KEY" $add_enckey_args
-	echo "# Creating another encrypted file"
-	echo foo > $dir/file2
-	echo "# Removing key while an encrypted file is open"
-	exec 3< $dir/file
-	_rm_enckey $SCRATCH_MNT $keyspec
-	echo "# Non-open file should have been evicted"
-	cat $dir/file2 |& _filter_scratch
-	echo "# Open file shouldn't have been evicted"
-	cat $dir/file
-	echo "# Key should be in \"incompletely removed\" state"
-	_enckey_status $SCRATCH_MNT $keyspec
-	echo "# Closing file and removing key for real now"
-	exec 3<&-
-	_rm_enckey $SCRATCH_MNT $keyspec
-	cat $dir/file |& _filter_scratch
-
-	echo "# Cleaning up"
-	rm -rf $dir
-	_scratch_cycle_mount	# Clear all keys
-	echo
-}
-
 dir=$SCRATCH_MNT/dir
+keyspec=$TEST_KEY_DESCRIPTOR
 
-test_with_policy_version 1
+mkdir $dir
+echo "# Setting v1 encryption policy"
+_set_encpolicy $dir $keyspec
+echo "# Getting v1 encryption policy"
+_get_encpolicy $dir | _filter_scratch
+echo "# Getting v1 encryption policy using old ioctl"
+_get_encpolicy $dir -1 | _filter_scratch
+echo "# Trying to create file without key added yet"
+$XFS_IO_PROG -f $dir/file |& _filter_scratch
+echo "# Getting encryption key status"
+_enckey_status $SCRATCH_MNT $keyspec
+echo "# Adding encryption key"
+_add_enckey $SCRATCH_MNT "$TEST_RAW_KEY" -d $keyspec
+echo "# Creating encrypted file"
+echo contents > $dir/file
+echo "# Getting encryption key status"
+_enckey_status $SCRATCH_MNT $keyspec
+echo "# Removing encryption key"
+_rm_enckey $SCRATCH_MNT $keyspec
+echo "# Getting encryption key status"
+_enckey_status $SCRATCH_MNT $keyspec
+echo "# Verifying that the encrypted directory was \"locked\""
+cat $dir/file |& _filter_scratch
+cat "$(find $dir -type f)" |& _filter_scratch | cut -d ' ' -f3-
 
-test_with_policy_version 2
+# Test removing key with a file open.
+echo "# Re-adding encryption key"
+_add_enckey $SCRATCH_MNT "$TEST_RAW_KEY" -d $keyspec
+echo "# Creating another encrypted file"
+echo foo > $dir/file2
+echo "# Removing key while an encrypted file is open"
+exec 3< $dir/file
+_rm_enckey $SCRATCH_MNT $keyspec
+echo "# Non-open file should have been evicted"
+cat $dir/file2 |& _filter_scratch
+echo "# Open file shouldn't have been evicted"
+cat $dir/file
+echo "# Key should be in \"incompletely removed\" state"
+_enckey_status $SCRATCH_MNT $keyspec
+echo "# Closing file and removing key for real now"
+exec 3<&-
+_rm_enckey $SCRATCH_MNT $keyspec
+cat $dir/file |& _filter_scratch
+
+echo "# Cleaning up"
+rm -rf $dir
+_scratch_cycle_mount	# Clear all keys
+echo
 
 echo "# Trying to remove absent key"
 _rm_enckey $SCRATCH_MNT abcdabcdabcdabcd
diff --git a/tests/generic/580.out b/tests/generic/580.out
index 989d4514..f2f4d490 100644
--- a/tests/generic/580.out
+++ b/tests/generic/580.out
@@ -47,45 +47,5 @@ Removed encryption key with descriptor 0000111122223333
 cat: SCRATCH_MNT/dir/file: No such file or directory
 # Cleaning up
 
-# Setting v2 encryption policy
-# Getting v2 encryption policy
-Encryption policy for SCRATCH_MNT/dir:
-	Policy version: 2
-	Master key identifier: 69b2f6edeee720cce0577937eb8a6751
-	Contents encryption mode: 1 (AES-256-XTS)
-	Filenames encryption mode: 4 (AES-256-CTS)
-	Flags: 0x02
-# Trying to create file without key added yet
-SCRATCH_MNT/dir/file: Required key not available
-# Getting encryption key status
-Absent
-# Adding encryption key
-Added encryption key with identifier 69b2f6edeee720cce0577937eb8a6751
-# Creating encrypted file
-# Getting encryption key status
-Present (user_count=1, added_by_self)
-# Removing encryption key
-Removed encryption key with identifier 69b2f6edeee720cce0577937eb8a6751
-# Getting encryption key status
-Absent
-# Verifying that the encrypted directory was "locked"
-cat: SCRATCH_MNT/dir/file: No such file or directory
-Required key not available
-# Re-adding encryption key
-Added encryption key with identifier 69b2f6edeee720cce0577937eb8a6751
-# Creating another encrypted file
-# Removing key while an encrypted file is open
-Removed encryption key with identifier 69b2f6edeee720cce0577937eb8a6751, but files still busy
-# Non-open file should have been evicted
-cat: SCRATCH_MNT/dir/file2: Required key not available
-# Open file shouldn't have been evicted
-contents
-# Key should be in "incompletely removed" state
-Incompletely removed
-# Closing file and removing key for real now
-Removed encryption key with identifier 69b2f6edeee720cce0577937eb8a6751
-cat: SCRATCH_MNT/dir/file: No such file or directory
-# Cleaning up
-
 # Trying to remove absent key
 Error removing encryption key: Required key not available
diff --git a/tests/generic/733 b/tests/generic/733
new file mode 100644
index 00000000..ae0434fb
--- /dev/null
+++ b/tests/generic/733
@@ -0,0 +1,79 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+#
+# FS QA Test generic/733
+#
+# A v2 only version of generic/580
+
+. ./common/preamble
+_begin_fstest auto quick encrypt
+echo
+
+# Import common functions.
+. ./common/filter
+. ./common/encrypt
+
+# real QA test starts here
+_supported_fs generic
+_require_scratch_encryption -v 2
+
+_scratch_mkfs_encrypted &>> $seqres.full
+_scratch_mount
+
+keyspec=$TEST_KEY_IDENTIFIER
+dir=$SCRATCH_MNT/dir
+
+mkdir $dir
+echo "# Setting v2 encryption policy"
+_set_encpolicy $dir $keyspec
+echo "# Getting v2 encryption policy"
+_get_encpolicy $dir | _filter_scratch
+echo "# Trying to create file without key added yet"
+$XFS_IO_PROG -f $dir/file |& _filter_scratch
+echo "# Getting encryption key status"
+_enckey_status $SCRATCH_MNT $keyspec
+echo "# Adding encryption key"
+_add_enckey $SCRATCH_MNT "$TEST_RAW_KEY"
+echo "# Creating encrypted file"
+echo contents > $dir/file
+echo "# Getting encryption key status"
+_enckey_status $SCRATCH_MNT $keyspec
+echo "# Removing encryption key"
+_rm_enckey $SCRATCH_MNT $keyspec
+echo "# Getting encryption key status"
+_enckey_status $SCRATCH_MNT $keyspec
+echo "# Verifying that the encrypted directory was \"locked\""
+cat $dir/file |& _filter_scratch
+cat "$(find $dir -type f)" |& _filter_scratch | cut -d ' ' -f3-
+
+# Test removing key with a file open.
+echo "# Re-adding encryption key"
+_add_enckey $SCRATCH_MNT "$TEST_RAW_KEY"
+echo "# Creating another encrypted file"
+echo foo > $dir/file2
+echo "# Removing key while an encrypted file is open"
+exec 3< $dir/file
+_rm_enckey $SCRATCH_MNT $keyspec
+echo "# Non-open file should have been evicted"
+cat $dir/file2 |& _filter_scratch
+echo "# Open file shouldn't have been evicted"
+cat $dir/file
+echo "# Key should be in \"incompletely removed\" state"
+_enckey_status $SCRATCH_MNT $keyspec
+echo "# Closing file and removing key for real now"
+exec 3<&-
+_rm_enckey $SCRATCH_MNT $keyspec
+cat $dir/file |& _filter_scratch
+
+echo "# Cleaning up"
+rm -rf $dir
+_scratch_cycle_mount	# Clear all keys
+echo
+
+echo "# Trying to remove absent key"
+_rm_enckey $SCRATCH_MNT abcdabcdabcdabcd
+
+# success, all done
+status=0
+exit
+
diff --git a/tests/generic/733.out b/tests/generic/733.out
new file mode 100644
index 00000000..02dce51d
--- /dev/null
+++ b/tests/generic/733.out
@@ -0,0 +1,44 @@
+QA output created by 733
+
+# Setting v2 encryption policy
+# Getting v2 encryption policy
+Encryption policy for SCRATCH_MNT/dir:
+	Policy version: 2
+	Master key identifier: 69b2f6edeee720cce0577937eb8a6751
+	Contents encryption mode: 1 (AES-256-XTS)
+	Filenames encryption mode: 4 (AES-256-CTS)
+	Flags: 0x02
+# Trying to create file without key added yet
+SCRATCH_MNT/dir/file: Required key not available
+# Getting encryption key status
+Absent
+# Adding encryption key
+Added encryption key with identifier 69b2f6edeee720cce0577937eb8a6751
+# Creating encrypted file
+# Getting encryption key status
+Present (user_count=1, added_by_self)
+# Removing encryption key
+Removed encryption key with identifier 69b2f6edeee720cce0577937eb8a6751
+# Getting encryption key status
+Absent
+# Verifying that the encrypted directory was "locked"
+cat: SCRATCH_MNT/dir/file: No such file or directory
+Required key not available
+# Re-adding encryption key
+Added encryption key with identifier 69b2f6edeee720cce0577937eb8a6751
+# Creating another encrypted file
+# Removing key while an encrypted file is open
+Removed encryption key with identifier 69b2f6edeee720cce0577937eb8a6751, but files still busy
+# Non-open file should have been evicted
+cat: SCRATCH_MNT/dir/file2: Required key not available
+# Open file shouldn't have been evicted
+contents
+# Key should be in "incompletely removed" state
+Incompletely removed
+# Closing file and removing key for real now
+Removed encryption key with identifier 69b2f6edeee720cce0577937eb8a6751
+cat: SCRATCH_MNT/dir/file: No such file or directory
+# Cleaning up
+
+# Trying to remove absent key
+Error removing encryption key: Required key not available
-- 
2.41.0

[PATCH 10/12] fstests: split generic/581 into two tests

[Josef Bacik]

generic/581 is mostly a v2 policy test, but it does do some quick checks
of v1 policies as a normal user.  Split the v1 and v2 related parts
into two different tests so that the v2 part can get properly tested for
btrfs file systems, which only support v2 policies.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 tests/generic/581     |  89 +---------------------------
 tests/generic/581.out |  50 ----------------
 tests/generic/734     | 135 ++++++++++++++++++++++++++++++++++++++++++
 tests/generic/734.out |  51 ++++++++++++++++
 4 files changed, 188 insertions(+), 137 deletions(-)
 create mode 100644 tests/generic/734
 create mode 100644 tests/generic/734.out

diff --git a/tests/generic/581 b/tests/generic/581
index cabc7e1c..ab930ac6 100755
--- a/tests/generic/581
+++ b/tests/generic/581
@@ -4,8 +4,7 @@
 #
 # FS QA Test No. generic/581
 #
-# Test non-root use of the fscrypt filesystem-level encryption keyring
-# and v2 encryption policies.
+# Test non-root use of the fscrypt filesystem-level encryption keyring policy.
 #
 
 . ./common/preamble
@@ -31,7 +30,7 @@ _cleanup()
 # real QA test starts here
 _supported_fs generic
 _require_user
-_require_scratch_encryption -v 2
+_require_scratch_encryption
 
 _scratch_mkfs_encrypted &>> $seqres.full
 _scratch_mount
@@ -58,90 +57,6 @@ echo "# Adding v1 policy key as regular user (should fail with EACCES)"
 _user_do_add_enckey $SCRATCH_MNT "$raw_key" -d $keydesc
 
 rm -rf $dir
-echo
-_user_do "mkdir $dir"
-
-echo "# Setting v2 policy as regular user without key already added (should fail with ENOKEY)"
-_user_do_set_encpolicy $dir $keyid |& _filter_scratch
-
-echo "# Adding v2 policy key as regular user (should succeed)"
-_user_do_add_enckey $SCRATCH_MNT "$raw_key"
-
-echo "# Setting v2 policy as regular user with key added (should succeed)"
-_user_do_set_encpolicy $dir $keyid
-
-echo "# Getting v2 policy as regular user (should succeed)"
-_user_do_get_encpolicy $dir | _filter_scratch
-
-echo "# Creating encrypted file as regular user (should succeed)"
-_user_do "echo contents > $dir/file"
-
-echo "# Removing v2 policy key as regular user (should succeed)"
-_user_do_rm_enckey $SCRATCH_MNT $keyid
-
-_scratch_cycle_mount	# Clear all keys
-
-# Wait for any invalidated keys to be garbage-collected.
-i=0
-while grep -E -q '^[0-9a-f]+ [^ ]*i[^ ]*' /proc/keys; do
-	if ((++i >= 20)); then
-		echo "Timed out waiting for invalidated keys to be GC'ed" >> $seqres.full
-		break
-	fi
-	sleep 0.5
-done
-
-# Set the user key quota to the fsgqa user's current number of keys plus 5.
-orig_keys=$(_user_do "awk '/^[[:space:]]*$(id -u fsgqa):/{print \$4}' /proc/key-users | cut -d/ -f1")
-: ${orig_keys:=0}
-echo "orig_keys=$orig_keys" >> $seqres.full
-orig_maxkeys=$(</proc/sys/kernel/keys/maxkeys)
-keys_to_add=5
-echo $((orig_keys + keys_to_add)) > /proc/sys/kernel/keys/maxkeys
-
-echo
-echo "# Testing user key quota"
-for i in `seq $((keys_to_add + 1))`; do
-	rand_raw_key=$(_generate_raw_encryption_key)
-	_user_do_add_enckey $SCRATCH_MNT "$rand_raw_key" \
-	    | sed 's/ with identifier .*$//'
-done
-
-# Restore the original key quota.
-echo "$orig_maxkeys" > /proc/sys/kernel/keys/maxkeys
-
-rm -rf $dir
-echo
-_user_do "mkdir $dir"
-_scratch_cycle_mount	# Clear all keys
-
-# Test multiple users adding the same key.
-echo "# Adding key as root"
-_add_enckey $SCRATCH_MNT "$raw_key"
-echo "# Getting key status as regular user"
-_user_do_enckey_status $SCRATCH_MNT $keyid
-echo "# Removing key only added by another user (should fail with ENOKEY)"
-_user_do_rm_enckey $SCRATCH_MNT $keyid
-echo "# Setting v2 encryption policy with key only added by another user (should fail with ENOKEY)"
-_user_do_set_encpolicy $dir $keyid |& _filter_scratch
-echo "# Adding second user of key"
-_user_do_add_enckey $SCRATCH_MNT "$raw_key"
-echo "# Getting key status as regular user"
-_user_do_enckey_status $SCRATCH_MNT $keyid
-echo "# Setting v2 encryption policy as regular user"
-_user_do_set_encpolicy $dir $keyid
-echo "# Removing this user's claim to the key"
-_user_do_rm_enckey $SCRATCH_MNT $keyid
-echo "# Getting key status as regular user"
-_user_do_enckey_status $SCRATCH_MNT $keyid
-echo "# Adding back second user of key"
-_user_do_add_enckey $SCRATCH_MNT "$raw_key"
-echo "# Remove key for \"all users\", as regular user (should fail with EACCES)"
-_user_do_rm_enckey $SCRATCH_MNT $keyid -a |& _filter_scratch
-_enckey_status $SCRATCH_MNT $keyid
-echo "# Remove key for \"all users\", as root"
-_rm_enckey $SCRATCH_MNT $keyid -a
-_enckey_status $SCRATCH_MNT $keyid
 
 # success, all done
 status=0
diff --git a/tests/generic/581.out b/tests/generic/581.out
index b3f7d889..a8cb96a8 100644
--- a/tests/generic/581.out
+++ b/tests/generic/581.out
@@ -10,53 +10,3 @@ Encryption policy for SCRATCH_MNT/dir:
 	Flags: 0x02
 # Adding v1 policy key as regular user (should fail with EACCES)
 Permission denied
-
-# Setting v2 policy as regular user without key already added (should fail with ENOKEY)
-SCRATCH_MNT/dir: failed to set encryption policy: Required key not available
-# Adding v2 policy key as regular user (should succeed)
-Added encryption key with identifier 69b2f6edeee720cce0577937eb8a6751
-# Setting v2 policy as regular user with key added (should succeed)
-# Getting v2 policy as regular user (should succeed)
-Encryption policy for SCRATCH_MNT/dir:
-	Policy version: 2
-	Master key identifier: 69b2f6edeee720cce0577937eb8a6751
-	Contents encryption mode: 1 (AES-256-XTS)
-	Filenames encryption mode: 4 (AES-256-CTS)
-	Flags: 0x02
-# Creating encrypted file as regular user (should succeed)
-# Removing v2 policy key as regular user (should succeed)
-Removed encryption key with identifier 69b2f6edeee720cce0577937eb8a6751
-
-# Testing user key quota
-Added encryption key
-Added encryption key
-Added encryption key
-Added encryption key
-Added encryption key
-Error adding encryption key: Disk quota exceeded
-
-# Adding key as root
-Added encryption key with identifier 69b2f6edeee720cce0577937eb8a6751
-# Getting key status as regular user
-Present (user_count=1)
-# Removing key only added by another user (should fail with ENOKEY)
-Error removing encryption key: Required key not available
-# Setting v2 encryption policy with key only added by another user (should fail with ENOKEY)
-SCRATCH_MNT/dir: failed to set encryption policy: Required key not available
-# Adding second user of key
-Added encryption key with identifier 69b2f6edeee720cce0577937eb8a6751
-# Getting key status as regular user
-Present (user_count=2, added_by_self)
-# Setting v2 encryption policy as regular user
-# Removing this user's claim to the key
-Removed user's claim to encryption key with identifier 69b2f6edeee720cce0577937eb8a6751
-# Getting key status as regular user
-Present (user_count=1)
-# Adding back second user of key
-Added encryption key with identifier 69b2f6edeee720cce0577937eb8a6751
-# Remove key for "all users", as regular user (should fail with EACCES)
-Permission denied
-Present (user_count=2, added_by_self)
-# Remove key for "all users", as root
-Removed encryption key with identifier 69b2f6edeee720cce0577937eb8a6751
-Absent
diff --git a/tests/generic/734 b/tests/generic/734
new file mode 100644
index 00000000..a6f46e7e
--- /dev/null
+++ b/tests/generic/734
@@ -0,0 +1,135 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright 2019 Google LLC
+#
+# FS QA Test No. generic/581
+#
+# Test non-root use of the fscrypt filesystem-level encryption v2 policy.
+#
+
+. ./common/preamble
+_begin_fstest auto quick encrypt
+echo
+
+orig_maxkeys=
+
+# Override the default cleanup function.
+_cleanup()
+{
+	cd /
+	rm -f $tmp.*
+	if [ -n "$orig_maxkeys" ]; then
+		echo "$orig_maxkeys" > /proc/sys/kernel/keys/maxkeys
+	fi
+}
+
+# Import common functions.
+. ./common/filter
+. ./common/encrypt
+
+# real QA test starts here
+_supported_fs generic
+_require_user
+_require_scratch_encryption -v 2
+
+_scratch_mkfs_encrypted &>> $seqres.full
+_scratch_mount
+
+dir=$SCRATCH_MNT/dir
+
+raw_key=""
+for i in `seq 64`; do
+	raw_key+="\\x$(printf "%02x" $i)"
+done
+keydesc="0000111122223333"
+keyid="69b2f6edeee720cce0577937eb8a6751"
+chmod 777 $SCRATCH_MNT
+
+_user_do "mkdir $dir"
+
+echo "# Setting v2 policy as regular user without key already added (should fail with ENOKEY)"
+_user_do_set_encpolicy $dir $keyid |& _filter_scratch
+
+echo "# Adding v2 policy key as regular user (should succeed)"
+_user_do_add_enckey $SCRATCH_MNT "$raw_key"
+
+echo "# Setting v2 policy as regular user with key added (should succeed)"
+_user_do_set_encpolicy $dir $keyid
+
+echo "# Getting v2 policy as regular user (should succeed)"
+_user_do_get_encpolicy $dir | _filter_scratch
+
+echo "# Creating encrypted file as regular user (should succeed)"
+_user_do "echo contents > $dir/file"
+
+echo "# Removing v2 policy key as regular user (should succeed)"
+_user_do_rm_enckey $SCRATCH_MNT $keyid
+
+_scratch_cycle_mount	# Clear all keys
+
+# Wait for any invalidated keys to be garbage-collected.
+i=0
+while grep -E -q '^[0-9a-f]+ [^ ]*i[^ ]*' /proc/keys; do
+	if ((++i >= 20)); then
+		echo "Timed out waiting for invalidated keys to be GC'ed" >> $seqres.full
+		break
+	fi
+	sleep 0.5
+done
+
+# Set the user key quota to the fsgqa user's current number of keys plus 5.
+orig_keys=$(_user_do "awk '/^[[:space:]]*$(id -u fsgqa):/{print \$4}' /proc/key-users | cut -d/ -f1")
+: ${orig_keys:=0}
+echo "orig_keys=$orig_keys" >> $seqres.full
+orig_maxkeys=$(</proc/sys/kernel/keys/maxkeys)
+keys_to_add=5
+echo $((orig_keys + keys_to_add)) > /proc/sys/kernel/keys/maxkeys
+
+echo
+echo "# Testing user key quota"
+for i in `seq $((keys_to_add + 1))`; do
+	rand_raw_key=$(_generate_raw_encryption_key)
+	_user_do_add_enckey $SCRATCH_MNT "$rand_raw_key" \
+	    | sed 's/ with identifier .*$//'
+done
+
+# Restore the original key quota.
+echo "$orig_maxkeys" > /proc/sys/kernel/keys/maxkeys
+
+rm -rf $dir
+echo
+_user_do "mkdir $dir"
+_scratch_cycle_mount	# Clear all keys
+
+# Test multiple users adding the same key.
+echo "# Adding key as root"
+_add_enckey $SCRATCH_MNT "$raw_key"
+echo "# Getting key status as regular user"
+_user_do_enckey_status $SCRATCH_MNT $keyid
+echo "# Removing key only added by another user (should fail with ENOKEY)"
+_user_do_rm_enckey $SCRATCH_MNT $keyid
+echo "# Setting v2 encryption policy with key only added by another user (should fail with ENOKEY)"
+_user_do_set_encpolicy $dir $keyid |& _filter_scratch
+echo "# Adding second user of key"
+_user_do_add_enckey $SCRATCH_MNT "$raw_key"
+echo "# Getting key status as regular user"
+_user_do_enckey_status $SCRATCH_MNT $keyid
+echo "# Setting v2 encryption policy as regular user"
+_user_do_set_encpolicy $dir $keyid
+echo "# Removing this user's claim to the key"
+_user_do_rm_enckey $SCRATCH_MNT $keyid
+echo "# Getting key status as regular user"
+_user_do_enckey_status $SCRATCH_MNT $keyid
+echo "# Adding back second user of key"
+_user_do_add_enckey $SCRATCH_MNT "$raw_key"
+echo "# Remove key for \"all users\", as regular user (should fail with EACCES)"
+_user_do_rm_enckey $SCRATCH_MNT $keyid -a |& _filter_scratch
+_enckey_status $SCRATCH_MNT $keyid
+echo "# Remove key for \"all users\", as root"
+_rm_enckey $SCRATCH_MNT $keyid -a
+_enckey_status $SCRATCH_MNT $keyid
+
+# success, all done
+status=0
+exit
+
diff --git a/tests/generic/734.out b/tests/generic/734.out
new file mode 100644
index 00000000..85a8c973
--- /dev/null
+++ b/tests/generic/734.out
@@ -0,0 +1,51 @@
+QA output created by 734
+
+# Setting v2 policy as regular user without key already added (should fail with ENOKEY)
+SCRATCH_MNT/dir: failed to set encryption policy: Required key not available
+# Adding v2 policy key as regular user (should succeed)
+Added encryption key with identifier 69b2f6edeee720cce0577937eb8a6751
+# Setting v2 policy as regular user with key added (should succeed)
+# Getting v2 policy as regular user (should succeed)
+Encryption policy for SCRATCH_MNT/dir:
+	Policy version: 2
+	Master key identifier: 69b2f6edeee720cce0577937eb8a6751
+	Contents encryption mode: 1 (AES-256-XTS)
+	Filenames encryption mode: 4 (AES-256-CTS)
+	Flags: 0x02
+# Creating encrypted file as regular user (should succeed)
+# Removing v2 policy key as regular user (should succeed)
+Removed encryption key with identifier 69b2f6edeee720cce0577937eb8a6751
+
+# Testing user key quota
+Added encryption key
+Added encryption key
+Added encryption key
+Added encryption key
+Added encryption key
+Error adding encryption key: Disk quota exceeded
+
+# Adding key as root
+Added encryption key with identifier 69b2f6edeee720cce0577937eb8a6751
+# Getting key status as regular user
+Present (user_count=1)
+# Removing key only added by another user (should fail with ENOKEY)
+Error removing encryption key: Required key not available
+# Setting v2 encryption policy with key only added by another user (should fail with ENOKEY)
+SCRATCH_MNT/dir: failed to set encryption policy: Required key not available
+# Adding second user of key
+Added encryption key with identifier 69b2f6edeee720cce0577937eb8a6751
+# Getting key status as regular user
+Present (user_count=2, added_by_self)
+# Setting v2 encryption policy as regular user
+# Removing this user's claim to the key
+Removed user's claim to encryption key with identifier 69b2f6edeee720cce0577937eb8a6751
+# Getting key status as regular user
+Present (user_count=1)
+# Adding back second user of key
+Added encryption key with identifier 69b2f6edeee720cce0577937eb8a6751
+# Remove key for "all users", as regular user (should fail with EACCES)
+Permission denied
+Present (user_count=2, added_by_self)
+# Remove key for "all users", as root
+Removed encryption key with identifier 69b2f6edeee720cce0577937eb8a6751
+Absent
-- 
2.41.0

[PATCH 11/12] fstests: split generic/613 into two tests

[Josef Bacik]

generic/613 tests v1 and v2 policies, but btrfs can only support v2
policies.  Split this into two different tests, 613 which will only test
v1 policies, and then 735 which will test v2 policies.

The 735 test will also add checks for the per-extent nonces to validate
they're all sufficiently random.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 tests/generic/613     |  20 ++------
 tests/generic/613.out |   5 +-
 tests/generic/735     | 117 ++++++++++++++++++++++++++++++++++++++++++
 tests/generic/735.out |  14 +++++
 4 files changed, 138 insertions(+), 18 deletions(-)
 create mode 100644 tests/generic/735
 create mode 100644 tests/generic/735.out

diff --git a/tests/generic/613 b/tests/generic/613
index 47c60e9c..96b81a96 100755
--- a/tests/generic/613
+++ b/tests/generic/613
@@ -22,22 +22,21 @@ _begin_fstest auto quick encrypt
 
 # real QA test starts here
 _supported_fs generic
-_require_scratch_encryption -v 2
+_require_scratch_encryption
 _require_get_encryption_nonce_support
 _require_command "$XZ_PROG" xz
 
 _scratch_mkfs_encrypted &>> $seqres.full
 _scratch_mount
 
-echo -e "\n# Adding encryption keys"
-_add_enckey $SCRATCH_MNT "$TEST_RAW_KEY"
+echo -e "\n# Adding encryption key"
 _add_enckey $SCRATCH_MNT "$TEST_RAW_KEY" -d $TEST_KEY_DESCRIPTOR
 
 # Create a bunch of encrypted files and directories -- enough for the uniqueness
 # and randomness tests to be meaningful, but not so many that this test takes a
-# long time.  Test using both v1 and v2 encryption policies, and for each of
-# those test the case of an encryption policy that is assigned to an empty
-# directory as well as the case of a file created in an encrypted directory.
+# long time.  Test using the v1 encryption policy, test the case of an
+# encryption policy that is assigned to an empty directory as well as the case
+# of a file created in an encrypted directory.
 echo -e "\n# Creating encrypted files and directories"
 inodes=()
 for i in {1..50}; do
@@ -45,20 +44,11 @@ for i in {1..50}; do
 	mkdir $dir
 	inodes+=("$(stat -c %i $dir)")
 	_set_encpolicy $dir $TEST_KEY_DESCRIPTOR
-
-	dir=$SCRATCH_MNT/v2_policy_dir_$i
-	mkdir $dir
-	inodes+=("$(stat -c %i $dir)")
-	_set_encpolicy $dir $TEST_KEY_IDENTIFIER
 done
 for i in {1..50}; do
 	file=$SCRATCH_MNT/v1_policy_dir_1/$i
 	touch $file
 	inodes+=("$(stat -c %i $file)")
-
-	file=$SCRATCH_MNT/v2_policy_dir_1/$i
-	touch $file
-	inodes+=("$(stat -c %i $file)")
 done
 _scratch_unmount
 
diff --git a/tests/generic/613.out b/tests/generic/613.out
index 203a64f2..4a218d03 100644
--- a/tests/generic/613.out
+++ b/tests/generic/613.out
@@ -1,7 +1,6 @@
 QA output created by 613
 
-# Adding encryption keys
-Added encryption key with identifier 69b2f6edeee720cce0577937eb8a6751
+# Adding encryption key
 Added encryption key with descriptor 0000111122223333
 
 # Creating encrypted files and directories
@@ -12,5 +11,5 @@ Added encryption key with descriptor 0000111122223333
 Listing non-unique nonces:
 
 # Verifying randomness of nonces
-Uncompressed size is 3200 bytes
+Uncompressed size is 1600 bytes
 Nonces are incompressible, as expected
diff --git a/tests/generic/735 b/tests/generic/735
new file mode 100644
index 00000000..c901be1f
--- /dev/null
+++ b/tests/generic/735
@@ -0,0 +1,117 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright 2023 Meta
+#
+# FS QA Test No. 735
+#
+# A variation of generic/613 that only tests v2, and checks data nonces for any
+# file system that supporst per-extent encryption.
+#
+# Test that encryption nonces are unique and random, where randomness is
+# approximated as "incompressible by the xz program".
+#
+# An encryption nonce is the 16-byte value that the filesystem generates for
+# each encrypted file.  These nonces must be unique in order to cause different
+# files to be encrypted differently, which is an important security property.
+# In practice, they need to be random to achieve that; and it's easy enough to
+# test for both uniqueness and randomness, so we test for both.
+#
+. ./common/preamble
+_begin_fstest auto quick encrypt
+
+# Import common functions.
+. ./common/filter
+. ./common/encrypt
+
+# real QA test starts here
+_supported_fs generic
+_require_scratch_encryption -v 2
+_require_get_encryption_nonce_support
+_require_command "$XZ_PROG" xz
+
+_check_nonce()
+{
+	local nonce=$1
+
+	if (( ${#nonce} != 32 )) || [ -n "$(echo "$nonce" | tr -d 0-9a-fA-F)" ]
+	then
+		_fail "Expected nonce for inode $inode to be 16 bytes (32 hex characters), but got \"$nonce\""
+	fi
+}
+
+_scratch_mkfs_encrypted &>> $seqres.full
+_scratch_mount
+
+echo -e "\n# Adding encryption key"
+_add_enckey $SCRATCH_MNT "$TEST_RAW_KEY"
+
+# Create a bunch of encrypted files and directories -- enough for the uniqueness
+# and randomness tests to be meaningful, but not so many that this test takes a
+# long time.  Test using the v2 encryption policy, test the case of an
+# encryption policy that is assigned to an empty directory as well as the case
+# of a file created in an encrypted directory.
+echo -e "\n# Creating encrypted files and directories"
+inodes=()
+for i in {1..50}; do
+	dir=$SCRATCH_MNT/v2_policy_dir_$i
+	mkdir $dir
+	inodes+=("$(stat -c %i $dir)")
+	_set_encpolicy $dir $TEST_KEY_IDENTIFIER
+done
+for i in {1..50}; do
+	file=$SCRATCH_MNT/v2_policy_dir_1/$i
+	$XFS_IO_PROG -f -c "pwrite 0 1m" $file > /dev/null
+	inodes+=("$(stat -c %i $file)")
+done
+_scratch_unmount
+
+# Build files that contain all the nonces.  nonces_hex contains them in hex, one
+# per line.  nonces_bin contains them in binary, all concatenated.
+echo -e "\n# Getting encryption nonces from inodes"
+echo -n > $tmp.nonces_hex
+echo -n > $tmp.nonces_bin
+for inode in "${inodes[@]}"; do
+	inode_nonce=$(_get_encryption_file_nonce $SCRATCH_DEV $inode)
+	_check_nonce $inode_nonce
+	
+	echo $inode_nonce >> $tmp.nonces_hex
+	echo -ne "$(echo $inode_nonce | sed 's/[0-9a-fA-F]\{2\}/\\x\0/g')" \
+		>> $tmp.nonces_bin
+
+	data_nonce=$(_get_encryption_data_nonce $SCRATCH_DEV $inode)
+
+	# If the inode is empty we won't have a data nonce
+	[ "$data_nonce" = "" ] && continue
+
+	# If the inode nonce and data nonce are the same continue
+	[ "$inode_nonce" = "$data_nonce" ] && continue
+
+	_check_nonce $data_nonce
+	
+	echo $data_nonce >> $tmp.nonces_hex
+	echo -ne "$(echo $data_nonce | sed 's/[0-9a-fA-F]\{2\}/\\x\0/g')" \
+		>> $tmp.nonces_bin
+done
+
+# Verify the uniqueness and randomness of the nonces.  In theory randomness
+# implies uniqueness here, but it's easy enough to explicitly test for both.
+
+echo -e "\n# Verifying uniqueness of nonces"
+echo "Listing non-unique nonces:"
+sort < $tmp.nonces_hex | uniq -d
+
+echo -e "\n# Verifying randomness of nonces"
+uncompressed_size=$(stat -c %s $tmp.nonces_bin)
+echo "Uncompressed size is $uncompressed_size bytes" >> $seqres.full
+compressed_size=$($XZ_PROG -c < $tmp.nonces_bin | wc -c)
+echo "Compressed size is $compressed_size bytes" >> $seqres.full
+# The xz format has 60 bytes of overhead.  Go a bit lower to avoid flakiness.
+if (( compressed_size >= uncompressed_size + 55 )); then
+	echo "Nonces are incompressible, as expected"
+else
+	_fail "Nonces are compressible (non-random); compressed $uncompressed_size => $compressed_size bytes!"
+fi
+
+# success, all done
+status=0
+exit
diff --git a/tests/generic/735.out b/tests/generic/735.out
new file mode 100644
index 00000000..bf73118b
--- /dev/null
+++ b/tests/generic/735.out
@@ -0,0 +1,14 @@
+QA output created by 735
+
+# Adding encryption key
+Added encryption key with identifier 69b2f6edeee720cce0577937eb8a6751
+
+# Creating encrypted files and directories
+
+# Getting encryption nonces from inodes
+
+# Verifying uniqueness of nonces
+Listing non-unique nonces:
+
+# Verifying randomness of nonces
+Nonces are incompressible, as expected
-- 
2.41.0

[PATCH 12/12] fstest: add a fsstress+fscrypt test

[Josef Bacik]

I noticed we don't run fsstress with fscrypt in any of our tests, and
this was helpful in uncovering a couple of symlink related corner cases
for the btrfs support work.  Add a basic test that creates a encrypted
directory and runs fsstress in that directory.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 tests/generic/736     | 38 ++++++++++++++++++++++++++++++++++++++
 tests/generic/736.out |  3 +++
 2 files changed, 41 insertions(+)
 create mode 100644 tests/generic/736
 create mode 100644 tests/generic/736.out

diff --git a/tests/generic/736 b/tests/generic/736
new file mode 100644
index 00000000..0ef37d7e
--- /dev/null
+++ b/tests/generic/736
@@ -0,0 +1,38 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright 2023 Meta
+#
+# FS QA Test No. generic/5736
+#
+# Run fscrypt on an encrypted directory
+#
+
+. ./common/preamble
+_begin_fstest auto quick encrypt
+echo
+
+# Import common functions.
+. ./common/filter
+. ./common/encrypt
+
+# real QA test starts here
+_supported_fs generic
+_require_scratch_encryption -v 2
+
+_scratch_mkfs_encrypted &>> $seqres.full
+_scratch_mount
+
+dir=$SCRATCH_MNT/dir
+mkdir $dir
+
+_set_encpolicy $dir $TEST_KEY_IDENTIFIER
+_add_enckey $SCRATCH_MNT "$TEST_RAW_KEY"
+
+args=$(_scale_fsstress_args -p 4 -n 10000 -p 2 $FSSTRESS_AVOID -d $dir)
+echo "Run fsstress $args" >>$seqres.full
+
+$FSSTRESS_PROG $args >> $seqres.full
+
+# success, all done
+status=0
+exit
diff --git a/tests/generic/736.out b/tests/generic/736.out
new file mode 100644
index 00000000..022754df
--- /dev/null
+++ b/tests/generic/736.out
@@ -0,0 +1,3 @@
+QA output created by 736
+
+Added encryption key with identifier 69b2f6edeee720cce0577937eb8a6751
-- 
2.41.0

Re: [PATCH 01/12] common/encrypt: separate data and inode nonces

[Eric Biggers]

On Tue, Oct 10, 2023 at 04:25:54PM -0400, Josef Bacik wrote:
> From: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
> 
> btrfs will have different inode and data nonces, so we need to be
> specific about which nonce each use needs. For now, there is no
> difference in the two functions.
> 
> Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
> ---
>  common/encrypt    | 33 ++++++++++++++++++++++++++-------
>  tests/f2fs/002    |  2 +-
>  tests/generic/613 |  4 ++--
>  3 files changed, 29 insertions(+), 10 deletions(-)
> 
> diff --git a/common/encrypt b/common/encrypt
> index 1a77e23b..04b6e5ac 100644
> --- a/common/encrypt
> +++ b/common/encrypt
> @@ -488,7 +488,7 @@ _add_fscrypt_provisioning_key()
>  # Retrieve the encryption nonce of the given inode as a hex string.  The nonce
>  # was randomly generated by the filesystem and isn't exposed directly to
>  # userspace.  But it can be read using the filesystem's debugging tools.
> -_get_encryption_nonce()
> +_get_encryption_file_nonce()
>  {
>  	local device=$1
>  	local inode=$2
> @@ -532,15 +532,34 @@ _get_encryption_nonce()
>  			}'
>  		;;
>  	*)
> -		_fail "_get_encryption_nonce() isn't implemented on $FSTYP"
> +		_fail "_get_encryption_file_nonce() isn't implemented on $FSTYP"
>  		;;
>  	esac
>  }
>  
> -# Require support for _get_encryption_nonce()
> +# Retrieve the encryption nonce used to encrypt the data of the given inode as
> +# a hex string.  The nonce was randomly generated by the filesystem and isn't
> +# exposed directly to userspace.  But it can be read using the filesystem's
> +# debugging tools.
> +_get_encryption_data_nonce()
> +{
> +	local device=$1
> +	local inode=$2
> +
> +	case $FSTYP in
> +	ext4|f2fs)
> +		_get_encryption_file_nonce $device $inode
> +		;;
> +	*)
> +		_fail "_get_encryption_data_nonce() isn't implemented on $FSTYP"
> +		;;
> +	esac
> +}

Shouldn't this be _get_encryption_extent_nonce(), taking the offset of the
extent as a parameter?

Also I think it would sound better as _get_extent_encryption_nonce(), and
likewise _get_file_encryption_nonce().

- Eric

Re: [PATCH 12/12] fstest: add a fsstress+fscrypt test

[Eric Biggers]

On Tue, Oct 10, 2023 at 04:26:05PM -0400, Josef Bacik wrote:
> I noticed we don't run fsstress with fscrypt in any of our tests, and
> this was helpful in uncovering a couple of symlink related corner cases
> for the btrfs support work.  Add a basic test that creates a encrypted
> directory and runs fsstress in that directory.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> ---
>  tests/generic/736     | 38 ++++++++++++++++++++++++++++++++++++++
>  tests/generic/736.out |  3 +++
>  2 files changed, 41 insertions(+)
>  create mode 100644 tests/generic/736
>  create mode 100644 tests/generic/736.out

This might be worth adding, but the way this sort of thing is tested on other
filesystems is through implementing the test_dummy_encryption mount option and
then doing a full run of xfstests with test_dummy_encryption enabled.  That's
more comprehensive than just running fsstress.

- Eric

Re: [PATCH 08/12] fstests: properly test for v1 encryption policies in encrypt tests

[Eric Biggers]

On Tue, Oct 10, 2023 at 04:26:01PM -0400, Josef Bacik wrote:
> With btrfs adding fscrypt support we're limiting the usage to plain v2
> policies only.  This means we need to update the _require's for
> generic/593 that tests both v1 and v2 policies.  The other sort of tests
> will be split into two tests in later patches.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> ---
>  common/encrypt    | 2 ++
>  tests/generic/593 | 1 +
>  2 files changed, 3 insertions(+)
> 
> diff --git a/common/encrypt b/common/encrypt
> index 1372af66..120ca612 100644
> --- a/common/encrypt
> +++ b/common/encrypt
> @@ -59,6 +59,8 @@ _require_scratch_encryption()
>  	# policy required by the test.
>  	if [ $# -ne 0 ]; then
>  		_require_encryption_policy_support $SCRATCH_MNT "$@"
> +	else
> +		_require_encryption_policy_support $SCRATCH_MNT -v 1
>  	fi

I guess this is okay for a start, but even after the test splits that this
patchset does, this will result in quite a few of the encrypt tests being
skipped on btrfs: generic/{395-399,419,429,435,440}.

I'm hoping that we can migrate most of them to support a v2-only world.  I'm not
sure what the best way to go about it would be.  I suppose one option would be
to just make copies of them and change those copies to test v2 instead of v1...
We could then consider removing or stripping down the v1 tests as appropriate.

- Eric

Re: [PATCH 06/12] btrfs: add simple test of reflink of encrypted data

[Anand Jain]

On 11/10/2023 04:25, Josef Bacik wrote:
> From: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
> 
> Make sure that we succeed at reflinking encrypted data.
> 
> Test deliberately numbered with a high number so it won't conflict with
> tests between now and merge.
> ---

Looks good. However, SOB is missing.

Thanks, Anand

Re: [PATCH 01/12] common/encrypt: separate data and inode nonces

[Anand Jain]

On 11/10/2023 04:25, Josef Bacik wrote:
> From: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
> 
> btrfs will have different inode and data nonces, so we need to be
> specific about which nonce each use needs. For now, there is no
> difference in the two functions.
> 
> Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
> ---
Looks good.

Reviewed-by: Anand Jain <anand.jain@oracle.com>

And, as Eric pointed out, the naming can be more intuitive. Keywords 
such as 'inode' and 'extent' will make them more intuitive, rather than 
'file' and 'data,' IMO.

Thanks, Anand

Re: [PATCH 02/12] common/encrypt: add btrfs to get_encryption_*nonce

[Anand Jain]

On 11/10/2023 04:25, Josef Bacik wrote:
> From: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
> 
> Add the modes of getting the encryption nonces, either inode or extent,
> to the various get_encryption_nonce functions. For now, no encrypt test
> makes a file with more than one extent, so we can just grab the first
> extent's nonce for the data nonce; when we write a bigger file test,
> we'll need to change that.
> 
> Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
> ---

Reviewed-by: Anand Jain <anand.jain@oracle.com>

Thanks, Anand

Re: [PATCH 03/12] common/encrypt: add btrfs to get_ciphertext_filename

[Anand Jain]

On 11/10/2023 04:25, Josef Bacik wrote:
> From: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
> 
> Add the relevant call to get an encrypted filename from btrfs.
> 
> Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>

Looks good.

Reviewed-by: Anand Jain <anand.jain@oracle.com>

Thanks, Anand

Re: [PATCH 04/12] common/encrypt: enable making a encrypted btrfs filesystem

[Anand Jain]

On 11/10/2023 04:25, Josef Bacik wrote:
> From: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
> 
> Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
> ---

Looks good.

Reviewed-by: Anand Jain <anand.jain@oracle.com>


Thanks, Anand

Re: [PATCH 05/12] common/verity: explicitly don't allow btrfs encryption

[Anand Jain]

On 11/10/2023 04:25, Josef Bacik wrote:
> From: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
> 
> Currently btrfs encryption doesn't support verity, but it is planned to
> one day. To be explicit about the lack of support, add a custom error
> message to the combination.
> 
> Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>


Looks Good.

Reviewed-by: Anand Jain <anand.jain@oracle.com>

Thanks, Anand

Re: [PATCH 07/12] btrfs: test snapshotting encrypted subvol

[Anand Jain]

On 11/10/2023 04:26, Josef Bacik wrote:
> From: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
> 
> Make sure that snapshots of encrypted data are readable and writeable.
> 
> Test deliberately high-numbered to not conflict.
> 
> Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>

Looks good to me.

Reviewed-by: Anand Jain <anand.jain@oracle.com>

Thanks, Anand

Re: [PATCH 07/12] btrfs: test snapshotting encrypted subvol

[Filipe Manana]

On Tue, Oct 10, 2023 at 9:26 PM Josef Bacik <josef@toxicpanda.com> wrote:
>
> From: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
>
> Make sure that snapshots of encrypted data are readable and writeable.
>
> Test deliberately high-numbered to not conflict.
>
> Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
> ---
>  tests/btrfs/614     |  76 ++++++++++++++++++++++++++++++
>  tests/btrfs/614.out | 111 ++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 187 insertions(+)
>  create mode 100755 tests/btrfs/614
>  create mode 100644 tests/btrfs/614.out
>
> diff --git a/tests/btrfs/614 b/tests/btrfs/614
> new file mode 100755
> index 00000000..87dd27f9
> --- /dev/null
> +++ b/tests/btrfs/614
> @@ -0,0 +1,76 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2023 Meta Platforms, Inc.  All Rights Reserved.
> +#
> +# FS QA Test 614
> +#
> +# Try taking a snapshot of an encrypted subvolume. Make sure the snapshot is
> +# still readable. Rewrite part of the subvol with the same data; make sure it's
> +# still readable.
> +#
> +. ./common/preamble
> +_begin_fstest auto encrypt

Should be in the 'snapshot' and 'subvol' groups too, as it creates a
snapshot and a subvolume.
Also maybe in the 'quick' group too, see the comments further below.

> +
> +# Import common functions.
> +. ./common/encrypt
> +. ./common/filter
> +
> +# real QA test starts here
> +_supported_fs btrfs
> +
> +_require_test

The test device is not used, so this can go away.

> +_require_scratch
> +_require_scratch_encryption -v 2
> +_require_command "$KEYCTL_PROG" keyctl
> +
> +_scratch_mkfs_encrypted &>> $seqres.full
> +_scratch_mount
> +
> +udir=$SCRATCH_MNT/reference
> +dir=$SCRATCH_MNT/subvol
> +dir2=$SCRATCH_MNT/subvol2
> +$BTRFS_UTIL_PROG subvolume create $dir >> $seqres.full
> +mkdir $udir
> +
> +_set_encpolicy $dir $TEST_KEY_IDENTIFIER
> +_add_enckey $SCRATCH_MNT "$TEST_RAW_KEY"
> +
> +# get files with lots of extents by using backwards writes.
> +for j in `seq 0 50`; do
> +       for i in `seq 20 -1 1`; do
> +               $XFS_IO_PROG -f -d -c "pwrite $(($i * 4096)) 4096" \
> +               $dir/foo-$j >> $seqres.full | _filter_xfs_io
> +               $XFS_IO_PROG -f -d -c "pwrite $(($i * 4096)) 4096" \
> +               $udir/foo-$j >> $seqres.full | _filter_xfs_io
> +       done
> +done
> +
> +$BTRFS_UTIL_PROG subvolume snapshot $dir $dir2 | _filter_scratch
> +
> +_scratch_remount
> +_add_enckey $SCRATCH_MNT "$TEST_RAW_KEY"
> +sleep 30

What's the sleep for?
Is the 30 seconds to wait for a transaction commit?
If it is then I'd rather mount the fs with -o commit=3 (or some other
low value) and then "sleep 3" to make the test run much faster.
A comment explaining why the sleep is there, what is its purpose,
should also be in place.

> +echo "Diffing $dir and $dir2"
> +diff $dir $dir2
> +
> +echo "Rewriting $dir2 partly"
> +# rewrite half of each file in the snapshot
> +for j in `seq 0 50`; do
> +       for i in `seq 10 -1 1`; do
> +               $XFS_IO_PROG -f -d -c "pwrite $(($i * 4096)) 4096" \
> +               $dir2/foo-$j >> $seqres.full | _filter_xfs_io
> +       done
> +done
> +
> +echo "Diffing $dir and $dir2"
> +diff $dir $dir2
> +
> +echo "Dropping key and diffing"
> +_rm_enckey $SCRATCH_MNT $TEST_KEY_IDENTIFIER
> +diff $dir $dir2 |& _filter_scratch | _filter_nokey_filenames
> +
> +$BTRFS_UTIL_PROG subvolume delete $dir > /dev/null 2>&1

What's the purpose of this subvolume delete?
It's ignoring stdout and stderr, so it doesn't care whether it
succeeds or fails, and we
don't do any tests/checks after it.

Thanks.



> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/btrfs/614.out b/tests/btrfs/614.out
> new file mode 100644
> index 00000000..390807e8
> --- /dev/null
> +++ b/tests/btrfs/614.out
> @@ -0,0 +1,111 @@
> +QA output created by 614
> +Added encryption key with identifier 69b2f6edeee720cce0577937eb8a6751
> +Create a snapshot of 'SCRATCH_MNT/subvol' in 'SCRATCH_MNT/subvol2'
> +Added encryption key with identifier 69b2f6edeee720cce0577937eb8a6751
> +Diffing /mnt/scratch/subvol and /mnt/scratch/subvol2
> +Rewriting /mnt/scratch/subvol2 partly
> +Diffing /mnt/scratch/subvol and /mnt/scratch/subvol2
> +Dropping key and diffing
> +Removed encryption key with identifier 69b2f6edeee720cce0577937eb8a6751
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
> --
> 2.41.0
>

Re: [PATCH 08/12] fstests: properly test for v1 encryption policies in encrypt tests

[Anand Jain]

On 11/10/2023 04:26, Josef Bacik wrote:
> With btrfs adding fscrypt support we're limiting the usage to plain v2
> policies only.  This means we need to update the _require's for
> generic/593 that tests both v1 and v2 policies.  The other sort of tests
> will be split into two tests in later patches.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> ---

For now this looks good;

Reviewed-by: Anand Jain <anand.jain@oracle.com>

Thanks, Anand

Re: [PATCH 09/12] fstests: split generic/580 into two tests

[Anand Jain]

On 10/11/23 04:26, Josef Bacik wrote:
> generic/580 tests both v1 and v2 encryption policies, however btrfs only
> supports v2 policies.  Split this into two tests so that we can get the
> v2 coverage for btrfs.

Instead of duplicating the test cases for v1 and v2 encryption policies,
can we check the supported version and run them accordingly within a
single test case?

The same applies 10 and 11/12 patches as well.

Thanks, Anand

Re: [PATCH 12/12] fstest: add a fsstress+fscrypt test

[Anand Jain]

On 10/11/23 04:26, Josef Bacik wrote:
> I noticed we don't run fsstress with fscrypt in any of our tests, and
> this was helpful in uncovering a couple of symlink related corner cases
> for the btrfs support work.  Add a basic test that creates a encrypted
> directory and runs fsstress in that directory.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

Looks good.

Reviewed-by: Anand Jain <anand.jain@oracle.com>

Re: [PATCH 09/12] fstests: split generic/580 into two tests

[Josef Bacik]

On Thu, Nov 02, 2023 at 07:42:50PM +0800, Anand Jain wrote:
> On 10/11/23 04:26, Josef Bacik wrote:
> > generic/580 tests both v1 and v2 encryption policies, however btrfs only
> > supports v2 policies.  Split this into two tests so that we can get the
> > v2 coverage for btrfs.
> 
> Instead of duplicating the test cases for v1 and v2 encryption policies,
> can we check the supported version and run them accordingly within a
> single test case?
> 
> The same applies 10 and 11/12 patches as well.

This will be awkward for file systems that support both, hence the split.  I
don't love suddenly generating a bunch of new tests, but this seems like the
better option since btrfs is the only file system that only supports v2, and
everybody else supports everything.  Thanks,

Josef

Re: [PATCH 09/12] fstests: split generic/580 into two tests

[Anand Jain]

On 11/9/23 04:25, Josef Bacik wrote:
> On Thu, Nov 02, 2023 at 07:42:50PM +0800, Anand Jain wrote:
>> On 10/11/23 04:26, Josef Bacik wrote:
>>> generic/580 tests both v1 and v2 encryption policies, however btrfs only
>>> supports v2 policies.  Split this into two tests so that we can get the
>>> v2 coverage for btrfs.
>>
>> Instead of duplicating the test cases for v1 and v2 encryption policies,
>> can we check the supported version and run them accordingly within a
>> single test case?
>>
>> The same applies 10 and 11/12 patches as well.
> 
> This will be awkward for file systems that support both, hence the split.  I
> don't love suddenly generating a bunch of new tests, but this seems like the
> better option since btrfs is the only file system that only supports v2, and
> everybody else supports everything.  Thanks,
> 
> Josef


Ok. That's fair.

Re: [PATCH 07/12] btrfs: test snapshotting encrypted subvol

[Anand Jain]

On 31/10/2023 23:39, Filipe Manana wrote:
> On Tue, Oct 10, 2023 at 9:26 PM Josef Bacik <josef@toxicpanda.com> wrote:
>>
>> From: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
>>
>> Make sure that snapshots of encrypted data are readable and writeable.
>>
>> Test deliberately high-numbered to not conflict.
>>
>> Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
>> ---
>>   tests/btrfs/614     |  76 ++++++++++++++++++++++++++++++
>>   tests/btrfs/614.out | 111 ++++++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 187 insertions(+)
>>   create mode 100755 tests/btrfs/614
>>   create mode 100644 tests/btrfs/614.out
>>
>> diff --git a/tests/btrfs/614 b/tests/btrfs/614
>> new file mode 100755
>> index 00000000..87dd27f9
>> --- /dev/null
>> +++ b/tests/btrfs/614
>> @@ -0,0 +1,76 @@
>> +#! /bin/bash
>> +# SPDX-License-Identifier: GPL-2.0
>> +# Copyright (c) 2023 Meta Platforms, Inc.  All Rights Reserved.
>> +#
>> +# FS QA Test 614
>> +#
>> +# Try taking a snapshot of an encrypted subvolume. Make sure the snapshot is
>> +# still readable. Rewrite part of the subvol with the same data; make sure it's
>> +# still readable.
>> +#
>> +. ./common/preamble
>> +_begin_fstest auto encrypt
> 
> Should be in the 'snapshot' and 'subvol' groups too, as it creates a
> snapshot and a subvolume.
> Also maybe in the 'quick' group too, see the comments further below.
> 
>> +
>> +# Import common functions.
>> +. ./common/encrypt
>> +. ./common/filter
>> +
>> +# real QA test starts here
>> +_supported_fs btrfs
>> +
>> +_require_test
> 
> The test device is not used, so this can go away.
> 
>> +_require_scratch
>> +_require_scratch_encryption -v 2
>> +_require_command "$KEYCTL_PROG" keyctl
>> +
>> +_scratch_mkfs_encrypted &>> $seqres.full
>> +_scratch_mount
>> +
>> +udir=$SCRATCH_MNT/reference
>> +dir=$SCRATCH_MNT/subvol
>> +dir2=$SCRATCH_MNT/subvol2
>> +$BTRFS_UTIL_PROG subvolume create $dir >> $seqres.full
>> +mkdir $udir
>> +
>> +_set_encpolicy $dir $TEST_KEY_IDENTIFIER
>> +_add_enckey $SCRATCH_MNT "$TEST_RAW_KEY"
>> +
>> +# get files with lots of extents by using backwards writes.
>> +for j in `seq 0 50`; do
>> +       for i in `seq 20 -1 1`; do
>> +               $XFS_IO_PROG -f -d -c "pwrite $(($i * 4096)) 4096" \
>> +               $dir/foo-$j >> $seqres.full | _filter_xfs_io
>> +               $XFS_IO_PROG -f -d -c "pwrite $(($i * 4096)) 4096" \
>> +               $udir/foo-$j >> $seqres.full | _filter_xfs_io
>> +       done
>> +done
>> +
>> +$BTRFS_UTIL_PROG subvolume snapshot $dir $dir2 | _filter_scratch
>> +
>> +_scratch_remount
>> +_add_enckey $SCRATCH_MNT "$TEST_RAW_KEY"
>> +sleep 30
> 
> What's the sleep for?
> Is the 30 seconds to wait for a transaction commit?
> If it is then I'd rather mount the fs with -o commit=3 (or some other
> low value) and then "sleep 3" to make the test run much faster.
> A comment explaining why the sleep is there, what is its purpose,
> should also be in place.
> 
>> +echo "Diffing $dir and $dir2"
>> +diff $dir $dir2
>> +
>> +echo "Rewriting $dir2 partly"
>> +# rewrite half of each file in the snapshot
>> +for j in `seq 0 50`; do
>> +       for i in `seq 10 -1 1`; do
>> +               $XFS_IO_PROG -f -d -c "pwrite $(($i * 4096)) 4096" \
>> +               $dir2/foo-$j >> $seqres.full | _filter_xfs_io
>> +       done
>> +done
>> +
>> +echo "Diffing $dir and $dir2"
>> +diff $dir $dir2
>> +
>> +echo "Dropping key and diffing"
>> +_rm_enckey $SCRATCH_MNT $TEST_KEY_IDENTIFIER
>> +diff $dir $dir2 |& _filter_scratch | _filter_nokey_filenames
>> +
>> +$BTRFS_UTIL_PROG subvolume delete $dir > /dev/null 2>&1
> 
> What's the purpose of this subvolume delete?
> It's ignoring stdout and stderr, so it doesn't care whether it
> succeeds or fails, and we
> don't do any tests/checks after it.
> 
> Thanks.


Josef, I'm planning to get this patchset ready for the PR. Are you 
planning to address the review comments as mentioned above? These
aren't bugs, but they definitely add more clarity and adds to the
missing groups.


Thanks, Anand


> 
> 
> 
>> +
>> +# success, all done
>> +status=0
>> +exit
>> diff --git a/tests/btrfs/614.out b/tests/btrfs/614.out
>> new file mode 100644
>> index 00000000..390807e8
>> --- /dev/null
>> +++ b/tests/btrfs/614.out
>> @@ -0,0 +1,111 @@
>> +QA output created by 614
>> +Added encryption key with identifier 69b2f6edeee720cce0577937eb8a6751
>> +Create a snapshot of 'SCRATCH_MNT/subvol' in 'SCRATCH_MNT/subvol2'
>> +Added encryption key with identifier 69b2f6edeee720cce0577937eb8a6751
>> +Diffing /mnt/scratch/subvol and /mnt/scratch/subvol2
>> +Rewriting /mnt/scratch/subvol2 partly
>> +Diffing /mnt/scratch/subvol and /mnt/scratch/subvol2
>> +Dropping key and diffing
>> +Removed encryption key with identifier 69b2f6edeee720cce0577937eb8a6751
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> +NOKEY_NAME: NOKEY_NAME/NOKEY_NAME/NOKEY_NAME: NOKEY_NAME NOKEY_NAME NOKEY_NAME NOKEY_NAME
>> --
>> 2.41.0
>>

Re: [PATCH 07/12] btrfs: test snapshotting encrypted subvol

[Josef Bacik]

On Mon, Nov 27, 2023 at 10:16:28PM +0800, Anand Jain wrote:
> 
> 
> On 31/10/2023 23:39, Filipe Manana wrote:
> > On Tue, Oct 10, 2023 at 9:26 PM Josef Bacik <josef@toxicpanda.com> wrote:
> > > 
> > > From: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
> > > 
> > > Make sure that snapshots of encrypted data are readable and writeable.
> > > 
> > > Test deliberately high-numbered to not conflict.
> > > 
> > > Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
> > > ---
> > >   tests/btrfs/614     |  76 ++++++++++++++++++++++++++++++
> > >   tests/btrfs/614.out | 111 ++++++++++++++++++++++++++++++++++++++++++++
> > >   2 files changed, 187 insertions(+)
> > >   create mode 100755 tests/btrfs/614
> > >   create mode 100644 tests/btrfs/614.out
> > > 
> > > diff --git a/tests/btrfs/614 b/tests/btrfs/614
> > > new file mode 100755
> > > index 00000000..87dd27f9
> > > --- /dev/null
> > > +++ b/tests/btrfs/614
> > > @@ -0,0 +1,76 @@
> > > +#! /bin/bash
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +# Copyright (c) 2023 Meta Platforms, Inc.  All Rights Reserved.
> > > +#
> > > +# FS QA Test 614
> > > +#
> > > +# Try taking a snapshot of an encrypted subvolume. Make sure the snapshot is
> > > +# still readable. Rewrite part of the subvol with the same data; make sure it's
> > > +# still readable.
> > > +#
> > > +. ./common/preamble
> > > +_begin_fstest auto encrypt
> > 
> > Should be in the 'snapshot' and 'subvol' groups too, as it creates a
> > snapshot and a subvolume.
> > Also maybe in the 'quick' group too, see the comments further below.
> > 
> > > +
> > > +# Import common functions.
> > > +. ./common/encrypt
> > > +. ./common/filter
> > > +
> > > +# real QA test starts here
> > > +_supported_fs btrfs
> > > +
> > > +_require_test
> > 
> > The test device is not used, so this can go away.
> > 
> > > +_require_scratch
> > > +_require_scratch_encryption -v 2
> > > +_require_command "$KEYCTL_PROG" keyctl
> > > +
> > > +_scratch_mkfs_encrypted &>> $seqres.full
> > > +_scratch_mount
> > > +
> > > +udir=$SCRATCH_MNT/reference
> > > +dir=$SCRATCH_MNT/subvol
> > > +dir2=$SCRATCH_MNT/subvol2
> > > +$BTRFS_UTIL_PROG subvolume create $dir >> $seqres.full
> > > +mkdir $udir
> > > +
> > > +_set_encpolicy $dir $TEST_KEY_IDENTIFIER
> > > +_add_enckey $SCRATCH_MNT "$TEST_RAW_KEY"
> > > +
> > > +# get files with lots of extents by using backwards writes.
> > > +for j in `seq 0 50`; do
> > > +       for i in `seq 20 -1 1`; do
> > > +               $XFS_IO_PROG -f -d -c "pwrite $(($i * 4096)) 4096" \
> > > +               $dir/foo-$j >> $seqres.full | _filter_xfs_io
> > > +               $XFS_IO_PROG -f -d -c "pwrite $(($i * 4096)) 4096" \
> > > +               $udir/foo-$j >> $seqres.full | _filter_xfs_io
> > > +       done
> > > +done
> > > +
> > > +$BTRFS_UTIL_PROG subvolume snapshot $dir $dir2 | _filter_scratch
> > > +
> > > +_scratch_remount
> > > +_add_enckey $SCRATCH_MNT "$TEST_RAW_KEY"
> > > +sleep 30
> > 
> > What's the sleep for?
> > Is the 30 seconds to wait for a transaction commit?
> > If it is then I'd rather mount the fs with -o commit=3 (or some other
> > low value) and then "sleep 3" to make the test run much faster.
> > A comment explaining why the sleep is there, what is its purpose,
> > should also be in place.
> > 
> > > +echo "Diffing $dir and $dir2"
> > > +diff $dir $dir2
> > > +
> > > +echo "Rewriting $dir2 partly"
> > > +# rewrite half of each file in the snapshot
> > > +for j in `seq 0 50`; do
> > > +       for i in `seq 10 -1 1`; do
> > > +               $XFS_IO_PROG -f -d -c "pwrite $(($i * 4096)) 4096" \
> > > +               $dir2/foo-$j >> $seqres.full | _filter_xfs_io
> > > +       done
> > > +done
> > > +
> > > +echo "Diffing $dir and $dir2"
> > > +diff $dir $dir2
> > > +
> > > +echo "Dropping key and diffing"
> > > +_rm_enckey $SCRATCH_MNT $TEST_KEY_IDENTIFIER
> > > +diff $dir $dir2 |& _filter_scratch | _filter_nokey_filenames
> > > +
> > > +$BTRFS_UTIL_PROG subvolume delete $dir > /dev/null 2>&1
> > 
> > What's the purpose of this subvolume delete?
> > It's ignoring stdout and stderr, so it doesn't care whether it
> > succeeds or fails, and we
> > don't do any tests/checks after it.
> > 
> > Thanks.
> 
> 
> Josef, I'm planning to get this patchset ready for the PR. Are you planning
> to address the review comments as mentioned above? These
> aren't bugs, but they definitely add more clarity and adds to the
> missing groups.
> 

Can you hold off Anand?  I haven't responded because I've been working on this
series and making appropriate changes to my local branch, I'll send a refreshed
version of the patches when I send the next set of the fscrypt enablement
patches.  I've got all the comments addressed locally, it'll save you some work.
Thanks,

Josef