From: xuyang2018.jy@fujitsu.com <xuyang2018.jy@fujitsu.com>
To: ltp@lists.linux.it
Subject: [LTP] [PATCH v3, 1/2] lib/tst_test.sh: Make tst_umount work with argument that has trailing slash
Date: Mon, 19 Jul 2021 06:20:47 +0000 [thread overview]
Message-ID: <60F519D1.9040508@fujitsu.com> (raw)
In-Reply-To: <YPUUaOMUjRMy43ZH@pevik>
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
>
next prev parent reply other threads:[~2021-07-19 6:20 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
2021-07-19 6:45 ` Petr Vorel
2021-07-19 8:18 ` Leo Liang
2021-07-19 6:49 ` Petr Vorel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=60F519D1.9040508@fujitsu.com \
--to=xuyang2018.jy@fujitsu.com \
--cc=ltp@lists.linux.it \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox