Loop umount'ing rootfs until there are no more mounts

Ian Campbell · Ian Campbell · commit d42cb88ba2b0 · 2017-07-20T10:50:08.000+01:00
This is simpler than trying to count how many successful mounts we made.

Signed-off-by: Ian Campbell &lt;ian.campbell@docker.com&gt;
diff --git a/linux/shim/init.go b/linux/shim/init.go
@@ -39,19 +39,18 @@ type initProcess struct {
 	// the reaper interface.
 	mu sync.Mutex
 
-	id           string
-	bundle       string
-	console      console.Console
-	io           runc.IO
-	runtime      *runc.Runc
-	status       int
-	exited       time.Time
-	pid          int
-	closers      []io.Closer
-	stdin        io.Closer
-	stdio        stdio
-	rootfs       string
-	nrRootMounts int // Number of rootfs overmounts
+	id      string
+	bundle  string
+	console console.Console
+	io      runc.IO
+	runtime *runc.Runc
+	status  int
+	exited  time.Time
+	pid     int
+	closers []io.Closer
+	stdin   io.Closer
+	stdio   stdio
+	rootfs  string
 }
 
 func newInitProcess(context context.Context, path, namespace string, r *shimapi.CreateTaskRequest) (*initProcess, error) {
@@ -73,12 +72,11 @@ func newInitProcess(context context.Context, path, namespace string, r *shimapi.
 	// count the number of successful mounts so we can undo
 	// what was actually done rather than what should have been
 	// done.
-	nrRootMounts := 0
 	defer func() {
 		if success {
 			return
 		}
-		if err2 := mount.UnmountN(rootfs, 0, nrRootMounts); err2 != nil {
+		if err2 := mount.UnmountAll(rootfs, 0); err2 != nil {
 			log.G(context).WithError(err2).Warn("Failed to cleanup rootfs mount")
 		}
 	}()
@@ -91,7 +89,6 @@ func newInitProcess(context context.Context, path, namespace string, r *shimapi.
 		if err := m.Mount(rootfs); err != nil {
 			return nil, errors.Wrapf(err, "failed to mount rootfs component %v", m)
 		}
-		nrRootMounts++
 	}
 	runtime := &runc.Runc{
 		Command:      r.Runtime,
@@ -110,8 +107,7 @@ func newInitProcess(context context.Context, path, namespace string, r *shimapi.
 			stderr:   r.Stderr,
 			terminal: r.Terminal,
 		},
-		rootfs:       rootfs,
-		nrRootMounts: nrRootMounts,
+		rootfs: rootfs,
 	}
 	var (
 		err    error
@@ -253,7 +249,7 @@ func (p *initProcess) Delete(context context.Context) error {
 	}
 	err = p.runtimeError(err, "OCI runtime delete failed")
 
-	if err2 := mount.UnmountN(p.rootfs, 0, p.nrRootMounts); err2 != nil {
+	if err2 := mount.UnmountAll(p.rootfs, 0); err2 != nil {
 		log.G(context).WithError(err2).Warn("Failed to cleanup rootfs mount")
 		if err == nil {
 			err = errors.Wrap(err2, "Failed rootfs umount")
diff --git a/mount/mount.go b/mount/mount.go
@@ -22,19 +22,3 @@ func MountAll(mounts []Mount, target string) error {
 	}
 	return nil
 }
-
-// UnmountN tries to unmount the given mount point nr times, which is
-// useful for undoing a stack of mounts on the same mount
-// point. Returns the first error encountered, but always attempts the
-// full nr umounts.
-func UnmountN(mount string, flags, nr int) error {
-	var err error
-	for i := 0; i < nr; i++ {
-		if err2 := Unmount(mount, flags); err2 != nil {
-			if err == nil {
-				err = err2
-			}
-		}
-	}
-	return err
-}
diff --git a/mount/mount_linux.go b/mount/mount_linux.go
@@ -15,6 +15,25 @@ func Unmount(mount string, flags int) error {
 	return unix.Unmount(mount, flags)
 }
 
+// UnmountAll repeatedly unmounts the given mount point until there
+// are no mounts remaining (EINVAL is returned by mount), which is
+// useful for undoing a stack of mounts on the same mount point.
+func UnmountAll(mount string, flags int) error {
+	for {
+		if err := Unmount(mount, flags); err != nil {
+			// EINVAL is returned if the target is not a
+			// mount point, indicating that we are
+			// done. It can also indicate a few other
+			// things (such as invalid flags) which we
+			// unfortunately end up squelching here too.
+			if err == unix.EINVAL {
+				return nil
+			}
+			return err
+		}
+	}
+}
+
 // parseMountOptions takes fstab style mount options and parses them for
 // use with a standard mount() syscall
 func parseMountOptions(options []string) (int, string) {
diff --git a/mount/mount_unix.go b/mount/mount_unix.go
@@ -15,3 +15,7 @@ func (m *Mount) Mount(target string) error {
 func Unmount(mount string, flags int) error {
 	return ErrNotImplementOnUnix
 }
+
+func UnmountAll(mount string, flags int) error {
+	return ErrNotImplementOnUnix
+}
diff --git a/mount/mount_windows.go b/mount/mount_windows.go
@@ -13,3 +13,7 @@ func (m *Mount) Mount(target string) error {
 func Unmount(mount string, flags int) error {
 	return ErrNotImplementOnWindows
 }
+
+func UnmountAll(mount string, flags int) error {
+	return ErrNotImplementOnWindows
+}

Original file line number	Diff line number	Diff line change
`@@ -15,3 +15,7 @@ func (m *Mount) Mount(target string) error {`
`15`	`15`	`func Unmount(mount string, flags int) error {`
`16`	`16`	`return ErrNotImplementOnUnix`
`17`	`17`	`}`
	`18`	`+`
	`19`	`+func UnmountAll(mount string, flags int) error {`
	`20`	`+ return ErrNotImplementOnUnix`
	`21`	`+}`
Original file line number	Diff line number	Diff line change
`@@ -13,3 +13,7 @@ func (m *Mount) Mount(target string) error {`
`13`	`13`	`func Unmount(mount string, flags int) error {`
`14`	`14`	`return ErrNotImplementOnWindows`
`15`	`15`	`}`
	`16`	`+`
	`17`	`+func UnmountAll(mount string, flags int) error {`
	`18`	`+ return ErrNotImplementOnWindows`
	`19`	`+}`