From 98b093998971969884196f4a988b1c17fe91574b Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Thu, 27 May 2021 14:51:25 +0200 Subject: [PATCH 1/3] store: load additional image stores once it solves a problem where s.getGraphDriver() was accessed without holding a lock on s.graphLock as it is expected. Signed-off-by: Giuseppe Scrivano --- store.go | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/store.go b/store.go index 759407c63..26333ba8c 100644 --- a/store.go +++ b/store.go @@ -788,6 +788,15 @@ func (s *store) load() error { } s.containerStore = rcs + for _, store := range driver.AdditionalImageStores() { + gipath := filepath.Join(store, driverPrefix+"images") + ris, err := newROImageStore(gipath) + if err != nil { + return err + } + s.roImageStores = append(s.roImageStores, ris) + } + s.digestLockRoot = filepath.Join(s.runRoot, driverPrefix+"locks") if err := os.MkdirAll(s.digestLockRoot, 0700); err != nil { return err @@ -910,22 +919,10 @@ func (s *store) ImageStore() (ImageStore, error) { // Store. Accessing these stores directly will bypass locking and // synchronization, so it is not a part of the exported Store interface. func (s *store) ROImageStores() ([]ROImageStore, error) { - if len(s.roImageStores) != 0 { - return s.roImageStores, nil - } - driver, err := s.getGraphDriver() - if err != nil { - return nil, err - } - driverPrefix := s.graphDriverName + "-" - for _, store := range driver.AdditionalImageStores() { - gipath := filepath.Join(store, driverPrefix+"images") - ris, err := newROImageStore(gipath) - if err != nil { - return nil, err - } - s.roImageStores = append(s.roImageStores, ris) + if s.imageStore == nil { + return nil, ErrLoadError } + return s.roImageStores, nil } From abb54202ce664e8cb4cf89ee321d1916ed036a67 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Thu, 27 May 2021 14:55:42 +0200 Subject: [PATCH 2/3] store: ReloadIfChanged propagates errors from Modified() Signed-off-by: Giuseppe Scrivano --- containers.go | 2 +- images.go | 2 +- layers.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/containers.go b/containers.go index 6407e9a83..b4f773f2b 100644 --- a/containers.go +++ b/containers.go @@ -626,5 +626,5 @@ func (r *containerStore) ReloadIfChanged() error { if err == nil && modified { return r.Load() } - return nil + return err } diff --git a/images.go b/images.go index c7b968e05..bca25a65b 100644 --- a/images.go +++ b/images.go @@ -810,5 +810,5 @@ func (r *imageStore) ReloadIfChanged() error { if err == nil && modified { return r.Load() } - return nil + return err } diff --git a/layers.go b/layers.go index 1ed265d5d..949d24a1a 100644 --- a/layers.go +++ b/layers.go @@ -1777,7 +1777,7 @@ func (r *layerStore) ReloadIfChanged() error { if err == nil && modified { return r.Load() } - return nil + return err } func closeAll(closes ...func() error) (rErr error) { From 09295b3c6b5269aa05093083c0739ba1c5fbb8dd Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Thu, 27 May 2021 14:56:33 +0200 Subject: [PATCH 3/3] store: fix graphLock reload use s.graphLock.Modified() instead of s.graphLock.TouchedSince(). TouchedSince() seems to fail in some cases when comparing the inode mtime with the current time. Avoid such kind of problems in a critical code path as store.mount(), as we must be sure there is already a driver home mount present, otherwise it the next process may cover the container mount with the home mount. Closes: https://github.com/containers/podman/issues/10454 Signed-off-by: Giuseppe Scrivano --- store.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/store.go b/store.go index 26333ba8c..d6d547c64 100644 --- a/store.go +++ b/store.go @@ -2652,8 +2652,13 @@ func (s *store) mount(id string, options drivers.MountOpts) (string, error) { return "", err } + modified, err := s.graphLock.Modified() + if err != nil { + return "", err + } + /* We need to make sure the home mount is present when the Mount is done. */ - if s.graphLock.TouchedSince(s.lastLoaded) { + if modified { s.graphDriver = nil s.layerStore = nil s.graphDriver, err = s.getGraphDriver()