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
