On Apr 22 06:49, Shinichiro Kawasaki wrote: > On Apr 22, 2020 / 07:29, Klaus Birkelund Jensen wrote: > > On Apr 22 07:13, Klaus Birkelund Jensen wrote: > > > On Apr 22 01:12, Shinichiro Kawasaki wrote: > > > > On Apr 21, 2020 / 09:33, Klaus Jensen wrote: > > > > > > > > > > -device_requires() { > > > > > - ! _test_dev_is_zoned || _have_fio_zbd_zonemode > > > > > -} > > > > > - > > > > > test_device() { > > > > > echo "Running ${TEST_NAME}" > > > > > > > > > > @@ -25,6 +21,10 @@ test_device() { > > > > > local zbdmode="" > > > > > > > > > > if _test_dev_is_zoned; then > > > > > + if ! _have_fio_zbd_zonemode; then > > > > > + return > > > > > + fi > > > > > + > > > > > > > > This check is equivalent to device_requires() you removed, isn't it? > > > > If the skip check in test_device() is the last resort, it would be the > > > > better to check in device_requires(), I think. > > > > > > > > > > I did fix something... just not the real problem I think. > > > > > > Negations doesnt really work well in device_requires. If changed to > > > > > > ! _require_test_dev_is_zoned || _have_fio_zbd_zonemode > > > > > > then, for non-zoned devices, even though device_requires returns 0, > > > SKIP_REASON ends up being set to "is not a zoned block device" and skips > > > the test in _call_test due to this. > > > > > > There are two fixes; either we add a _require_test_dev_is_not_zoned > > > again or put the negated check in an arithmetic context, that is > > > > > > (( !_require_test_dev_is_zoned )) || _have_fio_zbd_zonemode > > > > > > I think the second option is a hack, so we'd better go with the first > > > choice. > > > > Doh. > > > > The _is_not_zoned version would of course just cause the test to be > > skipped for zoned devices instead. > > > > So I actually think my original fix is the best option here. > > Thank you for sharing your thoghts. I agree not to use _is_not_zoned version. > > I think _test_dev_is_zoned in device_requires() does not need to be replaced > with _require_test_dev_is_zoned. It does not check requirement. It just checks > if _have_fio_zbd_zonemode check is required or not. Right, that is a good observation. I'll revert my change there and keep it as device_requires() { ! _test_dev_is_zoned || _have_fio_zbd_zonemode } Thanks, Klaus