From 7cac518a00309136d7887fa059b740f3ea7c893e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Fri, 27 Mar 2026 17:05:31 +0100 Subject: [PATCH 1/3] Introduce readPrimaryLayerStore MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We already have 3 users, and we will need another one. Should not change behavior. Signed-off-by: Miloslav Trmač --- storage/store.go | 97 +++++++++++++++++++++++++++++------------------- 1 file changed, 59 insertions(+), 38 deletions(-) diff --git a/storage/store.go b/storage/store.go index 4b66388fd2..1d5f62c6d9 100644 --- a/storage/store.go +++ b/storage/store.go @@ -1273,6 +1273,26 @@ func readAllLayerStores[T any](s *store, fn func(store roLayerStore) (T, bool, e return zeroRes, false, nil } +// readPrimaryLayerStore is a helper for working with store.getLayerStore(): +// It locks the store for reading, checks for updates, and calls fn() +// It returns the return value of fn, or its own error initializing the store. +// +// Most callers should call readAllLayerStores instead. +func readPrimaryLayerStore[T any](s *store, fn func(store rwLayerStore) (T, error)) (T, error) { + var zeroRes T // A zero value of T + + store, err := s.getLayerStore() + if err != nil { + return zeroRes, err + } + + if err := store.startReading(); err != nil { + return zeroRes, err + } + defer store.stopReading() + return fn(store) +} + // writeToLayerStore is a helper for working with store.getLayerStore(): // It locks the store for writing, checks for updates, and calls fn() // It returns the return value of fn, or its own error initializing the store. @@ -3085,16 +3105,9 @@ func (s *store) Mounted(id string) (int, error) { if layerID, err := s.ContainerLayerID(id); err == nil { id = layerID } - rlstore, err := s.getLayerStore() - if err != nil { - return 0, err - } - if err := rlstore.startReading(); err != nil { - return 0, err - } - defer rlstore.stopReading() - - return rlstore.Mounted(id) + return readPrimaryLayerStore(s, func(store rwLayerStore) (int, error) { + return store.Mounted(id) + }) } func (s *store) UnmountImage(id string, force bool) (bool, error) { @@ -3389,41 +3402,49 @@ func (s *store) LayerSize(id string) (int64, error) { } func (s *store) LayerParentOwners(id string) ([]int, []int, error) { - rlstore, err := s.getLayerStore() - if err != nil { - return nil, nil, err - } - if err := rlstore.startReading(); err != nil { + var parentUIDs, parentGIDs []int + if _, err := readPrimaryLayerStore(s, func(store rwLayerStore) (struct{}, error) { + if store.Exists(id) { + u, g, err := store.ParentOwners(id) + if err != nil { + return struct{}{}, err + } + parentUIDs = u + parentGIDs = g + return struct{}{}, nil + } + return struct{}{}, ErrLayerUnknown + }); err != nil { return nil, nil, err } - defer rlstore.stopReading() - if rlstore.Exists(id) { - return rlstore.ParentOwners(id) - } - return nil, nil, ErrLayerUnknown + return parentUIDs, parentGIDs, nil } func (s *store) ContainerParentOwners(id string) ([]int, []int, error) { - rlstore, err := s.getLayerStore() - if err != nil { - return nil, nil, err - } - if err := rlstore.startReading(); err != nil { - return nil, nil, err - } - defer rlstore.stopReading() - if err := s.containerStore.startReading(); err != nil { - return nil, nil, err - } - defer s.containerStore.stopReading() - container, err := s.containerStore.Get(id) - if err != nil { + var parentUIDs, parentGIDs []int + if _, err := readPrimaryLayerStore(s, func(store rwLayerStore) (struct{}, error) { + if err := s.containerStore.startReading(); err != nil { + return struct{}{}, err + } + defer s.containerStore.stopReading() + container, err := s.containerStore.Get(id) + if err != nil { + return struct{}{}, err + } + if store.Exists(container.LayerID) { + u, g, err := store.ParentOwners(container.LayerID) + if err != nil { + return struct{}{}, err + } + parentUIDs = u + parentGIDs = g + return struct{}{}, nil + } + return struct{}{}, ErrLayerUnknown + }); err != nil { return nil, nil, err } - if rlstore.Exists(container.LayerID) { - return rlstore.ParentOwners(container.LayerID) - } - return nil, nil, ErrLayerUnknown + return parentUIDs, parentGIDs, nil } func (s *store) Layers() ([]Layer, error) { From 4e5f9d59bd2ecad80367f8e0e9053940e87d2092 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Fri, 27 Mar 2026 17:12:08 +0100 Subject: [PATCH 2/3] Use readContainerStore in ContainerParentOwners MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Should not change behavior. Signed-off-by: Miloslav Trmač --- storage/store.go | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/storage/store.go b/storage/store.go index 1d5f62c6d9..5eeff65534 100644 --- a/storage/store.go +++ b/storage/store.go @@ -3423,24 +3423,23 @@ func (s *store) LayerParentOwners(id string) ([]int, []int, error) { func (s *store) ContainerParentOwners(id string) ([]int, []int, error) { var parentUIDs, parentGIDs []int if _, err := readPrimaryLayerStore(s, func(store rwLayerStore) (struct{}, error) { - if err := s.containerStore.startReading(); err != nil { - return struct{}{}, err - } - defer s.containerStore.stopReading() - container, err := s.containerStore.Get(id) - if err != nil { - return struct{}{}, err - } - if store.Exists(container.LayerID) { - u, g, err := store.ParentOwners(container.LayerID) + _, _, err := readContainerStore(s, func() (struct{}, bool, error) { + container, err := s.containerStore.Get(id) if err != nil { - return struct{}{}, err + return struct{}{}, true, err } - parentUIDs = u - parentGIDs = g - return struct{}{}, nil - } - return struct{}{}, ErrLayerUnknown + if store.Exists(container.LayerID) { + u, g, err := store.ParentOwners(container.LayerID) + if err != nil { + return struct{}{}, true, err + } + parentUIDs = u + parentGIDs = g + return struct{}{}, true, nil + } + return struct{}{}, true, ErrLayerUnknown + }) + return struct{}{}, err }); err != nil { return nil, nil, err } From 8311e6c7bba621e2e32526e9c8451eb1c44e78c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Fri, 27 Mar 2026 17:20:56 +0100 Subject: [PATCH 3/3] Fix locking for ErrLayerUnaccounted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - The graph driver contents are locked by layerStore's locking, not by the graph driver lock - Because we dropped the layer store lock, a layer might have legitimately been added in the meantime; don't treat is as unaccounted. Signed-off-by: Miloslav Trmač --- storage/check.go | 49 +++++++++++++++++++++++++++++++----------------- 1 file changed, 32 insertions(+), 17 deletions(-) diff --git a/storage/check.go b/storage/check.go index eb39311cde..c375c9ffcb 100644 --- a/storage/check.go +++ b/storage/check.go @@ -688,25 +688,40 @@ func (s *store) Check(options *CheckOptions) (CheckReport, error) { return CheckReport{}, err } - // If the driver can tell us about which layers it knows about, we should have previously - // examined all of them. Any that we didn't are probably just wasted space. - // Note: if the driver doesn't support enumerating layers, it returns ErrNotSupported. - if err := s.startUsingGraphDriver(); err != nil { - return CheckReport{}, err - } - defer s.stopUsingGraphDriver() - layerList, err := s.graphDriver.ListLayers() - if err != nil && !errors.Is(err, drivers.ErrNotSupported) { - return CheckReport{}, err - } - if !errors.Is(err, drivers.ErrNotSupported) { - for i, id := range layerList { - if _, known := referencedLayers[id]; !known { - err := fmt.Errorf("layer %s: %w", id, ErrLayerUnaccounted) - report.Layers[id] = append(report.Layers[id], err) + if _, err := readPrimaryLayerStore(s, func(store rwLayerStore) (struct{}, error) { + // If the driver can tell us about which layers it knows about, we should have + // corresponding metadata records. + // Any layers don’t are probably just wasted space. + // Note: if the driver doesn't support enumerating layers, it returns ErrNotSupported. + lowerLevelLayers, err := s.graphDriver.ListLayers() + if err != nil && !errors.Is(err, drivers.ErrNotSupported) { + return struct{}{}, err + } + if !errors.Is(err, drivers.ErrNotSupported) { + // Update the list of layers known to the layerStore, something + // might have been added recently. + currentHigherLevelLayers, err := store.Layers() + if err != nil { + return struct{}{}, err + } + for i := range currentHigherLevelLayers { + id := currentHigherLevelLayers[i].ID + if _, known := referencedLayers[id]; !known { + referencedLayers[id] = false + } + } + + for i, id := range lowerLevelLayers { + if _, known := referencedLayers[id]; !known { + err := fmt.Errorf("layer %s: %w", id, ErrLayerUnaccounted) + report.Layers[id] = append(report.Layers[id], err) + } + report.layerOrder[id] = i + 1 } - report.layerOrder[id] = i + 1 } + return struct{}{}, nil + }); err != nil { + return CheckReport{}, err } return report, nil