Messages in this thread | | | From | zhaogongyi <> | Subject | Re: [PATCH -next v3 1/3] selftests/memory-hotplug: Add checking after online or offline | Date | Tue, 27 Sep 2022 01:36:18 +0000 |
| |
> > On 26.09.22 15:03, Zhao Gongyi wrote: > > Add checking for online_memory_expect_success()/ > > offline_memory_expect_success()/offline_memory_expect_fail(), or > the > > test would exit 0 although the functions return 1. > > > > Signed-off-by: Zhao Gongyi <zhaogongyi@huawei.com> > > --- > > .../selftests/memory-hotplug/mem-on-off-test.sh | 15 > ++++++++++++--- > > 1 file changed, 12 insertions(+), 3 deletions(-) > > > > diff --git a/tools/testing/selftests/memory-hotplug/mem-on-off-test.sh > > b/tools/testing/selftests/memory-hotplug/mem-on-off-test.sh > > index 46a97f318f58..3edda1f13f7b 100755 > > --- a/tools/testing/selftests/memory-hotplug/mem-on-off-test.sh > > +++ b/tools/testing/selftests/memory-hotplug/mem-on-off-test.sh > > @@ -266,7 +266,10 @@ done > > # > > echo $error > > $NOTIFIER_ERR_INJECT_DIR/actions/MEM_GOING_ONLINE/error > > for memory in `hotpluggable_offline_memory`; do > > - online_memory_expect_fail $memory > > + online_memory_expect_fail $memory || { > > + echo "online memory $memory: unexpected success" > > The functions themself already print an error, isn't it sufficient to set > retval=1?
Indeed, this function is only called once. It is no need to add the log info.
> > > + retval=1 > > + } > > done > > > > # > > @@ -274,7 +277,10 @@ done > > # > > echo 0 > > $NOTIFIER_ERR_INJECT_DIR/actions/MEM_GOING_ONLINE/error > > for memory in `hotpluggable_offline_memory`; do > > - online_memory_expect_success $memory > > + online_memory_expect_success $memory || { > > + echo "online memory $memory: unexpected fail" > > + retval=1 > > + } > > done > > > > # > > @@ -283,7 +289,10 @@ done > > echo $error > > $NOTIFIER_ERR_INJECT_DIR/actions/MEM_GOING_OFFLINE/error > > for memory in `hotpluggable_online_memory`; do > > if [ $((RANDOM % 100)) -lt $ratio ]; then > > - offline_memory_expect_fail $memory > > + offline_memory_expect_fail $memory || { > > + echo "offline memory $memory: unexpected success" > > + retval=1 > > + } > > These functions return 0 if the result is as expected and 1 if the result is > unexpected. > > ... but wouldn't we evaluate the right hand side only if the result is "0" -- > expected? I might be wrong. >
Yes, if offline_memory_expect_fail's return value is not zero, then it will set 'retval=1'
> > Wouldn't it be simpler do it as in "Online all hot-pluggable memory again" > > if ! online_memory_expect_success $memory; then > retval=1 > fi > > (similarly adjusting the function name)
I will send a new version of the patch to fix it.
Kind regards, Gongyi
| |