kotovalexarian-likes-github/moby--moby
1
0
Fork 0
mirror of https://github.com/moby/moby.git synced 2022-11-09 12:21:53 -05:00
Code Releases Activity

Fix error removing diff path

Browse source 
In d42dbdd3d4 the code was re-arranged to
better report errors, and ignore non-errors.
In doing so we removed a deferred remove of the AUFS diff path, but did
not replace it with a non-deferred one.

This fixes the issue and makes the code a bit more readable.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
This commit is contained in:
Brian Goff 2017-08-21 17:38:13 -04:00
parent 361a473121
commit 276b44608b
2 changed files with 75 additions and 21 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

69
daemon/graphdriver/aufs/aufs.go
View file

@ -142,6 +142,23 @@ func Init(root string, options []string, uidMaps, gidMaps []idtools.IDMap) (grap
}
}
for _, path := range []string{"mnt", "diff"} {
p := filepath.Join(root, path)
dirs, err := ioutil.ReadDir(p)
if err != nil {
logrus.WithError(err).WithField("dir", p).Error("error reading dir entries")
continue
}
for _, dir := range dirs {
if strings.HasSuffix(dir.Name(), "-removing") {
logrus.WithField("dir", dir.Name()).Debug("Cleaning up stale layer dir")
if err := system.EnsureRemoveAll(filepath.Join(p, dir.Name())); err != nil {
logrus.WithField("dir", dir.Name()).WithError(err).Error("Error removing stale layer dir")
}
}
}
}
a.naiveDiff = graphdriver.NewNaiveDiffDriver(a, uidMaps, gidMaps)
return a, nil
}
@ -317,26 +334,6 @@ func (a *Driver) Remove(id string) error {
retries++
logger.Warnf("unmount failed due to EBUSY: retry count: %d", retries)
time.Sleep(100 * time.Millisecond)
continue
}
// Atomically remove each directory in turn by first moving it out of the
// way (so that docker doesn't find it anymore) before doing removal of
// the whole tree.
tmpMntPath := path.Join(a.mntPath(), fmt.Sprintf("%s-removing", id))
if err := os.Rename(mountpoint, tmpMntPath); err != nil && !os.IsNotExist(err) {
if err == unix.EBUSY {
logger.WithField("dir", mountpoint).WithError(err).Warn("os.Rename err due to EBUSY")
}
return errors.Wrapf(err, "error preparing atomic delete of aufs mountpoint for id: %s", id)
}
if err := system.EnsureRemoveAll(tmpMntPath); err != nil {
return errors.Wrapf(err, "error removing aufs layer %s", id)
}
tmpDiffpath := path.Join(a.diffPath(), fmt.Sprintf("%s-removing", id))
if err := os.Rename(a.getDiffPath(id), tmpDiffpath); err != nil && !os.IsNotExist(err) {
return errors.Wrapf(err, "error preparing atomic delete of aufs diff dir for id: %s", id)
}
// Remove the layers file for the id
@ -344,12 +341,44 @@ func (a *Driver) Remove(id string) error {
return errors.Wrapf(err, "error removing layers dir for %s", id)
}
if err := atomicRemove(a.getDiffPath(id)); err != nil {
return errors.Wrapf(err, "could not remove diff path for id %s", id)
}
// Atomically remove each directory in turn by first moving it out of the
// way (so that docker doesn't find it anymore) before doing removal of
// the whole tree.
if err := atomicRemove(mountpoint); err != nil {
if errors.Cause(err) == unix.EBUSY {
logger.WithField("dir", mountpoint).WithError(err).Warn("error performing atomic remove due to EBUSY")
}
return errors.Wrapf(err, "could not remove mountpoint for id %s", id)
}
a.pathCacheLock.Lock()
delete(a.pathCache, id)
a.pathCacheLock.Unlock()
return nil
}
func atomicRemove(source string) error {
target := source + "-removing"
err := os.Rename(source, target)
switch {
case err == nil, os.IsNotExist(err):
case os.IsExist(err):
// Got error saying the target dir already exists, maybe the source doesn't exist due to a previous (failed) remove
if _, e := os.Stat(source); !os.IsNotExist(e) {
return errors.Wrapf(err, "target rename dir '%s' exists but should not, this needs to be manually cleaned up")
}
default:
return errors.Wrapf(err, "error preparing atomic delete")
}
return system.EnsureRemoveAll(target)
}
// Get returns the rootfs path for the id.
// This will mount the dir at its given path
func (a *Driver) Get(id, mountLabel string) (string, error) {

27
daemon/graphdriver/aufs/aufs_test.go
View file

@ -12,6 +12,8 @@ import (
"sync"
"testing"
"path/filepath"
"github.com/docker/docker/daemon/graphdriver"
"github.com/docker/docker/pkg/archive"
"github.com/docker/docker/pkg/reexec"
@ -147,7 +149,10 @@ func TestRemoveImage(t *testing.T) {
for _, p := range paths {
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)
t.Fatalf("Error should not be nil because dirs with id 1 should be deleted: %s", p)
}
if _, err := os.Stat(path.Join(tmp, p, "1-removing")); err == nil {
t.Fatalf("Error should not be nil because dirs with id 1-removing should be deleted: %s", p)
}
}
}
@ -800,3 +805,23 @@ func BenchmarkConcurrentAccess(b *testing.B) {
}
}
}
func TestInitStaleCleanup(t *testing.T) {
if err := os.MkdirAll(tmp, 0755); err != nil {
t.Fatal(err)
}
defer os.RemoveAll(tmp)
for _, d := range []string{"diff", "mnt"} {
if err := os.MkdirAll(filepath.Join(tmp, d, "123-removing"), 0755); err != nil {
t.Fatal(err)
}
}
testInit(tmp, t)
for _, d := range []string{"diff", "mnt"} {
if _, err := os.Stat(filepath.Join(tmp, d, "123-removing")); err == nil {
t.Fatal("cleanup failed")
}
}
}