Merge pull request #438 from crosbymichael/remove-mnt-cmd

Remove shelling out to mount

mount.go

@@ -1,8 +1,6 @@
 package containerd
 
 import (
-	"os"
-	"os/exec"
 	"strings"
 	"syscall"
 )
@@ -12,48 +10,23 @@ import (
 type Mount struct {
 	// Type specifies the host-specific of the mount.
 	Type string
-
 	// Source specifies where to mount from. Depending on the host system, this
 	// can be a source path or device.
 	Source string
-
-	// Target is the filesystem mount location.
-	Target string
-
 	// Options contains zero or more fstab-style mount options. Typically,
 	// these are platform specific.
 	Options []string
 }
 
-// MountCommand converts the provided mount into a CLI arguments that can be used to mount the
+func (m *Mount) Mount(target string) error {
-func MountCommand(m Mount) []string {
+	flags, data := parseMountOptions(m.Options)
-	return []string{
+	return syscall.Mount(m.Source, target, m.Type, uintptr(flags), data)
-		"mount",
-		"-t", strings.ToLower(m.Type),
-		m.Source,
-		m.Target,
-		"-o", strings.Join(m.Options, ","),
-	}
 }
 
-func MountAll(mounts ...Mount) error {
+// MountAll mounts all the provided mounts to the provided target
-	for _, mount := range mounts {
+func MountAll(mounts []Mount, target string) error {
-		cmd := exec.Command("mount", MountCommand(mount)[1:]...)
-		cmd.Stderr = os.Stderr
-		cmd.Stdout = os.Stdout
-
-		if err := cmd.Run(); err != nil {
-			return err
-		}
-	}
-
-	return nil
-}
-
-func MountFS(mounts []Mount, target string) error {
 	for _, m := range mounts {
-		flags, data := parseMountOptions(m.Options)
+		if err := m.Mount(target); err != nil {
-		if err := syscall.Mount(m.Source, target, m.Type, uintptr(flags), data); err != nil {
 			return err
 		}
 	}

snapshot/btrfs/btrfs.go

@@ -50,8 +50,6 @@ func (lm *Btrfs) Prepare(key, parent string) ([]containerd.Mount, error) {
 		{
 			Type:   "btrfs",
 			Source: lm.device, // device?
-			Target: key,
-
 			// NOTE(stevvooe): While it would be nice to use to uuids for
 			// mounts, they don't work reliably if the uuids are missing.
 			Options: []string{fmt.Sprintf("subvolid=%d", info.ID)},

snapshot/btrfs/btrfs_test.go

@@ -27,11 +27,12 @@ func TestBtrfs(t *testing.T) {
 	}
 	defer os.RemoveAll(root)
 
+	target := filepath.Join(root, "test")
 	sm, err := NewBtrfs(device.deviceName, root)
 	if err != nil {
 		t.Fatal(err)
 	}
-	mounts, err := sm.Prepare(filepath.Join(root, "test"), "")
+	mounts, err := sm.Prepare(target, "")
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -48,16 +49,16 @@ func TestBtrfs(t *testing.T) {
 		}
 	}
 
-	if err := os.MkdirAll(mounts[0].Target, 0755); err != nil {
+	if err := os.MkdirAll(target, 0755); err != nil {
 		t.Fatal(err)
 	}
-	if err := containerd.MountAll(mounts...); err != nil {
+	if err := containerd.MountAll(mounts, target); err != nil {
 		t.Fatal(err)
 	}
-	defer testutil.UnmountAll(t, mounts)
+	defer testutil.Unmount(t, target)
 
 	// write in some data
-	if err := ioutil.WriteFile(filepath.Join(mounts[0].Target, "foo"), []byte("content"), 0777); err != nil {
+	if err := ioutil.WriteFile(filepath.Join(target, "foo"), []byte("content"), 0777); err != nil {
 		t.Fatal(err)
 	}
 
@@ -78,26 +79,27 @@ func TestBtrfs(t *testing.T) {
 		}
 	}()
 
-	mounts, err = sm.Prepare(filepath.Join(root, "test2"), filepath.Join(root, "snapshots/committed"))
+	target = filepath.Join(root, "test2")
+	mounts, err = sm.Prepare(target, filepath.Join(root, "snapshots/committed"))
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	if err := os.MkdirAll(filepath.Join(root, "test2"), 0755); err != nil {
+	if err := os.MkdirAll(target, 0755); err != nil {
 		t.Fatal(err)
 	}
 
-	if err := containerd.MountAll(mounts...); err != nil {
+	if err := containerd.MountAll(mounts, target); err != nil {
 		t.Fatal(err)
 	}
-	defer testutil.UnmountAll(t, mounts)
+	defer testutil.Unmount(t, target)
 
 	// TODO(stevvooe): Verify contents of "foo"
-	if err := ioutil.WriteFile(filepath.Join(mounts[0].Target, "bar"), []byte("content"), 0777); err != nil {
+	if err := ioutil.WriteFile(filepath.Join(target, "bar"), []byte("content"), 0777); err != nil {
 		t.Fatal(err)
 	}
 
-	if err := sm.Commit(filepath.Join(root, "snapshots/committed2"), filepath.Join(root, "test2")); err != nil {
+	if err := sm.Commit(filepath.Join(root, "snapshots/committed2"), target); err != nil {
 		t.Fatal(err)
 	}
 	defer func() {

snapshot/manager.go

@@ -197,7 +197,6 @@ func (lm *Manager) Prepare(dst, parent string) ([]containerd.Mount, error) {
 		{
 			Type:    "overlay",
 			Source:  "none",
-			Target:  dst,
 			Options: opts,
 		},
 	}, nil

snapshot/manager_test.go

@@ -4,7 +4,6 @@ import (
 	"io/ioutil"
 	"os"
 	"path/filepath"
-	"strings"
 	"testing"
 
 	"github.com/docker/containerd"
@@ -48,18 +47,10 @@ func TestSnapshotManagerBasic(t *testing.T) {
 		t.Fatal("expected mounts to have entries")
 	}
 
-	for _, mount := range mounts {
+	if err := containerd.MountAll(mounts, preparing); err != nil {
-		if !strings.HasPrefix(mount.Target, preparing) {
-			t.Fatalf("expected mount target to be prefixed with tmpDir: %q does not startwith %q", mount.Target, preparing)
-		}
-
-		t.Log(containerd.MountCommand(mount))
-	}
-
-	if err := containerd.MountAll(mounts...); err != nil {
 		t.Fatal(err)
 	}
-	defer testutil.UnmountAll(t, mounts)
+	defer testutil.Unmount(t, preparing)
 
 	if err := ioutil.WriteFile(filepath.Join(preparing, "foo"), []byte("foo\n"), 0777); err != nil {
 		t.Fatal(err)
@@ -86,18 +77,10 @@ func TestSnapshotManagerBasic(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if err := containerd.MountAll(mounts...); err != nil {
+	if err := containerd.MountAll(mounts, next); err != nil {
 		t.Fatal(err)
 	}
-	defer testutil.UnmountAll(t, mounts)
+	defer testutil.Unmount(t, next)
-
-	for _, mount := range mounts {
-		if !strings.HasPrefix(mount.Target, next) {
-			t.Fatalf("expected mount target to be prefixed with tmpDir: %q does not startwith %q", mount.Target, next)
-		}
-
-		t.Log(containerd.MountCommand(mount))
-	}
 
 	if err := ioutil.WriteFile(filepath.Join(next, "bar"), []byte("bar\n"), 0777); err != nil {
 		t.Fatal(err)

snapshot/naive/naive_test.go

@@ -4,7 +4,6 @@ import (
 	"io/ioutil"
 	"os"
 	"path/filepath"
-	"strings"
 	"testing"
 
 	"github.com/docker/containerd"
@@ -35,15 +34,7 @@ func TestSnapshotNaiveBasic(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	for _, mount := range mounts {
+	if err := containerd.MountAll(mounts, preparing); err != nil {
-		if !strings.HasPrefix(mount.Target, preparing) {
-			t.Fatalf("expected mount target to be prefixed with tmpDir: %q does not startwith %q", mount.Target, preparing)
-		}
-
-		t.Log(containerd.MountCommand(mount))
-	}
-
-	if err := containerd.MountAll(mounts...); err != nil {
 		t.Fatal(err)
 	}
 
@@ -74,18 +65,10 @@ func TestSnapshotNaiveBasic(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if err := containerd.MountAll(mounts...); err != nil {
+	if err := containerd.MountAll(mounts, next); err != nil {
 		t.Fatal(err)
 	}
 
-	for _, mount := range mounts {
-		if !strings.HasPrefix(mount.Target, next) {
-			t.Fatalf("expected mount target to be prefixed with tmpDir: %q does not startwith %q", mount.Target, next)
-		}
-
-		t.Log(containerd.MountCommand(mount))
-	}
-
 	if err := ioutil.WriteFile(filepath.Join(next, "bar"), []byte("bar\n"), 0777); err != nil {
 		t.Fatal(err)
 	}

snapshot/overlay/overlayfs_test.go

@@ -163,7 +163,7 @@ func TestOverlayfsOverlayRead(t *testing.T) {
 		t.Error(err)
 		return
 	}
-	if err := containerd.MountFS(mounts, dest); err != nil {
+	if err := containerd.MountAll(mounts, dest); err != nil {
 		t.Error(err)
 		return
 	}

snapshot/testutil/helpers.go

@@ -1,24 +1,13 @@
 package testutil
 
 import (
-	"os/exec"
+	"syscall"
 	"testing"
-
-	"github.com/docker/containerd"
 )
 
-func UnmountAll(t *testing.T, mounts []containerd.Mount) {
-	for _, mount := range mounts {
-		Unmount(t, mount.Target)
-	}
-}
-
 func Unmount(t *testing.T, mountPoint string) {
 	t.Log("unmount", mountPoint)
-	umount := exec.Command("umount", mountPoint)
+	if err := syscall.Unmount(mountPoint, 0); err != nil {
-	err := umount.Run()
-	if err != nil {
-
 		t.Error("Could not umount", mountPoint, err)
 	}
 }