New issue
Advanced search Search tips

Issue 776302 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Feature



Sign in to add a comment

maitre'd should unmount all filesystems before rebooting

Project Member Reported by chirantan@chromium.org, Oct 19 2017

Issue description

maitre'd should ensure that all file systems are unmounted before rebooting the VM to prevent data corruption in writable file systems.  It should do this after all processes have exited (or been killed) and before actually making the reboot system call.

The easiest way I can think of to do this is to read /proc/self/mountinfo, build a graph of the mount dependencies (for example, we can't unmount /sys/fs/cgroup before unmounting /sys/fs/cgroup/cpu), do a topological sort, and then unmount everything in order.

The only thing I'm a bit unsure about is other mount namespaces.  If the last process in a mount namespace exits do the unique mounts in that namespace (ones that don't appear in other namespaces) automatically get unmounted?  If not, will they show up in /proc/self/mountinfo?

The man page (http://man7.org/linux/man-pages/man7/mount_namespaces.7.html) seems to suggest that the mount is automatically unmounted when the namespace is empty but it says this only in passing when talking about peer groups:

 > "A mount ceases to be a member of a peer group when either the mount is explicitly unmounted, or when the mount is implicitly unmounted because a mount namespace is removed (because it has no more member processes)."

Seems like important information that people might want to know.
 
Actually, now that I think about it if we set things up so that we only mount writable storage inside the container's mount namespace then we wouldn't need to do anything because it would automatically get unmounted when the last container process exits and everything outside the container would either be a read-only mount or a tmpfs.

Comment 2 by vapier@chromium.org, Oct 19 2017

there's no need to get fancy with mountinfo.  remember that the order of the file reflects the order of mount calls made, so it's always going to be already sorted.  you simply have to walk it backwards.  this is how distros do it already.

depending on the access given to people, there might be other references hanging around keeping things from being unmounted (like loopback files).  and as mentioned in the CL, we want this tear down to be more of a safety/sanity check against bugs in the overall system.

i was thinking we'd do:
- walk all the loop devices and detach them (they might not get detached right away if they're mounted)
- break down all device mappers (they might not get detached right away if they're mounted)
- walk all the mounts in reverse and unmount them (except for critical ones like /sys /proc /dev), and if unmount fails, try to remount read-only at least
- call sync
- reboot
Project Member

Comment 3 by bugdroid1@chromium.org, Nov 22 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/394fe1038ed868447878d1166778169af3a94186

commit 394fe1038ed868447878d1166778169af3a94186
Author: Chirantan Ekbote <chirantan@chromium.org>
Date: Wed Nov 22 02:53:32 2017

vm_tools: maitred: Clean up mounts before shutdown

Detach all loopback devices, remove device-mapper devices, and unmount
non-essential filesystems before shutting down the VM.

BUG= chromium:776302 
TEST=manual

Change-Id: If521d604faff69b3abc67c13b7b3a863193ed99e
Signed-off-by: Chirantan Ekbote <chirantan@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/729555
Reviewed-by: Ben Chan <benchan@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/394fe1038ed868447878d1166778169af3a94186/vm_tools/maitred/main.cc
[modify] https://crrev.com/394fe1038ed868447878d1166778169af3a94186/vm_tools/maitred/init.cc

Status: Fixed (was: Assigned)

Sign in to add a comment