Merge pull request #10 from crosbymichael/aufs-fixes

Aufs fixes and driver dir namespaceing

aufs/aufs.go

@@ -57,16 +57,15 @@ func Init(root string) (graphdriver.Driver, error) {
 	// Create the root aufs driver dir and return
 	// if it already exists
 	// If not populate the dir structure
-	aufsPath := path.Join(root, "aufs")
-	if err := os.MkdirAll(aufsPath, 0755); err != nil {
+	if err := os.MkdirAll(root, 0755); err != nil {
 		if os.IsExist(err) {
 			return &AufsDriver{root}, nil
 		}
 		return nil, err
 	}
 
 	for _, p := range paths {
-		if err := os.MkdirAll(path.Join(aufsPath, p), 0755); err != nil {
+		if err := os.MkdirAll(path.Join(root, p), 0755); err != nil {
 			return nil, err
 		}
 	}
@@ -93,7 +92,7 @@ func supportsAufs() error {
 }
 
 func (a *AufsDriver) rootPath() string {
-	return path.Join(a.root, "aufs")
+	return a.root
 }
 
 func (a *AufsDriver) String() string {
@@ -119,9 +118,13 @@ func (a *AufsDriver) Create(id, parent string) error {
 			return err
 		}
 
-		fmt.Fprintln(f, parent)
+		if _, err := fmt.Fprintln(f, parent); err != nil {
+			return err
+		}
 		for _, i := range ids {
-			fmt.Fprintln(f, i)
+			if _, err := fmt.Fprintln(f, i); err != nil {
+				return err
+			}
 		}
 	}
 	return nil
@@ -145,8 +148,7 @@ func (a *AufsDriver) createDirsFor(id string) error {
 // Unmount and remove the dir information
 func (a *AufsDriver) Remove(id string) error {
 	// Make sure the dir is umounted first
-	mntPoint := path.Join(a.rootPath(), "mnt", id)
-	if err := a.unmount(mntPoint); err != nil {
+	if err := a.unmount(id); err != nil {
 		return err
 	}
 	tmpDirs := []string{
@@ -213,7 +215,28 @@ func (a *AufsDriver) DiffSize(id string) (int64, error) {
 }
 
 func (a *AufsDriver) Changes(id string) ([]archive.Change, error) {
-	return nil, nil
+	layers, err := a.getParentLayerPaths(id)
+	if err != nil {
+		return nil, err
+	}
+	return archive.Changes(layers, path.Join(a.rootPath(), "diff", id))
+}
+
+func (a *AufsDriver) getParentLayerPaths(id string) ([]string, error) {
+	parentIds, err := getParentIds(a.rootPath(), id)
+	if err != nil {
+		return nil, err
+	}
+	if len(parentIds) == 0 {
+		return nil, fmt.Errorf("Dir %s does not have any parent layers", id)
+	}
+	layers := make([]string, len(parentIds))
+
+	// Get the diff paths for all the parent ids
+	for i, p := range parentIds {
+		layers[i] = path.Join(a.rootPath(), "diff", p)
+	}
+	return layers, nil
 }
 
 func (a *AufsDriver) mount(id string) error {
@@ -222,22 +245,14 @@ func (a *AufsDriver) mount(id string) error {
 		return err
 	}
 
-	parentIds, err := getParentIds(a.rootPath(), id)
-	if err != nil {
-		return err
-	}
-	if len(parentIds) == 0 {
-		return fmt.Errorf("Dir %s does not have any parent layers", id)
-	}
 	var (
 		target = path.Join(a.rootPath(), "mnt", id)
 		rw     = path.Join(a.rootPath(), "diff", id)
-		layers = make([]string, len(parentIds))
 	)
 
-	// Get the diff paths for all the parent ids
-	for i, p := range parentIds {
-		layers[i] = path.Join(a.rootPath(), "diff", p)
+	layers, err := a.getParentLayerPaths(id)
+	if err != nil {
+		return err
 	}
 
 	if err := a.aufsMount(layers, rw, target); err != nil {

aufs/aufs_test.go

@@ -1,13 +1,14 @@
 package aufs
 
 import (
+	"github.com/dotcloud/docker/archive"
 	"os"
 	"path"
 	"testing"
 )
 
 var (
-	tmp = path.Join(os.TempDir(), "aufs-tests")
+	tmp = path.Join(os.TempDir(), "aufs-tests", "aufs")
 )
 
 func newDriver(t *testing.T) *AufsDriver {
@@ -57,7 +58,7 @@ func TestCreateDirStructure(t *testing.T) {
 	}
 
 	for _, p := range paths {
-		if _, err := os.Stat(path.Join(tmp, "aufs", p)); err != nil {
+		if _, err := os.Stat(path.Join(tmp, p)); err != nil {
 			t.Fatal(err)
 		}
 	}
@@ -102,7 +103,7 @@ func TestCreateNewDirStructure(t *testing.T) {
 	}
 
 	for _, p := range paths {
-		if _, err := os.Stat(path.Join(tmp, "aufs", p, "1")); err != nil {
+		if _, err := os.Stat(path.Join(tmp, p, "1")); err != nil {
 			t.Fatal(err)
 		}
 	}
@@ -127,7 +128,7 @@ func TestRemoveImage(t *testing.T) {
 	}
 
 	for _, p := range paths {
-		if _, err := os.Stat(path.Join(tmp, "aufs", p, "1")); err == nil {
+		if _, err := os.Stat(path.Join(tmp, p, "1")); err == nil {
 			t.Fatalf("Error should not be nil because dirs with id 1 should be delted: %s", p)
 		}
 	}
@@ -145,7 +146,7 @@ func TestGetWithoutParent(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	expected := path.Join(tmp, "aufs", "diff", "1")
+	expected := path.Join(tmp, "diff", "1")
 	if diffPath != expected {
 		t.Fatalf("Expected path %s got %s", expected, diffPath)
 	}
@@ -229,6 +230,12 @@ func TestMountWithParent(t *testing.T) {
 		t.Fatal(err)
 	}
 
+	defer func() {
+		if err := d.Cleanup(); err != nil {
+			t.Fatal(err)
+		}
+	}()
+
 	mntPath, err := d.Get("2")
 	if err != nil {
 		t.Fatal(err)
@@ -237,12 +244,47 @@ func TestMountWithParent(t *testing.T) {
 		t.Fatal("mntPath should not be empty string")
 	}
 
-	expected := path.Join(tmp, "aufs", "mnt", "2")
+	expected := path.Join(tmp, "mnt", "2")
 	if mntPath != expected {
 		t.Fatalf("Expected %s got %s", expected, mntPath)
 	}
+}
 
-	if err := d.Cleanup(); err != nil {
+func TestRemoveMountedDir(t *testing.T) {
+	d := newDriver(t)
+	defer os.RemoveAll(tmp)
+
+	if err := d.Create("1", ""); err != nil {
+		t.Fatal(err)
+	}
+	if err := d.Create("2", "1"); err != nil {
+		t.Fatal(err)
+	}
+
+	defer func() {
+		if err := d.Cleanup(); err != nil {
+			t.Fatal(err)
+		}
+	}()
+
+	mntPath, err := d.Get("2")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if mntPath == "" {
+		t.Fatal("mntPath should not be empty string")
+	}
+
+	mounted, err := d.mounted("2")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if !mounted {
+		t.Fatalf("Dir id 2 should be mounted")
+	}
+
+	if err := d.Remove("2"); err != nil {
 		t.Fatal(err)
 	}
 }
@@ -290,6 +332,60 @@ func TestGetDiff(t *testing.T) {
 	}
 }
 
+func TestChanges(t *testing.T) {
+	d := newDriver(t)
+	defer os.RemoveAll(tmp)
+
+	if err := d.Create("1", ""); err != nil {
+		t.Fatal(err)
+	}
+	if err := d.Create("2", "1"); err != nil {
+		t.Fatal(err)
+	}
+
+	defer func() {
+		if err := d.Cleanup(); err != nil {
+			t.Fatal(err)
+		}
+	}()
+
+	mntPoint, err := d.Get("2")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Create a file to save in the mountpoint
+	f, err := os.Create(path.Join(mntPoint, "test.txt"))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if _, err := f.WriteString("testline"); err != nil {
+		t.Fatal(err)
+	}
+	if err := f.Close(); err != nil {
+		t.Fatal(err)
+	}
+
+	changes, err := d.Changes("2")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(changes) != 1 {
+		t.Fatalf("Dir 2 should have one change from parent got %d", len(changes))
+	}
+	change := changes[0]
+
+	expectedPath := "/test.txt"
+	if change.Path != expectedPath {
+		t.Fatalf("Expected path %s got %s", expectedPath, change.Path)
+	}
+
+	if change.Kind != archive.ChangeAdd {
+		t.Fatalf("Change kind should be ChangeAdd got %s", change.Kind)
+	}
+}
+
 /* FIXME: How to properly test this?
 func TestDiffSize(t *testing.T) {
 	d := newDriver(t)

aufs/dirs.go

@@ -38,10 +38,9 @@ func getParentIds(root, id string) ([]string, error) {
 	s := bufio.NewScanner(f)
 
 	for s.Scan() {
-		if err := s.Err(); err != nil {
-			return nil, err
+		if t := s.Text(); t != "" {
+			out = append(out, s.Text())
 		}
-		out = append(out, s.Text())
 	}
-	return out, nil
+	return out, s.Err()
 }

aufs/mount.go

@@ -1,13 +1,11 @@
 package aufs
 
 import (
-	"fmt"
 	"github.com/dotcloud/docker/utils"
 	"os"
 	"os/exec"
 	"path/filepath"
 	"syscall"
-	"time"
 )
 
 func Unmount(target string) error {
@@ -17,22 +15,7 @@ func Unmount(target string) error {
 	if err := syscall.Unmount(target, 0); err != nil {
 		return err
 	}
-	// Even though we just unmounted the filesystem, AUFS will prevent deleting the mntpoint
-	// for some time. We'll just keep retrying until it succeeds.
-	for retries := 0; retries < 1000; retries++ {
-		err := os.Remove(target)
-		if err == nil {
-			// rm mntpoint succeeded
-			return nil
-		}
-		if os.IsNotExist(err) {
-			// mntpoint doesn't exist anymore. Success.
-			return nil
-		}
-		// fmt.Printf("(%v) Remove %v returned: %v\n", retries, target, err)
-		time.Sleep(10 * time.Millisecond)
-	}
-	return fmt.Errorf("Umount: Failed to umount %v", target)
+	return nil
 }
 
 func Mounted(mountpoint string) (bool, error) {

graphdriver/driver.go

@@ -5,6 +5,7 @@ import (
 	"github.com/dotcloud/docker/archive"
 	"github.com/dotcloud/docker/utils"
 	"os"
+	"path"
 )
 
 type InitFunc func(root string) (Driver, error)
@@ -48,7 +49,7 @@ func Register(name string, initFunc InitFunc) error {
 
 func getDriver(name, home string) (Driver, error) {
 	if initFunc, exists := drivers[name]; exists {
-		return initFunc(home)
+		return initFunc(path.Join(home, name))
 	}
 	return nil, fmt.Errorf("No such driver: %s", name)
 }