[LTP] [PATCH v4, 2/2] cgroup/cgroup_regression_test: Fix umount failure

Cyril Hrubis chrubis@suse.cz
Tue Jul 27 15:09:05 CEST 2021


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


More information about the ltp mailing list