From 52f31657cc03c0097e6fd0d5e1d2c10fb31d2b95 Mon Sep 17 00:00:00 2001 From: Michael Crosby Date: Fri, 8 Nov 2013 10:17:51 -0800 Subject: [PATCH 1/3] Do not remove mountpoint on umount --- aufs/aufs.go | 3 +-- aufs/aufs_test.go | 43 ++++++++++++++++++++++++++++++++++++++++++- aufs/mount.go | 19 +------------------ 3 files changed, 44 insertions(+), 21 deletions(-) diff --git a/aufs/aufs.go b/aufs/aufs.go index f4cb52dd9..932e415e0 100644 --- a/aufs/aufs.go +++ b/aufs/aufs.go @@ -145,8 +145,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{ diff --git a/aufs/aufs_test.go b/aufs/aufs_test.go index d3de559fb..0afaf9b78 100644 --- a/aufs/aufs_test.go +++ b/aufs/aufs_test.go @@ -229,6 +229,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) @@ -241,8 +247,43 @@ func TestMountWithParent(t *testing.T) { 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) } } diff --git a/aufs/mount.go b/aufs/mount.go index 4355d3b77..6f3476f99 100644 --- a/aufs/mount.go +++ b/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) { From ed1884461331d7c2d6561be30b09da9df6612d39 Mon Sep 17 00:00:00 2001 From: Michael Crosby Date: Fri, 8 Nov 2013 11:10:33 -0800 Subject: [PATCH 2/3] Implement changes for aufs driver --- aufs/aufs.go | 45 ++++++++++++++++++++++++++------------ aufs/aufs_test.go | 55 +++++++++++++++++++++++++++++++++++++++++++++++ aufs/dirs.go | 7 +++--- 3 files changed, 89 insertions(+), 18 deletions(-) diff --git a/aufs/aufs.go b/aufs/aufs.go index 932e415e0..7d4aea859 100644 --- a/aufs/aufs.go +++ b/aufs/aufs.go @@ -119,9 +119,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 @@ -212,7 +216,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 { @@ -221,22 +246,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 { diff --git a/aufs/aufs_test.go b/aufs/aufs_test.go index 0afaf9b78..7f49aec92 100644 --- a/aufs/aufs_test.go +++ b/aufs/aufs_test.go @@ -1,6 +1,7 @@ package aufs import ( + "github.com/dotcloud/docker/archive" "os" "path" "testing" @@ -331,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) diff --git a/aufs/dirs.go b/aufs/dirs.go index dc0007b90..fb9b81edd 100644 --- a/aufs/dirs.go +++ b/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() } From 08a276986c363c5b2e7435fb59935485c53aae59 Mon Sep 17 00:00:00 2001 From: Michael Crosby Date: Fri, 8 Nov 2013 11:36:58 -0800 Subject: [PATCH 3/3] Name sure drivers are confined into their own dir --- aufs/aufs.go | 7 +++---- aufs/aufs_test.go | 12 ++++++------ graphdriver/driver.go | 3 ++- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/aufs/aufs.go b/aufs/aufs.go index 7d4aea859..22fa487f3 100644 --- a/aufs/aufs.go +++ b/aufs/aufs.go @@ -57,8 +57,7 @@ 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 } @@ -66,7 +65,7 @@ func Init(root string) (graphdriver.Driver, error) { } 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 { diff --git a/aufs/aufs_test.go b/aufs/aufs_test.go index 7f49aec92..b71f17f00 100644 --- a/aufs/aufs_test.go +++ b/aufs/aufs_test.go @@ -8,7 +8,7 @@ import ( ) var ( - tmp = path.Join(os.TempDir(), "aufs-tests") + tmp = path.Join(os.TempDir(), "aufs-tests", "aufs") ) func newDriver(t *testing.T) *AufsDriver { @@ -58,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) } } @@ -103,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) } } @@ -128,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) } } @@ -146,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) } @@ -244,7 +244,7 @@ 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) } diff --git a/graphdriver/driver.go b/graphdriver/driver.go index 3a44f86a0..7149e2223 100644 --- a/graphdriver/driver.go +++ b/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) }