Merge pull request #3512 from kolyshkin/fix-mntns-userns-II

Refactor mountFd code

Author: kolyshkin

libcontainer/console_linux.go

@@ -18,7 +18,7 @@ func mountConsole(slavePath string) error {
 	if f != nil {
 		f.Close()
 	}
-	return mount(slavePath, "/dev/console", "", "bind", unix.MS_BIND, "")
+	return mount(slavePath, "/dev/console", "bind", unix.MS_BIND, "")
 }
 
 // dupStdio opens the slavePath for the console and dups the fds to the current

libcontainer/container_linux.go

@@ -1277,9 +1277,8 @@ func (c *Container) makeCriuRestoreMountpoints(m *configs.Mount) error {
 	case "bind":
 		// The prepareBindMount() function checks if source
 		// exists. So it cannot be used for other filesystem types.
-		// TODO: pass something else than nil? Not sure if criu is
-		// impacted by issue #2484
-		if err := prepareBindMount(m, c.config.Rootfs, nil); err != nil {
+		// TODO: pass srcFD? Not sure if criu is impacted by issue #2484.
+		if err := prepareBindMount(mountEntry{Mount: m}, c.config.Rootfs); err != nil {
 			return err
 		}
 	default:
@@ -1357,8 +1356,8 @@ func (c *Container) prepareCriuRestoreMounts(mounts []*configs.Mount) error {
 			// because during initial container creation mounts are
 			// set up in the order they are configured.
 			if m.Device == "bind" {
-				if err := utils.WithProcfd(c.config.Rootfs, m.Destination, func(procfd string) error {
-					if err := mount(m.Source, m.Destination, procfd, "", unix.MS_BIND|unix.MS_REC, ""); err != nil {
+				if err := utils.WithProcfd(c.config.Rootfs, m.Destination, func(dstFD string) error {
+					if err := mountViaFDs(m.Source, "", m.Destination, dstFD, "", unix.MS_BIND|unix.MS_REC, ""); err != nil {
 						return err
 					}
 					return nil
@@ -1411,7 +1410,7 @@ func (c *Container) Restore(process *Process, criuOpts *CriuOpts) error {
 	if err != nil {
 		return err
 	}
-	err = mount(c.config.Rootfs, root, "", "", unix.MS_BIND|unix.MS_REC, "")
+	err = mount(c.config.Rootfs, root, "", unix.MS_BIND|unix.MS_REC, "")
 	if err != nil {
 		return err
 	}

libcontainer/mount_linux.go

@@ -10,8 +10,9 @@ import (
 type mountError struct {
 	op     string
 	source string
+	srcFD  string
 	target string
-	procfd string
+	dstFD  string
 	flags  uintptr
 	data   string
 	err    error
@@ -22,19 +23,21 @@ func (e *mountError) Error() string {
 	out := e.op + " "
 
 	if e.source != "" {
-		out += e.source + ":" + e.target
-	} else {
-		out += e.target
+		out += "src=" + e.source + ", "
+		if e.srcFD != "" {
+			out += "srcFD=" + e.srcFD + ", "
+		}
 	}
-	if e.procfd != "" {
-		out += " (via " + e.procfd + ")"
+	out += "dst=" + e.target
+	if e.dstFD != "" {
+		out += ", dstFD=" + e.dstFD
 	}
 
 	if e.flags != uintptr(0) {
-		out += ", flags: 0x" + strconv.FormatUint(uint64(e.flags), 16)
+		out += ", flags=0x" + strconv.FormatUint(uint64(e.flags), 16)
 	}
 	if e.data != "" {
-		out += ", data: " + e.data
+		out += ", data=" + e.data
 	}
 
 	out += ": " + e.err.Error()
@@ -47,19 +50,36 @@ func (e *mountError) Unwrap() error {
 	return e.err
 }
 
-// mount is a simple unix.Mount wrapper. If procfd is not empty, it is used
-// instead of target (and the target is only used to add context to an error).
-func mount(source, target, procfd, fstype string, flags uintptr, data string) error {
+// mount is a simple unix.Mount wrapper, returning an error with more context
+// in case it failed.
+func mount(source, target, fstype string, flags uintptr, data string) error {
+	return mountViaFDs(source, "", target, "", fstype, flags, data)
+}
+
+// mountViaFDs is a unix.Mount wrapper which uses srcFD instead of source,
+// and dstFD instead of target, unless those are empty. The *FD arguments,
+// if non-empty, are expected to be in the form of a path to an opened file
+// descriptor on procfs (i.e. "/proc/self/fd/NN").
+//
+// If case an FD is used instead of a source or a target path, the
+// corresponding path is only used to add context to an error in case
+// the mount operation has failed.
+func mountViaFDs(source, srcFD, target, dstFD, fstype string, flags uintptr, data string) error {
+	src := source
+	if srcFD != "" {
+		src = srcFD
+	}
 	dst := target
-	if procfd != "" {
-		dst = procfd
+	if dstFD != "" {
+		dst = dstFD
 	}
-	if err := unix.Mount(source, dst, fstype, flags, data); err != nil {
+	if err := unix.Mount(src, dst, fstype, flags, data); err != nil {
 		return &mountError{
 			op:     "mount",
 			source: source,
+			srcFD:  srcFD,
 			target: target,
-			procfd: procfd,
+			dstFD:  dstFD,
 			flags:  flags,
 			data:   data,
 			err:    err,