* [LTP] [PATCH v3, 1/2] lib/tst_test.sh: Make tst_umount work with argument that has trailing slash
@ 2021-07-16 14:01 Leo Liang
2021-07-19 5:19 ` xuyang2018.jy
2021-07-19 6:49 ` Petr Vorel
0 siblings, 2 replies; 8+ messages in thread
From: Leo Liang @ 2021-07-16 14:01 UTC (permalink / raw)
To: ltp
/proc/mounts shows the mount point without trailing slashes, e.g.
~ $ cat /proc/mounts
xxx /root/cgroup cgroup rw,relatime,cpu 0 0
So current tst_umount would not work with argument that has trailing slash, e.g.
tst_umount cgroup/ would give "The device is not mounted".
Fix this by filtering out the trailing slash before grepping /proc/mounts.
Signed-off-by: Leo Yu-Chi Liang <ycliang@andestech.com>
---
testcases/lib/tst_test.sh | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
index c6aa2c487..f132512e7 100644
--- a/testcases/lib/tst_test.sh
+++ b/testcases/lib/tst_test.sh
@@ -282,13 +282,14 @@ tst_umount()
[ -z "$device" ] && return
+ device=${device%/}
if ! grep -q "$device" /proc/mounts; then
tst_res TINFO "The $device is not mounted, skipping umount"
return
fi
while [ "$i" -lt 50 ]; do
- if umount "$device" > /dev/null; then
+ if umount "$device"/ > /dev/null; then
return
fi
--
2.17.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [LTP] [PATCH v3, 1/2] lib/tst_test.sh: Make tst_umount work with argument that has trailing slash
2021-07-16 14:01 [LTP] [PATCH v3, 1/2] lib/tst_test.sh: Make tst_umount work with argument that has trailing slash Leo Liang
@ 2021-07-19 5:19 ` xuyang2018.jy
2021-07-19 5:51 ` Petr Vorel
2021-07-19 6:49 ` Petr Vorel
1 sibling, 1 reply; 8+ messages in thread
From: xuyang2018.jy @ 2021-07-19 5:19 UTC (permalink / raw)
To: ltp
Hi Leo
> /proc/mounts shows the mount point without trailing slashes, e.g.
> ~ $ cat /proc/mounts
> xxx /root/cgroup cgroup rw,relatime,cpu 0 0
>
> So current tst_umount would not work with argument that has trailing slash, e.g.
> tst_umount cgroup/ would give "The device is not mounted".
>
> Fix this by filtering out the trailing slash before grepping /proc/mounts.
>
> Signed-off-by: Leo Yu-Chi Liang<ycliang@andestech.com>
> ---
> testcases/lib/tst_test.sh | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
> index c6aa2c487..f132512e7 100644
> --- a/testcases/lib/tst_test.sh
> +++ b/testcases/lib/tst_test.sh
> @@ -282,13 +282,14 @@ tst_umount()
>
> [ -z "$device" ]&& return
>
> + device=${device%/}
> if ! grep -q "$device" /proc/mounts; then
> tst_res TINFO "The $device is not mounted, skipping umount"
> return
> fi
>
> while [ "$i" -lt 50 ]; do
> - if umount "$device"> /dev/null; then
> + if umount "$device"/> /dev/null; then
With removing this(we don't need add "/" here), this patch looks good to me
Reviewed-by: Yang Xu <xuyang2018.jy@fujitsu.com>
> return
> fi
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [LTP] [PATCH v3, 1/2] lib/tst_test.sh: Make tst_umount work with argument that has trailing slash
2021-07-19 5:19 ` xuyang2018.jy
@ 2021-07-19 5:51 ` Petr Vorel
2021-07-19 5:58 ` Petr Vorel
0 siblings, 1 reply; 8+ messages in thread
From: Petr Vorel @ 2021-07-19 5:51 UTC (permalink / raw)
To: ltp
> Hi Leo
> > /proc/mounts shows the mount point without trailing slashes, e.g.
> > ~ $ cat /proc/mounts
> > xxx /root/cgroup cgroup rw,relatime,cpu 0 0
> > So current tst_umount would not work with argument that has trailing slash, e.g.
> > tst_umount cgroup/ would give "The device is not mounted".
> > Fix this by filtering out the trailing slash before grepping /proc/mounts.
> > Signed-off-by: Leo Yu-Chi Liang<ycliang@andestech.com>
> > ---
> > testcases/lib/tst_test.sh | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> > diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
> > index c6aa2c487..f132512e7 100644
> > --- a/testcases/lib/tst_test.sh
> > +++ b/testcases/lib/tst_test.sh
> > @@ -282,13 +282,14 @@ tst_umount()
> > [ -z "$device" ]&& return
> > + device=${device%/}
> > if ! grep -q "$device" /proc/mounts; then
> > tst_res TINFO "The $device is not mounted, skipping umount"
> > return
> > fi
> > while [ "$i" -lt 50 ]; do
> > - if umount "$device"> /dev/null; then
> > + if umount "$device"/> /dev/null; then
> With removing this(we don't need add "/" here), this patch looks good to me
+1
Reviewed-by: Petr Vorel <pvorel@suse.cz>
Kind regards,
Petr
> Reviewed-by: Yang Xu <xuyang2018.jy@fujitsu.com>
> > return
> > fi
^ permalink raw reply [flat|nested] 8+ messages in thread
* [LTP] [PATCH v3, 1/2] lib/tst_test.sh: Make tst_umount work with argument that has trailing slash
2021-07-19 5:51 ` Petr Vorel
@ 2021-07-19 5:58 ` Petr Vorel
2021-07-19 6:20 ` xuyang2018.jy
2021-07-19 8:18 ` Leo Liang
0 siblings, 2 replies; 8+ messages in thread
From: Petr Vorel @ 2021-07-19 5:58 UTC (permalink / raw)
To: ltp
Hi all,
> > Hi Leo
> > > /proc/mounts shows the mount point without trailing slashes, e.g.
> > > ~ $ cat /proc/mounts
> > > xxx /root/cgroup cgroup rw,relatime,cpu 0 0
> > > So current tst_umount would not work with argument that has trailing slash, e.g.
> > > tst_umount cgroup/ would give "The device is not mounted".
> > > Fix this by filtering out the trailing slash before grepping /proc/mounts.
> > > Signed-off-by: Leo Yu-Chi Liang<ycliang@andestech.com>
> > > ---
> > > testcases/lib/tst_test.sh | 3 ++-
> > > 1 file changed, 2 insertions(+), 1 deletion(-)
> > > diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
> > > index c6aa2c487..f132512e7 100644
> > > --- a/testcases/lib/tst_test.sh
> > > +++ b/testcases/lib/tst_test.sh
> > > @@ -282,13 +282,14 @@ tst_umount()
> > > [ -z "$device" ]&& return
> > > + device=${device%/}
> > > if ! grep -q "$device" /proc/mounts; then
> > > tst_res TINFO "The $device is not mounted, skipping umount"
> > > return
> > > fi
> > > while [ "$i" -lt 50 ]; do
> > > - if umount "$device"> /dev/null; then
> > > + if umount "$device"/> /dev/null; then
> > With removing this(we don't need add "/" here), this patch looks good to me
> +1
Actually we need to keep / for next patch, right? (cgroup/)
Thus why not just changing argument for grep?
- if ! grep -q "$device" /proc/mounts; then
+ if ! grep -q "${device%/}" /proc/mounts; then
Kind regards,
Petr
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
> Kind regards,
> Petr
> > Reviewed-by: Yang Xu <xuyang2018.jy@fujitsu.com>
> > > return
> > > fi
^ permalink raw reply [flat|nested] 8+ messages in thread
* [LTP] [PATCH v3, 1/2] lib/tst_test.sh: Make tst_umount work with argument that has trailing slash
2021-07-19 5:58 ` Petr Vorel
@ 2021-07-19 6:20 ` xuyang2018.jy
2021-07-19 6:45 ` Petr Vorel
2021-07-19 8:18 ` Leo Liang
1 sibling, 1 reply; 8+ messages in thread
From: xuyang2018.jy @ 2021-07-19 6:20 UTC (permalink / raw)
To: ltp
Hi Petr
> Hi all,
>
>>> Hi Leo
>
>>>> /proc/mounts shows the mount point without trailing slashes, e.g.
>>>> ~ $ cat /proc/mounts
>>>> xxx /root/cgroup cgroup rw,relatime,cpu 0 0
>
>>>> So current tst_umount would not work with argument that has trailing slash, e.g.
>>>> tst_umount cgroup/ would give "The device is not mounted".
>
>>>> Fix this by filtering out the trailing slash before grepping /proc/mounts.
>
>>>> Signed-off-by: Leo Yu-Chi Liang<ycliang@andestech.com>
>>>> ---
>>>> testcases/lib/tst_test.sh | 3 ++-
>>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>>>> diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
>>>> index c6aa2c487..f132512e7 100644
>>>> --- a/testcases/lib/tst_test.sh
>>>> +++ b/testcases/lib/tst_test.sh
>>>> @@ -282,13 +282,14 @@ tst_umount()
>
>>>> [ -z "$device" ]&& return
>
>>>> + device=${device%/}
>>>> if ! grep -q "$device" /proc/mounts; then
>>>> tst_res TINFO "The $device is not mounted, skipping umount"
>>>> return
>>>> fi
>
>>>> while [ "$i" -lt 50 ]; do
>>>> - if umount "$device"> /dev/null; then
>>>> + if umount "$device"/> /dev/null; then
>>> With removing this(we don't need add "/" here), this patch looks good to me
>> +1
> Actually we need to keep / for next patch, right? (cgroup/)
I guess our ltp tst_umount api should support to umount cgroup or
cgroup/ like umount command does.
>
> Thus why not just changing argument for grep?
> - if ! grep -q "$device" /proc/mounts; then
> + if ! grep -q "${device%/}" /proc/mounts; then
Yes, it is more easier. But I think it still existed the problme when
we only use "/" parameters. I guess we should reject this situation.
code maybe as below:
--- a/testcases/lib/tst_test.sh
+++ b/testcases/lib/tst_test.sh
@@ -282,7 +282,12 @@ tst_umount()
[ -z "$device" ] && return
- if ! grep -q "$device" /proc/mounts; then
+ if [ "$device" = "/" ]; then
+ tst_res TINFO "We can not umount / directory"
+ return
+ fi
+
+ if ! grep -q "${device%/}" /proc/mounts; then
tst_res TINFO "The $device is not mounted, skipping umount"
return
fi
>
> Kind regards,
> Petr
>
>> Reviewed-by: Petr Vorel<pvorel@suse.cz>
>
>> Kind regards,
>> Petr
>
>>> Reviewed-by: Yang Xu<xuyang2018.jy@fujitsu.com>
>>>> return
>>>> fi
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [LTP] [PATCH v3, 1/2] lib/tst_test.sh: Make tst_umount work with argument that has trailing slash
2021-07-19 6:20 ` xuyang2018.jy
@ 2021-07-19 6:45 ` Petr Vorel
0 siblings, 0 replies; 8+ messages in thread
From: Petr Vorel @ 2021-07-19 6:45 UTC (permalink / raw)
To: ltp
Hi Xu, all,
> Hi Petr
> > Hi all,
> >>> Hi Leo
> >>>> /proc/mounts shows the mount point without trailing slashes, e.g.
> >>>> ~ $ cat /proc/mounts
> >>>> xxx /root/cgroup cgroup rw,relatime,cpu 0 0
> >>>> So current tst_umount would not work with argument that has trailing slash, e.g.
> >>>> tst_umount cgroup/ would give "The device is not mounted".
> >>>> Fix this by filtering out the trailing slash before grepping /proc/mounts.
> >>>> Signed-off-by: Leo Yu-Chi Liang<ycliang@andestech.com>
> >>>> ---
> >>>> testcases/lib/tst_test.sh | 3 ++-
> >>>> 1 file changed, 2 insertions(+), 1 deletion(-)
> >>>> diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
> >>>> index c6aa2c487..f132512e7 100644
> >>>> --- a/testcases/lib/tst_test.sh
> >>>> +++ b/testcases/lib/tst_test.sh
> >>>> @@ -282,13 +282,14 @@ tst_umount()
> >>>> [ -z "$device" ]&& return
> >>>> + device=${device%/}
> >>>> if ! grep -q "$device" /proc/mounts; then
> >>>> tst_res TINFO "The $device is not mounted, skipping umount"
> >>>> return
> >>>> fi
> >>>> while [ "$i" -lt 50 ]; do
> >>>> - if umount "$device"> /dev/null; then
> >>>> + if umount "$device"/> /dev/null; then
> >>> With removing this(we don't need add "/" here), this patch looks good to me
> >> +1
> > Actually we need to keep / for next patch, right? (cgroup/)
> I guess our ltp tst_umount api should support to umount cgroup or
> cgroup/ like umount command does.
Also the patch always added trailing /:
+ if umount "$device"/ > /dev/null; then
if we keep that, the second patch wouldn't have to add it to "tst_umount
cgroup/" call ("tst_umount cgroup" would be enough).
I'm not sure if we want it. @Cyril?
> > Thus why not just changing argument for grep?
> > - if ! grep -q "$device" /proc/mounts; then
> > + if ! grep -q "${device%/}" /proc/mounts; then
> Yes, it is more easier. But I think it still existed the problme when
> we only use "/" parameters. I guess we should reject this situation.
I would not care about it. 1) no test does 'tst_umount /' 2) error reporting
would be correct: umount: /: target is busy.
Kind regards,
Petr
> code maybe as below:
> --- a/testcases/lib/tst_test.sh
> +++ b/testcases/lib/tst_test.sh
> @@ -282,7 +282,12 @@ tst_umount()
> [ -z "$device" ] && return
> - if ! grep -q "$device" /proc/mounts; then
> + if [ "$device" = "/" ]; then
> + tst_res TINFO "We can not umount / directory"
> + return
> + fi
> +
> + if ! grep -q "${device%/}" /proc/mounts; then
> tst_res TINFO "The $device is not mounted, skipping umount"
> return
> fi
^ permalink raw reply [flat|nested] 8+ messages in thread
* [LTP] [PATCH v3, 1/2] lib/tst_test.sh: Make tst_umount work with argument that has trailing slash
2021-07-16 14:01 [LTP] [PATCH v3, 1/2] lib/tst_test.sh: Make tst_umount work with argument that has trailing slash Leo Liang
2021-07-19 5:19 ` xuyang2018.jy
@ 2021-07-19 6:49 ` Petr Vorel
1 sibling, 0 replies; 8+ messages in thread
From: Petr Vorel @ 2021-07-19 6:49 UTC (permalink / raw)
To: ltp
Hi,
...
> while [ "$i" -lt 50 ]; do
> - if umount "$device" > /dev/null; then
> + if umount "$device"/ > /dev/null; then
nit: although previous is working, I'd prefer "$device/"
Kind regards,
Petr
^ permalink raw reply [flat|nested] 8+ messages in thread
* [LTP] [PATCH v3, 1/2] lib/tst_test.sh: Make tst_umount work with argument that has trailing slash
2021-07-19 5:58 ` Petr Vorel
2021-07-19 6:20 ` xuyang2018.jy
@ 2021-07-19 8:18 ` Leo Liang
1 sibling, 0 replies; 8+ messages in thread
From: Leo Liang @ 2021-07-19 8:18 UTC (permalink / raw)
To: ltp
Hi all,
On Mon, Jul 19, 2021 at 01:58:00PM +0800, Petr Vorel wrote:
> Hi all,
>
> > > Hi Leo
>
> > > > /proc/mounts shows the mount point without trailing slashes, e.g.
> > > > ~ $ cat /proc/mounts
> > > > xxx /root/cgroup cgroup rw,relatime,cpu 0 0
>
> > > > So current tst_umount would not work with argument that has trailing slash, e.g.
> > > > tst_umount cgroup/ would give "The device is not mounted".
>
> > > > Fix this by filtering out the trailing slash before grepping /proc/mounts.
>
> > > > Signed-off-by: Leo Yu-Chi Liang<ycliang@andestech.com>
> > > > ---
> > > > testcases/lib/tst_test.sh | 3 ++-
> > > > 1 file changed, 2 insertions(+), 1 deletion(-)
>
> > > > diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
> > > > index c6aa2c487..f132512e7 100644
> > > > --- a/testcases/lib/tst_test.sh
> > > > +++ b/testcases/lib/tst_test.sh
> > > > @@ -282,13 +282,14 @@ tst_umount()
>
> > > > [ -z "$device" ]&& return
>
> > > > + device=${device%/}
> > > > if ! grep -q "$device" /proc/mounts; then
> > > > tst_res TINFO "The $device is not mounted, skipping umount"
> > > > return
> > > > fi
>
> > > > while [ "$i" -lt 50 ]; do
> > > > - if umount "$device"> /dev/null; then
> > > > + if umount "$device"/> /dev/null; then
> > > With removing this(we don't need add "/" here), this patch looks good to me
> > +1
> Actually we need to keep / for next patch, right? (cgroup/)
Yes, I thought so as well, for commit 46172493.
> Thus why not just changing argument for grep?
> - if ! grep -q "$device" /proc/mounts; then
> + if ! grep -q "${device%/}" /proc/mounts; then
Thanks! That's a better way to do it.
Will send a v4 patch later with this modification.
Best regards,
Leo
> Kind regards,
> Petr
>
> > Reviewed-by: Petr Vorel <pvorel@suse.cz>
>
> > Kind regards,
> > Petr
>
> > > Reviewed-by: Yang Xu <xuyang2018.jy@fujitsu.com>
> > > > return
> > > > fi
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-07-19 8:18 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-16 14:01 [LTP] [PATCH v3, 1/2] lib/tst_test.sh: Make tst_umount work with argument that has trailing slash Leo Liang
2021-07-19 5:19 ` xuyang2018.jy
2021-07-19 5:51 ` Petr Vorel
2021-07-19 5:58 ` Petr Vorel
2021-07-19 6:20 ` xuyang2018.jy
2021-07-19 6:45 ` Petr Vorel
2021-07-19 8:18 ` Leo Liang
2021-07-19 6:49 ` Petr Vorel
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.