| Message ID | 20230919114701.15327-4-mdoucha@suse.cz |
|---|---|
| State | Accepted |
| Headers | show |
| Series | NFS test fixes | expand |
Hi Martin, > Signed-off-by: Martin Doucha <mdoucha@suse.cz> > --- > testcases/network/nfs/nfs_stress/nfs_lib.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > diff --git a/testcases/network/nfs/nfs_stress/nfs_lib.sh b/testcases/network/nfs/nfs_stress/nfs_lib.sh > index a996f7cc8..099c78759 100644 > --- a/testcases/network/nfs/nfs_stress/nfs_lib.sh > +++ b/testcases/network/nfs/nfs_stress/nfs_lib.sh > @@ -214,7 +214,7 @@ nfs_cleanup() > local_dir="$(get_local_dir $i $n)" > if grep -q "$local_dir" /proc/mounts; then > tst_res TINFO "Unmounting $local_dir" > - umount $local_dir > + umount $local_dir || tst_res TFAIL "Unmount failed" I'd personally add TWARN (i.e. less critical error). @Cyril WDYT? Reviewed-by: Petr Vorel <pvorel@suse.cz> Kind regards, Petr > fi > n=$(( n + 1 )) > done
Reviewed-by: Richard Palethorpe <rpalethorpe@suse.com> Martin Doucha <mdoucha@suse.cz> writes: > Signed-off-by: Martin Doucha <mdoucha@suse.cz> > --- > testcases/network/nfs/nfs_stress/nfs_lib.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/testcases/network/nfs/nfs_stress/nfs_lib.sh b/testcases/network/nfs/nfs_stress/nfs_lib.sh > index a996f7cc8..099c78759 100644 > --- a/testcases/network/nfs/nfs_stress/nfs_lib.sh > +++ b/testcases/network/nfs/nfs_stress/nfs_lib.sh > @@ -214,7 +214,7 @@ nfs_cleanup() > local_dir="$(get_local_dir $i $n)" > if grep -q "$local_dir" /proc/mounts; then > tst_res TINFO "Unmounting $local_dir" > - umount $local_dir > + umount $local_dir || tst_res TFAIL "Unmount failed" > fi > n=$(( n + 1 )) > done > -- > 2.42.0
Hi! > diff --git a/testcases/network/nfs/nfs_stress/nfs_lib.sh b/testcases/network/nfs/nfs_stress/nfs_lib.sh > index a996f7cc8..099c78759 100644 > --- a/testcases/network/nfs/nfs_stress/nfs_lib.sh > +++ b/testcases/network/nfs/nfs_stress/nfs_lib.sh > @@ -214,7 +214,7 @@ nfs_cleanup() > local_dir="$(get_local_dir $i $n)" > if grep -q "$local_dir" /proc/mounts; then > tst_res TINFO "Unmounting $local_dir" > - umount $local_dir > + umount $local_dir || tst_res TFAIL "Unmount failed" I suppose that this should be TBROK instead. And that this, apart from the previous patches, should be applied after the release. Otherwise: Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
> Hi! > > diff --git a/testcases/network/nfs/nfs_stress/nfs_lib.sh b/testcases/network/nfs/nfs_stress/nfs_lib.sh > > index a996f7cc8..099c78759 100644 > > --- a/testcases/network/nfs/nfs_stress/nfs_lib.sh > > +++ b/testcases/network/nfs/nfs_stress/nfs_lib.sh > > @@ -214,7 +214,7 @@ nfs_cleanup() > > local_dir="$(get_local_dir $i $n)" > > if grep -q "$local_dir" /proc/mounts; then > > tst_res TINFO "Unmounting $local_dir" > > - umount $local_dir > > + umount $local_dir || tst_res TFAIL "Unmount failed" > I suppose that this should be TBROK instead. And that this, apart from Right, TBROK looks to be the best. Martin, if you're ok with the change, I'll update it before merge (after the release). Kind regards, Petr > the previous patches, should be applied after the release. > Otherwise: > Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
On 25. 09. 23 15:24, Petr Vorel wrote: >> Hi! >>> diff --git a/testcases/network/nfs/nfs_stress/nfs_lib.sh b/testcases/network/nfs/nfs_stress/nfs_lib.sh >>> index a996f7cc8..099c78759 100644 >>> --- a/testcases/network/nfs/nfs_stress/nfs_lib.sh >>> +++ b/testcases/network/nfs/nfs_stress/nfs_lib.sh >>> @@ -214,7 +214,7 @@ nfs_cleanup() >>> local_dir="$(get_local_dir $i $n)" >>> if grep -q "$local_dir" /proc/mounts; then >>> tst_res TINFO "Unmounting $local_dir" >>> - umount $local_dir >>> + umount $local_dir || tst_res TFAIL "Unmount failed" > >> I suppose that this should be TBROK instead. And that this, apart from > > Right, TBROK looks to be the best. > Martin, if you're ok with the change, I'll update it before merge (after the > release). I don't see the point. This is a cleanup function. The TBROK will be changed to TWARN and the test will continue anyway.
Hi, ... > > > > - umount $local_dir > > > > + umount $local_dir || tst_res TFAIL "Unmount failed" > > > I suppose that this should be TBROK instead. And that this, apart from > > Right, TBROK looks to be the best. > > Martin, if you're ok with the change, I'll update it before merge (after the > > release). > I don't see the point. This is a cleanup function. The TBROK will be changed > to TWARN and the test will continue anyway. But TFAIL in cleanup function looks to me strange as well. Should we endup with TWARN then? Kind regards, Petr
On 25. 09. 23 19:40, Petr Vorel wrote: > Hi, > > ... >>>>> - umount $local_dir >>>>> + umount $local_dir || tst_res TFAIL "Unmount failed" > >>>> I suppose that this should be TBROK instead. And that this, apart from > >>> Right, TBROK looks to be the best. >>> Martin, if you're ok with the change, I'll update it before merge (after the >>> release). >> I don't see the point. This is a cleanup function. The TBROK will be changed >> to TWARN and the test will continue anyway. > > But TFAIL in cleanup function looks to me strange as well. Should we endup with > TWARN then? TWARN makes sense. Feel free to change it during merge.
> On 25. 09. 23 19:40, Petr Vorel wrote: > > Hi, > > ... > > > > > > - umount $local_dir > > > > > > + umount $local_dir || tst_res TFAIL "Unmount failed" > > > > > I suppose that this should be TBROK instead. And that this, apart from > > > > Right, TBROK looks to be the best. > > > > Martin, if you're ok with the change, I'll update it before merge (after the > > > > release). > > > I don't see the point. This is a cleanup function. The TBROK will be changed > > > to TWARN and the test will continue anyway. > > But TFAIL in cleanup function looks to me strange as well. Should we endup with > > TWARN then? > TWARN makes sense. Feel free to change it during merge. Good, changed and merged. Thanks! Kind regards, Petr
diff --git a/testcases/network/nfs/nfs_stress/nfs_lib.sh b/testcases/network/nfs/nfs_stress/nfs_lib.sh index a996f7cc8..099c78759 100644 --- a/testcases/network/nfs/nfs_stress/nfs_lib.sh +++ b/testcases/network/nfs/nfs_stress/nfs_lib.sh @@ -214,7 +214,7 @@ nfs_cleanup() local_dir="$(get_local_dir $i $n)" if grep -q "$local_dir" /proc/mounts; then tst_res TINFO "Unmounting $local_dir" - umount $local_dir + umount $local_dir || tst_res TFAIL "Unmount failed" fi n=$(( n + 1 )) done
Signed-off-by: Martin Doucha <mdoucha@suse.cz> --- testcases/network/nfs/nfs_stress/nfs_lib.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)