Browse code

Fix delete a image while saving it, delete successfully but failed to save it

Issue Description:
* 1. Saving more than one images, `docker save -o a.tar aaa bbb`
* 2. Delete the last image which in saving progress. `docker rmi bbb`

Espected:
Saving images operation shouldn't be disturbed. But the real result is that failed to
save image and get an error as below:
`Error response from daemon: open
/var/lib/docker/image/devicemapper/imagedb/content/sha256/7c24e4d533a76e801662ad1b7e6e06bc1204f80110b5623e96ba2877c51479a1:
no such file or directory`

Analysis:
1. While saving more than one images, it will get all the image info from reference/imagestore,
and then using the `cached data` to save the images to a tar file.
2. But this process doesn't have a resource lock, if a deletion operation comes, the image will be deleted,
so saving operation will fail.

Solution:
When begin to save an image, `Get` all the layers first. then the
deletion operation won't delete the layers.

Signed-off-by: Wentao Zhang <zhangwentao234@huawei.com>

Wentao Zhang authored on 2017/05/24 01:40:33
Showing 1 changed files
... ...
@@ -21,8 +21,10 @@ import (
21 21
 )
22 22
 
23 23
 type imageDescriptor struct {
24
-	refs   []reference.NamedTagged
25
-	layers []string
24
+	refs     []reference.NamedTagged
25
+	layers   []string
26
+	image    *image.Image
27
+	layerRef layer.Layer
26 28
 }
27 29
 
28 30
 type saveSession struct {
... ...
@@ -39,33 +41,47 @@ func (l *tarexporter) Save(names []string, outStream io.Writer) error {
39 39
 		return err
40 40
 	}
41 41
 
42
+	// Release all the image top layer references
43
+	defer l.releaseLayerReferences(images)
42 44
 	return (&saveSession{tarexporter: l, images: images}).save(outStream)
43 45
 }
44 46
 
45
-func (l *tarexporter) parseNames(names []string) (map[image.ID]*imageDescriptor, error) {
47
+// parseNames will parse the image names to a map which contains image.ID to *imageDescriptor.
48
+// Each imageDescriptor holds an image top layer reference named 'layerRef'. It is taken here, should be released later.
49
+func (l *tarexporter) parseNames(names []string) (desc map[image.ID]*imageDescriptor, rErr error) {
46 50
 	imgDescr := make(map[image.ID]*imageDescriptor)
51
+	defer func() {
52
+		if rErr != nil {
53
+			l.releaseLayerReferences(imgDescr)
54
+		}
55
+	}()
47 56
 
48
-	addAssoc := func(id image.ID, ref reference.Named) {
57
+	addAssoc := func(id image.ID, ref reference.Named) error {
49 58
 		if _, ok := imgDescr[id]; !ok {
50
-			imgDescr[id] = &imageDescriptor{}
59
+			descr := &imageDescriptor{}
60
+			if err := l.takeLayerReference(id, descr); err != nil {
61
+				return err
62
+			}
63
+			imgDescr[id] = descr
51 64
 		}
52 65
 
53 66
 		if ref != nil {
54 67
 			if _, ok := ref.(reference.Canonical); ok {
55
-				return
68
+				return nil
56 69
 			}
57 70
 			tagged, ok := reference.TagNameOnly(ref).(reference.NamedTagged)
58 71
 			if !ok {
59
-				return
72
+				return nil
60 73
 			}
61 74
 
62 75
 			for _, t := range imgDescr[id].refs {
63 76
 				if tagged.String() == t.String() {
64
-					return
77
+					return nil
65 78
 				}
66 79
 			}
67 80
 			imgDescr[id].refs = append(imgDescr[id].refs, tagged)
68 81
 		}
82
+		return nil
69 83
 	}
70 84
 
71 85
 	for _, name := range names {
... ...
@@ -78,11 +94,9 @@ func (l *tarexporter) parseNames(names []string) (map[image.ID]*imageDescriptor,
78 78
 			// Check if digest ID reference
79 79
 			if digested, ok := ref.(reference.Digested); ok {
80 80
 				id := image.IDFromDigest(digested.Digest())
81
-				_, err := l.is.Get(id)
82
-				if err != nil {
81
+				if err := addAssoc(id, nil); err != nil {
83 82
 					return nil, err
84 83
 				}
85
-				addAssoc(id, nil)
86 84
 				continue
87 85
 			}
88 86
 			return nil, errors.Errorf("invalid reference: %v", name)
... ...
@@ -93,20 +107,26 @@ func (l *tarexporter) parseNames(names []string) (map[image.ID]*imageDescriptor,
93 93
 			if err != nil {
94 94
 				return nil, err
95 95
 			}
96
-			addAssoc(imgID, nil)
96
+			if err := addAssoc(imgID, nil); err != nil {
97
+				return nil, err
98
+			}
97 99
 			continue
98 100
 		}
99 101
 		if reference.IsNameOnly(namedRef) {
100 102
 			assocs := l.rs.ReferencesByName(namedRef)
101 103
 			for _, assoc := range assocs {
102
-				addAssoc(image.IDFromDigest(assoc.ID), assoc.Ref)
104
+				if err := addAssoc(image.IDFromDigest(assoc.ID), assoc.Ref); err != nil {
105
+					return nil, err
106
+				}
103 107
 			}
104 108
 			if len(assocs) == 0 {
105 109
 				imgID, err := l.is.Search(name)
106 110
 				if err != nil {
107 111
 					return nil, err
108 112
 				}
109
-				addAssoc(imgID, nil)
113
+				if err := addAssoc(imgID, nil); err != nil {
114
+					return nil, err
115
+				}
110 116
 			}
111 117
 			continue
112 118
 		}
... ...
@@ -114,12 +134,43 @@ func (l *tarexporter) parseNames(names []string) (map[image.ID]*imageDescriptor,
114 114
 		if err != nil {
115 115
 			return nil, err
116 116
 		}
117
-		addAssoc(image.IDFromDigest(id), namedRef)
117
+		if err := addAssoc(image.IDFromDigest(id), namedRef); err != nil {
118
+			return nil, err
119
+		}
118 120
 
119 121
 	}
120 122
 	return imgDescr, nil
121 123
 }
122 124
 
125
+// takeLayerReference will take/Get the image top layer reference
126
+func (l *tarexporter) takeLayerReference(id image.ID, imgDescr *imageDescriptor) error {
127
+	img, err := l.is.Get(id)
128
+	if err != nil {
129
+		return err
130
+	}
131
+	imgDescr.image = img
132
+	topLayerID := img.RootFS.ChainID()
133
+	if topLayerID == "" {
134
+		return nil
135
+	}
136
+	layer, err := l.ls.Get(topLayerID)
137
+	if err != nil {
138
+		return err
139
+	}
140
+	imgDescr.layerRef = layer
141
+	return nil
142
+}
143
+
144
+// releaseLayerReferences will release all the image top layer references
145
+func (l *tarexporter) releaseLayerReferences(imgDescr map[image.ID]*imageDescriptor) error {
146
+	for _, descr := range imgDescr {
147
+		if descr.layerRef != nil {
148
+			l.ls.Release(descr.layerRef)
149
+		}
150
+	}
151
+	return nil
152
+}
153
+
123 154
 func (s *saveSession) save(outStream io.Writer) error {
124 155
 	s.savedLayers = make(map[string]struct{})
125 156
 	s.diffIDPaths = make(map[layer.DiffID]string)
... ...
@@ -224,11 +275,7 @@ func (s *saveSession) save(outStream io.Writer) error {
224 224
 }
225 225
 
226 226
 func (s *saveSession) saveImage(id image.ID) (map[layer.DiffID]distribution.Descriptor, error) {
227
-	img, err := s.is.Get(id)
228
-	if err != nil {
229
-		return nil, err
230
-	}
231
-
227
+	img := s.images[id].image
232 228
 	if len(img.RootFS.DiffIDs) == 0 {
233 229
 		return nil, fmt.Errorf("empty export - not implemented")
234 230
 	}