On 5/26/25 1:25 AM, Shinichiro Kawasaki wrote:
On May 23, 2025 / 09:49, Bart Van Assche wrote:
[ ... ]
+. common/null_blk
Nit: this line can be removed since tests/zbd/rc sources common/null_blk.
+requires() {
+ _have_driver dm-crypt
+ _have_fio
+ _have_module null_blk
Nit: the line above can be removed since group_requires() zbd/rc checks it.
I prefer to make such dependencies explicit. Relying on indirect
dependencies is considered a bad practice in multiple coding style
guides since it makes it almost impossible to remove dependencies
from dependencies, e.g. removing "_have_module null_blk" from
"zbd/rc".
+test() {
+ set -e
Is this required? When I comment out this line and the "set +e" below,
still I was able to recreate the deadlock.
I want the entire test to fail if a single command fails. Debugging
shell scripts is much harder if a script continues after having
encountered an unexpected error.
I plan to address the other comments.
Thanks,
Bart.