On Apr 22 01:12, Shinichiro Kawasaki wrote:
> Hello Klaus,
>
Hi Shinichiro,
Thanks for taking a look.
> Thank you for this patch. I also face the unexpected "not run" messages and this
> patch fixes them. I made a couple of comments on your patch in line.
>
> On Apr 21, 2020 / 09:33, Klaus Jensen wrote:
> >
> > +_test_dev_can_discard() {
> > + if [[ $(cat "${TEST_DEV_SYSFS}/queue/discard_max_bytes") -eq 0 ]]; then
> > + return 1
> > + fi
> > + return 0
> > +}
> > +
> > +_require_test_dev_can_discard() {
> > + if ! _test_dev_can_discard; then
> > + SKIP_REASON="$TEST_DEV does not support discard"
> > + return 1
> > + fi
> > + return 0
> > +}
> > +
>
> Don't we need replace _test_dev_can_discard() in block/003 with
> _require_test_dev_can_discard()?
>
Thought I got them all... Good catch!
> >
> > -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.
