Merge pull request #30696 from yummypeng/lock-RWLayer-while-committing

Fixes #29654: take reference to RWLayer while committing/exporting

container/state.go

@@ -357,6 +357,15 @@ func (s *State) ResetRemovalInProgress() {
 	s.Unlock()
 }
 
+// IsRemovalInProgress returns whether the RemovalInProgress flag is set.
+// Used by Container to check whether a container is being removed.
+func (s *State) IsRemovalInProgress() bool {
+	s.Lock()
+	res := s.RemovalInProgress
+	s.Unlock()
+	return res
+}
+
 // SetDead sets the container state to "dead"
 func (s *State) SetDead() {
 	s.Lock()
@@ -364,6 +373,14 @@ func (s *State) SetDead() {
 	s.Unlock()
 }
 
+// IsDead returns whether the Dead flag is set. Used by Container to check whether a container is dead.
+func (s *State) IsDead() bool {
+	s.Lock()
+	res := s.Dead
+	s.Unlock()
+	return res
+}
+
 // SetRemoved assumes this container is already in the "dead" state and
 // closes the internal waitRemove channel to unblock callers waiting for a
 // container to be removed.

daemon/commit.go

@@ -2,6 +2,7 @@ package daemon
 
 import (
 	"encoding/json"
+	"fmt"
 	"io"
 	"runtime"
 	"strings"
@@ -133,6 +134,16 @@ func (daemon *Daemon) Commit(name string, c *backend.ContainerCommitConfig) (str
 		return "", errors.Errorf("%+v does not support commit of a running container", runtime.GOOS)
 	}
 
+	if container.IsDead() {
+		err := fmt.Errorf("You cannot commit container %s which is Dead", container.ID)
+		return "", stateConflictError{err}
+	}
+
+	if container.IsRemovalInProgress() {
+		err := fmt.Errorf("You cannot commit container %s which is being removed", container.ID)
+		return "", stateConflictError{err}
+	}
+
 	if c.Pause && !container.IsPaused() {
 		daemon.containerPause(container)
 		defer daemon.containerUnpause(container)
@@ -234,19 +245,36 @@ func (daemon *Daemon) Commit(name string, c *backend.ContainerCommitConfig) (str
 	return id.String(), nil
 }
 
-func (daemon *Daemon) exportContainerRw(container *container.Container) (io.ReadCloser, error) {
-	if err := daemon.Mount(container); err != nil {
+func (daemon *Daemon) exportContainerRw(container *container.Container) (arch io.ReadCloser, err error) {
+	rwlayer, err := daemon.stores[container.Platform].layerStore.GetRWLayer(container.ID)
+	if err != nil {
+		return nil, err
+	}
+	defer func() {
+		if err != nil {
+			daemon.stores[container.Platform].layerStore.ReleaseRWLayer(rwlayer)
+		}
+	}()
+
+	// TODO: this mount call is not necessary as we assume that TarStream() should
+	// mount the layer if needed. But the Diff() function for windows requests that
+	// the layer should be mounted when calling it. So we reserve this mount call
+	// until windows driver can implement Diff() interface correctly.
+	_, err = rwlayer.Mount(container.GetMountLabel())
+	if err != nil {
 		return nil, err
 	}
 
-	archive, err := container.RWLayer.TarStream()
+	archive, err := rwlayer.TarStream()
 	if err != nil {
-		daemon.Unmount(container) // logging is already handled in the `Unmount` function
+		rwlayer.Unmount()
 		return nil, err
 	}
 	return ioutils.NewReadCloserWrapper(archive, func() error {
 			archive.Close()
-			return container.RWLayer.Unmount()
+			err = rwlayer.Unmount()
+			daemon.stores[container.Platform].layerStore.ReleaseRWLayer(rwlayer)
+			return err
 		}),
 		nil
 }

daemon/export.go

@@ -22,6 +22,16 @@ func (daemon *Daemon) ContainerExport(name string, out io.Writer) error {
 		return fmt.Errorf("the daemon on this platform does not support exporting Windows containers")
 	}
 
+	if container.IsDead() {
+		err := fmt.Errorf("You cannot export container %s which is Dead", container.ID)
+		return stateConflictError{err}
+	}
+
+	if container.IsRemovalInProgress() {
+		err := fmt.Errorf("You cannot export container %s which is being removed", container.ID)
+		return stateConflictError{err}
+	}
+
 	data, err := daemon.containerExport(container)
 	if err != nil {
 		return fmt.Errorf("Error exporting container %s: %v", name, err)
@@ -35,8 +45,19 @@ func (daemon *Daemon) ContainerExport(name string, out io.Writer) error {
 	return nil
 }
 
-func (daemon *Daemon) containerExport(container *container.Container) (io.ReadCloser, error) {
-	if err := daemon.Mount(container); err != nil {
+func (daemon *Daemon) containerExport(container *container.Container) (arch io.ReadCloser, err error) {
+	rwlayer, err := daemon.stores[container.Platform].layerStore.GetRWLayer(container.ID)
+	if err != nil {
+		return nil, err
+	}
+	defer func() {
+		if err != nil {
+			daemon.stores[container.Platform].layerStore.ReleaseRWLayer(rwlayer)
+		}
+	}()
+
+	_, err = rwlayer.Mount(container.GetMountLabel())
+	if err != nil {
 		return nil, err
 	}
 
@@ -46,12 +67,13 @@ func (daemon *Daemon) containerExport(container *container.Container) (io.ReadCl
 		GIDMaps:     daemon.idMappings.GIDs(),
 	})
 	if err != nil {
-		daemon.Unmount(container)
+		rwlayer.Unmount()
 		return nil, err
 	}
-	arch := ioutils.NewReadCloserWrapper(archive, func() error {
+	arch = ioutils.NewReadCloserWrapper(archive, func() error {
 		err := archive.Close()
-		daemon.Unmount(container)
+		rwlayer.Unmount()
+		daemon.stores[container.Platform].layerStore.ReleaseRWLayer(rwlayer)
 		return err
 	})
 	daemon.LogContainerEvent(container, "export")