From 01f508786623f0fb49d50b42f3e3975155d24b80 Mon Sep 17 00:00:00 2001 From: Derek McGowan Date: Mon, 27 Jul 2026 08:53:16 -0700 Subject: [PATCH] snapshots/erofs: keep lowers stacked above a merged fsmeta mounts() moved `first` to the merged fsmeta mount's index whenever mountFsMeta matched, collapsing the overlay's lowerdir range to the fsmeta alone and dropping any plain lowers already appended above it. When fsmerge was added, fsmeta was only ever written for the top parent of the chain being prepared, so the loop matched at i == 0, where first already equalled the fsmeta's index and the reassignment was a no-op. It only mattered once the fsmeta could sit below the top parent, i.e. a chain extending an already-merged sub-chain. The snapshotter no longer generates fsmeta itself, so today this only affects externally supplied fsmeta files. first only marks the start of the lowerdir range, so it must stay at the first lower's index. Signed-off-by: Derek McGowan --- plugins/snapshots/erofs/erofs.go | 3 +- plugins/snapshots/erofs/erofs_linux_test.go | 82 +++++++++++++++++++++ 2 files changed, 84 insertions(+), 1 deletion(-) diff --git a/plugins/snapshots/erofs/erofs.go b/plugins/snapshots/erofs/erofs.go index 4af0f034e7a3e..9224f8b5c21b0 100644 --- a/plugins/snapshots/erofs/erofs.go +++ b/plugins/snapshots/erofs/erofs.go @@ -521,13 +521,14 @@ func (s *snapshotter) mounts(snap storage.Snapshot, info snapshots.Info) ([]moun return []mount.Mount{m}, nil } + // first marks the start of the lowerdir range. A merged fsmeta ends the + // range but never moves its start: lowers stacked above it stay in range. first := len(mounts) for i := range snap.ParentIDs { // If a merged fsmeta is valid for this layer, skip the remaining bottom layers. // Why? Because bottom layers have been flattened with the thin fsmeta. if m, ok := s.mountFsMeta(snap, i); ok { mounts = append(mounts, m) - first = len(mounts) - 1 break } diff --git a/plugins/snapshots/erofs/erofs_linux_test.go b/plugins/snapshots/erofs/erofs_linux_test.go index 6b6ff9b13e279..c8edcaf0fa10e 100644 --- a/plugins/snapshots/erofs/erofs_linux_test.go +++ b/plugins/snapshots/erofs/erofs_linux_test.go @@ -820,6 +820,88 @@ func TestMountFsMeta(t *testing.T) { }) } +// TestMountsWithMergedFsMeta covers s.mounts()'s assembly of the overlay +// lowerdir when a merged fsmeta is present on a parent below the top of the +// chain, i.e. one or more plain (non-merged) layers are stacked on top of a +// merged fsmeta lower. +func TestMountsWithMergedFsMeta(t *testing.T) { + root := t.TempDir() + s := &snapshotter{root: root} + + // Chain (top to bottom): p0, p1, p2, p3. A merged fsmeta is only + // present for p2, flattening the sub-chain [p2, p3]. p0 and p1 are + // plain layers stacked on top of that merged base. + parents := []string{"p0", "p1", "p2", "p3"} + for _, id := range parents { + require.NoError(t, os.MkdirAll(filepath.Join(root, "snapshots", id), 0755)) + require.NoError(t, os.WriteFile(s.layerBlobPath(id), []byte("layer"), 0644)) + } + require.NoError(t, os.WriteFile(s.fsMetaPath("p2"), []byte("merged"), 0644)) + + snap := storage.Snapshot{Kind: snapshots.KindView, ParentIDs: parents} + info := snapshots.Info{} + + mounts, err := s.mounts(snap, info) + require.NoError(t, err) + + // Expect: [erofs(p0), erofs(p1), erofs(fsmeta p2, device=p3,p2), overlay] + require.Len(t, mounts, 4) + + assert.Equal(t, "erofs", mounts[0].Type) + assert.Equal(t, s.layerBlobPath("p0"), mounts[0].Source) + + assert.Equal(t, "erofs", mounts[1].Type) + assert.Equal(t, s.layerBlobPath("p1"), mounts[1].Source) + + assert.Equal(t, "erofs", mounts[2].Type) + assert.Equal(t, s.fsMetaPath("p2"), mounts[2].Source) + assert.Equal(t, []string{ + "ro", "loop", + "device=" + s.layerBlobPath("p3"), + "device=" + s.layerBlobPath("p2"), + }, mounts[2].Options) + + // The overlay must span all three lowers (indices 0-2): the two plain + // top layers plus the merged fsmeta. + overlay := mounts[3] + assert.Equal(t, "format/mkdir/overlay", overlay.Type) + assert.Contains(t, overlay.Options, "lowerdir={{ overlay 0 2 }}") +} + +// TestMountsWithMergedFsMetaOnTopParent covers the case where the merged +// fsmeta is valid for the topmost parent, so it is the only lower. Since +// overlayfs rejects a lowerdir with no upperdir, this collapses to a plain +// bind mount instead. +func TestMountsWithMergedFsMetaOnTopParent(t *testing.T) { + root := t.TempDir() + s := &snapshotter{root: root} + + parents := []string{"p0", "p1"} + for _, id := range parents { + require.NoError(t, os.MkdirAll(filepath.Join(root, "snapshots", id), 0755)) + require.NoError(t, os.WriteFile(s.layerBlobPath(id), []byte("layer"), 0644)) + } + require.NoError(t, os.WriteFile(s.fsMetaPath("p0"), []byte("merged"), 0644)) + + snap := storage.Snapshot{Kind: snapshots.KindView, ParentIDs: parents} + info := snapshots.Info{} + + mounts, err := s.mounts(snap, info) + require.NoError(t, err) + + require.Len(t, mounts, 2) + assert.Equal(t, "erofs", mounts[0].Type) + assert.Equal(t, s.fsMetaPath("p0"), mounts[0].Source) + assert.Equal(t, []string{ + "ro", "loop", + "device=" + s.layerBlobPath("p1"), + "device=" + s.layerBlobPath("p0"), + }, mounts[0].Options) + + assert.Equal(t, "format/bind", mounts[1].Type) + assert.Equal(t, "{{ mount 0 }}", mounts[1].Source) +} + // --- layer content cache tests --- const (