Browse Source

Fix AUFS silent mount errors on many layers

Fixes #1171
Fixes #6465

Data passed to mount(2) is clipped to PAGE_SIZE if its bigger. Previous 
implementation checked if error was returned and then started to append layers 
one by one. But if the PAGE_SIZE clipping appeared in between the paths, in the 
permission sections or in xino definition the call would not error and 
remaining layers would just be skipped(or some other unknown situation).

This also optimizes system calls as it tries to mount as much as possible with 
the first mount.


Signed-off-by: Tõnis Tiigi <tonistiigi@gmail.com> (github: tonistiigi)
Tonis Tiigi 10 years ago
parent
commit
6d97339ca2
2 changed files with 62 additions and 31 deletions
  1. 33 28
      daemon/graphdriver/aufs/aufs.go
  2. 29 3
      daemon/graphdriver/aufs/aufs_test.go

+ 33 - 28
daemon/graphdriver/aufs/aufs.go

@@ -412,39 +412,44 @@ func (a *Driver) aufsMount(ro []string, rw, target, mountLabel string) (err erro
 		}
 	}()
 
-	if err = a.tryMount(ro, rw, target, mountLabel); err != nil {
-		if err = a.mountRw(rw, target, mountLabel); err != nil {
-			return
+	// Mount options are clipped to page size(4096 bytes). If there are more
+	// layers then these are remounted individually using append.
+
+	b := make([]byte, syscall.Getpagesize()-len(mountLabel)-50) // room for xino & mountLabel
+	bp := copy(b, fmt.Sprintf("br:%s=rw", rw))
+
+	firstMount := true
+	i := 0
+
+	for {
+		for ; i < len(ro); i++ {
+			layer := fmt.Sprintf(":%s=ro+wh", ro[i])
+
+			if firstMount {
+				if bp+len(layer) > len(b) {
+					break
+				}
+				bp += copy(b[bp:], layer)
+			} else {
+				data := label.FormatMountLabel(fmt.Sprintf("append%s", layer), mountLabel)
+				if err = mount("none", target, "aufs", MsRemount, data); err != nil {
+					return
+				}
+			}
 		}
 
-		for _, layer := range ro {
-			data := label.FormatMountLabel(fmt.Sprintf("append:%s=ro+wh", layer), mountLabel)
-			if err = mount("none", target, "aufs", MsRemount, data); err != nil {
+		if firstMount {
+			data := label.FormatMountLabel(fmt.Sprintf("%s,xino=/dev/shm/aufs.xino", string(b[:bp])), mountLabel)
+			if err = mount("none", target, "aufs", 0, data); err != nil {
 				return
 			}
+			firstMount = false
 		}
-	}
-	return
-}
-
-// Try to mount using the aufs fast path, if this fails then
-// append ro layers.
-func (a *Driver) tryMount(ro []string, rw, target, mountLabel string) (err error) {
-	var (
-		rwBranch   = fmt.Sprintf("%s=rw", rw)
-		roBranches = fmt.Sprintf("%s=ro+wh:", strings.Join(ro, "=ro+wh:"))
-		data       = label.FormatMountLabel(fmt.Sprintf("br:%v:%v,xino=/dev/shm/aufs.xino", rwBranch, roBranches), mountLabel)
-	)
-	return mount("none", target, "aufs", 0, data)
-}
-
-func (a *Driver) mountRw(rw, target, mountLabel string) error {
-	data := label.FormatMountLabel(fmt.Sprintf("br:%s,xino=/dev/shm/aufs.xino", rw), mountLabel)
-	return mount("none", target, "aufs", 0, data)
-}
 
-func rollbackMount(target string, err error) {
-	if err != nil {
-		Unmount(target)
+		if i == len(ro) {
+			break
+		}
 	}
+
+	return
 }

+ 29 - 3
daemon/graphdriver/aufs/aufs_test.go

@@ -635,9 +635,13 @@ func hash(c string) string {
 	return hex.EncodeToString(h.Sum(nil))
 }
 
-func TestMountMoreThan42Layers(t *testing.T) {
-	d := newDriver(t)
-	defer os.RemoveAll(tmp)
+func testMountMoreThan42Layers(t *testing.T, mountPath string) {
+	if err := os.MkdirAll(mountPath, 0755); err != nil {
+		t.Fatal(err)
+	}
+
+	d := testInit(mountPath, t).(*Driver)
+	defer os.RemoveAll(mountPath)
 	defer d.Cleanup()
 	var last string
 	var expected int
@@ -695,3 +699,25 @@ func TestMountMoreThan42Layers(t *testing.T) {
 		t.Fatalf("Expected %d got %d", expected, len(files))
 	}
 }
+
+func TestMountMoreThan42Layers(t *testing.T) {
+	testMountMoreThan42Layers(t, tmp)
+}
+
+func TestMountMoreThan42LayersMatchingPathLength(t *testing.T) {
+	tmp := "aufs-tests"
+	for {
+		// This finds a mount path so that when combined into aufs mount options
+		// 4096 byte boundary would be in between the paths or in permission
+		// section. For '/tmp' it will use '/tmp/aufs-tests00000000/aufs'
+		mountPath := path.Join(os.TempDir(), tmp, "aufs")
+		pathLength := 77 + len(mountPath)
+
+		if mod := 4095 % pathLength; mod == 0 || mod > pathLength-2 {
+			t.Logf("Using path: %s", mountPath)
+			testMountMoreThan42Layers(t, mountPath)
+			return
+		}
+		tmp += "0"
+	}
+}