Browse code

Fixing releaseableLayer handling of layer streams and mounts.

releaseableLayer includes automatic handling for creating a read/write layer and mounting it on a call to Mount(), but then does not correspondingly unmount the layer before trying to delete it, which will fail for some graphdrivers. Commit on a releaseable layer also leaks the tarstream for the layer. To fix this, the stream close is deferred in Commit and releaseRWLayer now correctly handles unmounting the layer before trying to delete it. In addition, the changes include better error handling in Release() to make sure that errors are returned to the caller for failures on read/write layers instead of being ignored.# Please enter the commit message for your changes. Lines starting

Signed-off-by: Stefan Wernli <swernli@ntdev.microsoft.com>

Stefan Wernli authored on 2017/07/20 06:35:23
Showing 1 changed files
... ...
@@ -27,6 +27,7 @@ type releaseableLayer struct {
27 27
 
28 28
 func (rl *releaseableLayer) Mount() (string, error) {
29 29
 	var err error
30
+	var mountPath string
30 31
 	var chainID layer.ChainID
31 32
 	if rl.roLayer != nil {
32 33
 		chainID = rl.roLayer.ChainID()
... ...
@@ -38,7 +39,19 @@ func (rl *releaseableLayer) Mount() (string, error) {
38 38
 		return "", errors.Wrap(err, "failed to create rwlayer")
39 39
 	}
40 40
 
41
-	return rl.rwLayer.Mount("")
41
+	mountPath, err = rl.rwLayer.Mount("")
42
+	if err != nil {
43
+		// Clean up the layer if we fail to mount it here.
44
+		metadata, err := rl.layerStore.ReleaseRWLayer(rl.rwLayer)
45
+		layer.LogReleaseMetadata(metadata)
46
+		if err != nil {
47
+			logrus.Errorf("Failed to release RWLayer: %s", err)
48
+		}
49
+		rl.rwLayer = nil
50
+		return "", err
51
+	}
52
+
53
+	return mountPath, nil
42 54
 }
43 55
 
44 56
 func (rl *releaseableLayer) Commit(platform string) (builder.ReleaseableLayer, error) {
... ...
@@ -51,6 +64,7 @@ func (rl *releaseableLayer) Commit(platform string) (builder.ReleaseableLayer, e
51 51
 	if err != nil {
52 52
 		return nil, err
53 53
 	}
54
+	defer stream.Close()
54 55
 
55 56
 	newLayer, err := rl.layerStore.Register(stream, chainID, layer.Platform(platform))
56 57
 	if err != nil {
... ...
@@ -75,20 +89,32 @@ func (rl *releaseableLayer) Release() error {
75 75
 	if rl.released {
76 76
 		return nil
77 77
 	}
78
+	if err := rl.releaseRWLayer(); err != nil {
79
+		// Best effort attempt at releasing read-only layer before returning original error.
80
+		rl.releaseROLayer()
81
+		return err
82
+	}
83
+	if err := rl.releaseROLayer(); err != nil {
84
+		return err
85
+	}
78 86
 	rl.released = true
79
-	rl.releaseRWLayer()
80
-	return rl.releaseROLayer()
87
+	return nil
81 88
 }
82 89
 
83 90
 func (rl *releaseableLayer) releaseRWLayer() error {
84 91
 	if rl.rwLayer == nil {
85 92
 		return nil
86 93
 	}
94
+	if err := rl.rwLayer.Unmount(); err != nil {
95
+		logrus.Errorf("Failed to unmount RWLayer: %s", err)
96
+		return err
97
+	}
87 98
 	metadata, err := rl.layerStore.ReleaseRWLayer(rl.rwLayer)
88 99
 	layer.LogReleaseMetadata(metadata)
89 100
 	if err != nil {
90 101
 		logrus.Errorf("Failed to release RWLayer: %s", err)
91 102
 	}
103
+	rl.rwLayer = nil
92 104
 	return err
93 105
 }
94 106
 
... ...
@@ -98,6 +124,10 @@ func (rl *releaseableLayer) releaseROLayer() error {
98 98
 	}
99 99
 	metadata, err := rl.layerStore.Release(rl.roLayer)
100 100
 	layer.LogReleaseMetadata(metadata)
101
+	if err != nil {
102
+		logrus.Errorf("Failed to release ROLayer: %s", err)
103
+	}
104
+	rl.roLayer = nil
101 105
 	return err
102 106
 }
103 107