From a6fdc5d208b6726a33d30bd25ab115131a298b90 Mon Sep 17 00:00:00 2001 From: Michael Crosby Date: Tue, 17 Dec 2013 18:40:20 -0800 Subject: [PATCH] Fix unmount issues --- graphdriver/aufs/aufs.go | 2 +- graphdriver/aufs/mount.go | 20 -------- graphdriver/driver.go | 100 +++++++++++++++++++++++++++---------- graphdriver/driver_test.go | 74 +++++++++++++++++++++++++++ 4 files changed, 150 insertions(+), 46 deletions(-) create mode 100644 graphdriver/driver_test.go diff --git a/graphdriver/aufs/aufs.go b/graphdriver/aufs/aufs.go index c3caf13c13..6b4b9024a9 100644 --- a/graphdriver/aufs/aufs.go +++ b/graphdriver/aufs/aufs.go @@ -295,7 +295,7 @@ func (a *Driver) unmount(id string) error { func (a *Driver) mounted(id string) (bool, error) { target := path.Join(a.rootPath(), "mnt", id) - return Mounted(target) + return graphdriver.Mounted(target) } // During cleanup aufs needs to unmount all mountpoints diff --git a/graphdriver/aufs/mount.go b/graphdriver/aufs/mount.go index 6f3476f99c..1f1d98f809 100644 --- a/graphdriver/aufs/mount.go +++ b/graphdriver/aufs/mount.go @@ -2,9 +2,7 @@ package aufs import ( "github.com/dotcloud/docker/utils" - "os" "os/exec" - "path/filepath" "syscall" ) @@ -17,21 +15,3 @@ func Unmount(target string) error { } return nil } - -func Mounted(mountpoint string) (bool, error) { - mntpoint, err := os.Stat(mountpoint) - if err != nil { - if os.IsNotExist(err) { - return false, nil - } - return false, err - } - parent, err := os.Stat(filepath.Join(mountpoint, "..")) - if err != nil { - return false, err - } - mntpointSt := mntpoint.Sys().(*syscall.Stat_t) - parentSt := parent.Sys().(*syscall.Stat_t) - - return mntpointSt.Dev != parentSt.Dev, nil -} diff --git a/graphdriver/driver.go b/graphdriver/driver.go index 86160c9023..7ca303cf12 100644 --- a/graphdriver/driver.go +++ b/graphdriver/driver.go @@ -1,6 +1,7 @@ package graphdriver import ( + "bufio" "fmt" "github.com/dotcloud/docker/archive" "github.com/dotcloud/docker/utils" @@ -8,8 +9,11 @@ import ( "path" "strings" "syscall" + "time" ) +const mountinfoFormat = "%d %d %d:%d %s %s %s - %s %s %s" + type InitFunc func(root string) (Driver, error) type Driver interface { @@ -99,16 +103,24 @@ func New(root string) (driver Driver, err error) { } func (m *Mount) Mount(root string) error { - var ( - flag int - data []string - target = path.Join(root, m.Target) - ) - + target := path.Join(root, m.Target) if mounted, err := Mounted(target); err != nil || mounted { return err } + flag, data := parseOptions(m.Options) + if err := syscall.Mount(m.Device, target, m.Type, uintptr(flag), data); err != nil { + return err + } + return nil +} + +func parseOptions(options string) (int, string) { + var ( + flag int + data []string + ) + flags := map[string]struct { clear bool flag int @@ -140,7 +152,7 @@ func (m *Mount) Mount(root string) error { "nostrictatime": {true, syscall.MS_STRICTATIME}, } - for _, o := range strings.Split(m.Options, ",") { + for _, o := range strings.Split(options, ",") { // If the option does not exist in the flags table then it is a // data value for a specific fs type if f, exists := flags[o]; exists { @@ -153,35 +165,73 @@ func (m *Mount) Mount(root string) error { data = append(data, o) } } - - if err := syscall.Mount(m.Device, target, m.Type, uintptr(flag), strings.Join(data, ",")); err != nil { - panic(err) - } - return nil + return flag, strings.Join(data, ",") } -func (m *Mount) Unmount(root string) error { +func (m *Mount) Unmount(root string) (err error) { target := path.Join(root, m.Target) if mounted, err := Mounted(target); err != nil || !mounted { return err } - return syscall.Unmount(target, 0) + + // Simple retry logic for unmount + for i := 0; i < 10; i++ { + if err = syscall.Unmount(target, 0); err == nil { + return nil + } + utils.Debugf("[Unmount] %s", err) + time.Sleep(100 * time.Millisecond) + } + return } func Mounted(mountpoint string) (bool, error) { - mntpoint, err := os.Stat(mountpoint) - if err != nil { - if os.IsNotExist(err) { - return false, nil - } - return false, err - } - parent, err := os.Stat(path.Join(mountpoint, "..")) + entries, err := parseMountTable() if err != nil { return false, err } - mntpointSt := mntpoint.Sys().(*syscall.Stat_t) - parentSt := parent.Sys().(*syscall.Stat_t) - return mntpointSt.Dev != parentSt.Dev, nil + // Search the table for the mountpoint + for _, e := range entries { + if e.mountpoint == mountpoint { + return true, nil + } + } + return false, nil +} + +// Represents one line from /proc/self/mountinfo +type procEntry struct { + id, parent, major, minor int + source, mountpoint, fstype, device string + vfsopts, opts string +} + +// Parse /proc/self/mountinfo because comparing Dev and ino does not work from bind mounts +// +// 180 20 0:2851 / /var/lib/docker/aufs/mnt/a22632d4ed3cb2438246064408f9f07734cbd331f50c81f1ca3dcbd78541ce83 rw,relatime - aufs none rw,si=e9663ac1adbdb4f8 +func parseMountTable() ([]*procEntry, error) { + f, err := os.Open("/proc/self/mountinfo") + if err != nil { + return nil, err + } + defer f.Close() + + s := bufio.NewScanner(f) + out := []*procEntry{} + for s.Scan() { + if err := s.Err(); err != nil { + return nil, err + } + + p := &procEntry{} + if _, err := fmt.Sscanf(s.Text(), mountinfoFormat, + &p.id, &p.parent, &p.major, &p.minor, + &p.source, &p.mountpoint, &p.vfsopts, &p.fstype, + &p.device, &p.opts); err != nil { + return nil, err + } + out = append(out, p) + } + return out, nil } diff --git a/graphdriver/driver_test.go b/graphdriver/driver_test.go new file mode 100644 index 0000000000..c16ce70369 --- /dev/null +++ b/graphdriver/driver_test.go @@ -0,0 +1,74 @@ +package graphdriver + +import ( + "os" + "path" + "syscall" + "testing" +) + +func TestMountOptionsParsing(t *testing.T) { + options := "bind,ro,size=10k" + + flag, data := parseOptions(options) + + if data != "size=10k" { + t.Fatalf("Expected size=10 got %s", data) + } + + expectedFlag := syscall.MS_BIND | syscall.MS_RDONLY + + if flag != expectedFlag { + t.Fatalf("Expected %d got %d", expectedFlag, flag) + } +} + +func TestMounted(t *testing.T) { + tmp := path.Join(os.TempDir(), "graphdriver-tests") + if err := os.MkdirAll(tmp, 0777); err != nil { + t.Fatal(err) + } + defer os.RemoveAll(tmp) + + var ( + sourcePath = path.Join(tmp, "sourcefile.txt") + targetPath = path.Join(tmp, "targetfile.txt") + ) + + f, err := os.Create(sourcePath) + if err != nil { + t.Fatal(err) + } + f.WriteString("hello") + f.Close() + + f, err = os.Create(targetPath) + if err != nil { + t.Fatal(err) + } + f.Close() + + mount := &Mount{ + Device: sourcePath, + Target: targetPath, + Type: "none", + Options: "bind,ro", + } + + if err := mount.Mount("/"); err != nil { + t.Fatal(err) + } + defer func() { + if err := mount.Unmount("/"); err != nil { + t.Fatal(err) + } + }() + + mounted, err := Mounted(targetPath) + if err != nil { + t.Fatal(err) + } + if !mounted { + t.Fatalf("Expected %s to be mounted", targetPath) + } +}