curtin-dev team mailing list archive

[Michael Hudson-Doyle <mp+406682@xxxxxxxxxxxxxxxxxx>]

Diff comments:

> diff --git a/curtin/util.py b/curtin/util.py
> index ec1d4f9..c02580c 100644
> --- a/curtin/util.py
> +++ b/curtin/util.py
> @@ -550,10 +550,14 @@ def do_umount(mountpoint, recursive=False):
>      #
>      # return boolean indicating if mountpoint was previously mounted.
>      ret = False
> +    if recursive:
> +        for line in load_file("/proc/mounts", decode=True).splitlines():

I'm pretty sure /proc/mounts is in the order the mounts happened, although I'm not really finding anything definitive that says so. I'm also pretty sure that this code is already broken in the face of doing things like this:

mwhudson@anduril:~/tmp/mt$ mkdir a a/b c
mwhudson@anduril:~/tmp/mt$ sudo mount -t sysfs sysfs a/b
mwhudson@anduril:~/tmp/mt$ sudo mount --bind c a

A mount at a/b still shows up in /proc/mounts but it's inaccessible and obviously umount a/b will fail. I don't think there's a way to defend against this other than turning the code on its head and walking the directory tree under the provided path and seeing if each directory is a mountpoint but I also don't think we need to bother.

The point about not repeatedly parsing /proc/mounts makes sense though, I'll change that.

> +            curmp = line.split()[1]
> +            if curmp == mp or curmp.startswith(mp + os.path.sep):
> +                subp(['mount', '--make-private', curmp])
>      for line in reversed(load_file("/proc/mounts", decode=True).splitlines()):
>          curmp = line.split()[1]
>          if curmp == mp or (recursive and curmp.startswith(mp + os.path.sep)):
> -            subp(['mount', '--make-private', curmp])
>              subp(['umount', curmp])
>          if curmp == mp:
>              ret = True


-- 
https://code.launchpad.net/~mwhudson/curtin/+git/curtin/+merge/406682
Your team curtin developers is requested to review the proposed merge of ~mwhudson/curtin:even-more-unmounting-tweaks into curtin:master.