From 4a014e6b0d7e2d310ee1ba442128a6802559a63a Mon Sep 17 00:00:00 2001 From: Wentao Zhang Date: Wed, 24 May 2017 00:40:33 +0800 Subject: [PATCH] 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 --- image/tarexport/save.go | 87 +++++++++++++++++++++++++++++++---------- 1 file changed, 67 insertions(+), 20 deletions(-) diff --git a/image/tarexport/save.go b/image/tarexport/save.go index 8d33a58724..d304a54c34 100644 --- a/image/tarexport/save.go +++ b/image/tarexport/save.go @@ -21,8 +21,10 @@ import ( ) type imageDescriptor struct { - refs []reference.NamedTagged - layers []string + refs []reference.NamedTagged + layers []string + image *image.Image + layerRef layer.Layer } type saveSession struct { @@ -39,33 +41,47 @@ func (l *tarexporter) Save(names []string, outStream io.Writer) error { return err } + // Release all the image top layer references + defer l.releaseLayerReferences(images) return (&saveSession{tarexporter: l, images: images}).save(outStream) } -func (l *tarexporter) parseNames(names []string) (map[image.ID]*imageDescriptor, error) { +// parseNames will parse the image names to a map which contains image.ID to *imageDescriptor. +// Each imageDescriptor holds an image top layer reference named 'layerRef'. It is taken here, should be released later. +func (l *tarexporter) parseNames(names []string) (desc map[image.ID]*imageDescriptor, rErr error) { imgDescr := make(map[image.ID]*imageDescriptor) + defer func() { + if rErr != nil { + l.releaseLayerReferences(imgDescr) + } + }() - addAssoc := func(id image.ID, ref reference.Named) { + addAssoc := func(id image.ID, ref reference.Named) error { if _, ok := imgDescr[id]; !ok { - imgDescr[id] = &imageDescriptor{} + descr := &imageDescriptor{} + if err := l.takeLayerReference(id, descr); err != nil { + return err + } + imgDescr[id] = descr } if ref != nil { if _, ok := ref.(reference.Canonical); ok { - return + return nil } tagged, ok := reference.TagNameOnly(ref).(reference.NamedTagged) if !ok { - return + return nil } for _, t := range imgDescr[id].refs { if tagged.String() == t.String() { - return + return nil } } imgDescr[id].refs = append(imgDescr[id].refs, tagged) } + return nil } for _, name := range names { @@ -78,11 +94,9 @@ func (l *tarexporter) parseNames(names []string) (map[image.ID]*imageDescriptor, // Check if digest ID reference if digested, ok := ref.(reference.Digested); ok { id := image.IDFromDigest(digested.Digest()) - _, err := l.is.Get(id) - if err != nil { + if err := addAssoc(id, nil); err != nil { return nil, err } - addAssoc(id, nil) continue } return nil, errors.Errorf("invalid reference: %v", name) @@ -93,20 +107,26 @@ func (l *tarexporter) parseNames(names []string) (map[image.ID]*imageDescriptor, if err != nil { return nil, err } - addAssoc(imgID, nil) + if err := addAssoc(imgID, nil); err != nil { + return nil, err + } continue } if reference.IsNameOnly(namedRef) { assocs := l.rs.ReferencesByName(namedRef) for _, assoc := range assocs { - addAssoc(image.IDFromDigest(assoc.ID), assoc.Ref) + if err := addAssoc(image.IDFromDigest(assoc.ID), assoc.Ref); err != nil { + return nil, err + } } if len(assocs) == 0 { imgID, err := l.is.Search(name) if err != nil { return nil, err } - addAssoc(imgID, nil) + if err := addAssoc(imgID, nil); err != nil { + return nil, err + } } continue } @@ -114,12 +134,43 @@ func (l *tarexporter) parseNames(names []string) (map[image.ID]*imageDescriptor, if err != nil { return nil, err } - addAssoc(image.IDFromDigest(id), namedRef) + if err := addAssoc(image.IDFromDigest(id), namedRef); err != nil { + return nil, err + } } return imgDescr, nil } +// takeLayerReference will take/Get the image top layer reference +func (l *tarexporter) takeLayerReference(id image.ID, imgDescr *imageDescriptor) error { + img, err := l.is.Get(id) + if err != nil { + return err + } + imgDescr.image = img + topLayerID := img.RootFS.ChainID() + if topLayerID == "" { + return nil + } + layer, err := l.ls.Get(topLayerID) + if err != nil { + return err + } + imgDescr.layerRef = layer + return nil +} + +// releaseLayerReferences will release all the image top layer references +func (l *tarexporter) releaseLayerReferences(imgDescr map[image.ID]*imageDescriptor) error { + for _, descr := range imgDescr { + if descr.layerRef != nil { + l.ls.Release(descr.layerRef) + } + } + return nil +} + func (s *saveSession) save(outStream io.Writer) error { s.savedLayers = make(map[string]struct{}) s.diffIDPaths = make(map[layer.DiffID]string) @@ -224,11 +275,7 @@ func (s *saveSession) save(outStream io.Writer) error { } func (s *saveSession) saveImage(id image.ID) (map[layer.DiffID]distribution.Descriptor, error) { - img, err := s.is.Get(id) - if err != nil { - return nil, err - } - + img := s.images[id].image if len(img.RootFS.DiffIDs) == 0 { return nil, fmt.Errorf("empty export - not implemented") }