public inbox for ltp@lists.linux.it
 help / color / mirror / Atom feed
From: Cyril Hrubis <chrubis@suse.cz>
To: ltp@lists.linux.it
Subject: [LTP] [PATCH v4, 2/2] cgroup/cgroup_regression_test: Fix umount failure
Date: Tue, 27 Jul 2021 15:09:05 +0200	[thread overview]
Message-ID: <YQAFcVwvz+eXGOsk@yuki> (raw)
In-Reply-To: <62262681-222f-8d09-a100-0d7be0c7526f@jv-coder.de>

Hi!
> I had a first look at this patches and was curious, what the reasoning 
> behind the "/" is.
> The comment you suggest is wrong. The / was introduced to prevent 
> unmounting some other mountpoint,
> where the device was cgroup.
> Imho the approach of adding a / to the end was wrong and intransparent. 
> I would rather use "./cgroup" or "$PWD/cgroup".

Passing full path to the cgroup directory sound much safer to me
especially when the directory name is just 'cgroup', try it yourself:

device=cgroup/; grep "${device%/}" /proc/mounts

On my machine this yields 10 lines and 21 matches.

> If possible, I'd actually change tst_umount, to always unmount the 
> mountpoint and not the device, i.e. if the given path is not an absolute 
> path, make it absolute (e.g. by prepending $PWD").
> This way the check if the mountpoint exist wouldn't be the fuzzy thing 
> it is right now.

Strongly agree here.

I would go even one step further and change the library so that it
rejects anything that does not start with '/'.

-- 
Cyril Hrubis
chrubis@suse.cz

  parent reply	other threads:[~2021-07-27 13:09 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-19  9:22 [LTP] [PATCH v4, 2/2] cgroup/cgroup_regression_test: Fix umount failure Leo Liang
2021-07-21 14:37 ` Petr Vorel
2021-07-22  4:35   ` Joerg Vehlow
2021-07-22  6:32     ` Leo Liang
2021-07-22  6:37       ` Joerg Vehlow
2021-07-27  5:27         ` Leo Liang
2021-07-27 13:09     ` Cyril Hrubis [this message]
2021-07-27 13:53       ` Petr Vorel
2021-07-29  7:41         ` Leo Liang
2021-07-22  4:55   ` Leo Liang

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=YQAFcVwvz+eXGOsk@yuki \
    --to=chrubis@suse.cz \
    --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