Skip to content

Commit 171a0bc

Browse files
giuseppeclaude
andcommitted
storage, overlay: add lower-layers file
Write a new "lower-layers" file alongside the existing "lower" file. It stores layer IDs directly instead of l/ symlink references; the reading side appends "/diff" itself. When present, lower-layers is preferred over lower. The old lower file is still written so that older tools (buildah, skopeo) continue to work. Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
1 parent 8044c96 commit 171a0bc

File tree

2 files changed

+66
-184
lines changed

2 files changed

+66
-184
lines changed

storage/drivers/overlay/overlay.go

Lines changed: 66 additions & 148 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,13 @@ const (
8484
stagingDir = "staging"
8585
tempDirName = "tempdirs"
8686
lowerFile = "lower"
87-
maxDepth = 500
87+
// lowerLayersFile references lower layers directly by layer ID
88+
// instead of going through the l/ symlinks. The code appends
89+
// "/diff" itself when consuming entries. It is preferred over
90+
// lowerFile when present. The old lowerFile is still written
91+
// for backward compatibility with older tools.
92+
lowerLayersFile = "lower-layers"
93+
maxDepth = 500
8894

8995
stagingLockFile = "staging.lock"
9096

@@ -1153,6 +1159,35 @@ func (d *Driver) create(id, parent string, opts *graphdriver.CreateOpts, readOnl
11531159
}
11541160
}
11551161

1162+
// Write a lower-layers file referencing layers by ID instead of
1163+
// l/ symlink references. The reading side appends "/diff" itself.
1164+
parentDir := d.dir(parent)
1165+
layerLower := parent
1166+
parentLower, err := os.ReadFile(path.Join(parentDir, lowerLayersFile))
1167+
if err == nil {
1168+
layerLower += ":" + string(parentLower)
1169+
} else if !errors.Is(err, unix.ENOENT) {
1170+
return err
1171+
} else {
1172+
// Parent has no lower-layers file. Convert old-format lower
1173+
// entries (l/ symlinks) to layer IDs.
1174+
oldLower, err := os.ReadFile(path.Join(parentDir, lowerFile))
1175+
if err == nil {
1176+
for _, s := range strings.Split(string(oldLower), ":") {
1177+
target, err := os.Readlink(d.dir(s))
1178+
if err != nil {
1179+
return fmt.Errorf("reading symlink for lower %q: %w", s, err)
1180+
}
1181+
layerLower += ":" + filepath.Base(filepath.Dir(target))
1182+
}
1183+
} else if !errors.Is(err, unix.ENOENT) {
1184+
return err
1185+
}
1186+
}
1187+
if err := os.WriteFile(path.Join(dir, lowerLayersFile), []byte(layerLower), 0o666); err != nil {
1188+
return err
1189+
}
1190+
11561191
return nil
11571192
}
11581193

@@ -1190,20 +1225,9 @@ func (d *Driver) getLower(parent string) (string, error) {
11901225
return "", err
11911226
}
11921227

1193-
// Read Parent link fileA
11941228
parentLink, err := os.ReadFile(path.Join(parentDir, "link"))
11951229
if err != nil {
1196-
if !os.IsNotExist(err) {
1197-
return "", err
1198-
}
1199-
logrus.Warnf("Can't read parent link %q because it does not exist. Going through storage to recreate the missing links.", path.Join(parentDir, "link"))
1200-
if err := d.recreateSymlinks(); err != nil {
1201-
return "", fmt.Errorf("recreating the links: %w", err)
1202-
}
1203-
parentLink, err = os.ReadFile(path.Join(parentDir, "link"))
1204-
if err != nil {
1205-
return "", err
1206-
}
1230+
return "", err
12071231
}
12081232
lowers := []string{path.Join(linkDir, string(parentLink))}
12091233

@@ -1258,27 +1282,25 @@ func (d *Driver) dir2(id string, useImageStore bool) (string, string, bool) {
12581282

12591283
func (d *Driver) getLowerDirs(id string) ([]string, error) {
12601284
var lowersArray []string
1261-
lowers, err := os.ReadFile(path.Join(d.dir(id), lowerFile))
1285+
dir := d.dir(id)
1286+
lowers, err := os.ReadFile(path.Join(dir, lowerLayersFile))
1287+
if err != nil {
1288+
if !errors.Is(err, unix.ENOENT) {
1289+
return nil, err
1290+
}
1291+
lowers, err = os.ReadFile(path.Join(dir, lowerFile))
1292+
}
12621293
if err == nil {
12631294
for s := range strings.SplitSeq(string(lowers), ":") {
12641295
lower := d.dir(s)
12651296
lp, err := os.Readlink(lower)
1266-
// if the link does not exist, we lost the symlinks during a sudden reboot.
1267-
// Let's go ahead and recreate those symlinks.
12681297
if err != nil {
1269-
if os.IsNotExist(err) {
1270-
logrus.Warnf("Can't read link %q because it does not exist. A storage corruption might have occurred, attempting to recreate the missing symlinks. It might be best wipe the storage to avoid further errors due to storage corruption.", lower)
1271-
if err := d.recreateSymlinks(); err != nil {
1272-
return nil, fmt.Errorf("recreating the missing symlinks: %w", err)
1273-
}
1274-
// let's call Readlink on lower again now that we have recreated the missing symlinks
1275-
lp, err = os.Readlink(lower)
1276-
if err != nil {
1277-
return nil, err
1278-
}
1279-
} else {
1280-
return nil, err
1298+
if errors.Is(err, syscall.EINVAL) {
1299+
// Not a symlink: layer ID, append /diff.
1300+
lowersArray = append(lowersArray, path.Join(lower, "diff"))
1301+
continue
12811302
}
1303+
return nil, err
12821304
}
12831305
lowersArray = append(lowersArray, path.Clean(d.dir(path.Join("link", lp))))
12841306
}
@@ -1390,112 +1412,6 @@ func (d *Driver) DeferredRemove(id string) (tempdir.CleanupTempDirFunc, error) {
13901412
return t.Cleanup, nil
13911413
}
13921414

1393-
// recreateSymlinks goes through the driver's home directory and checks if the diff directory
1394-
// under each layer has a symlink created for it under the linkDir. If the symlink does not
1395-
// exist, it creates them
1396-
func (d *Driver) recreateSymlinks() error {
1397-
// We have at most 3 corrective actions per layer, so 10 iterations is plenty.
1398-
const maxIterations = 10
1399-
1400-
// List all the directories under the home directory
1401-
dirs, err := os.ReadDir(d.home)
1402-
if err != nil {
1403-
return fmt.Errorf("reading driver home directory %q: %w", d.home, err)
1404-
}
1405-
// This makes the link directory if it doesn't exist
1406-
if err := idtools.MkdirAllAs(path.Join(d.home, linkDir), 0o755, 0, 0); err != nil {
1407-
return err
1408-
}
1409-
// Keep looping as long as we take some corrective action in each iteration
1410-
var errs error
1411-
madeProgress := true
1412-
iterations := 0
1413-
for madeProgress {
1414-
errs = nil
1415-
madeProgress = false
1416-
// Check that for each layer, there's a link in "l" with the name in
1417-
// the layer's "link" file that points to the layer's "diff" directory.
1418-
for _, dir := range dirs {
1419-
// Skip over the linkDir, stagingDir, tempDirName and anything that is not a directory
1420-
if dir.Name() == linkDir || dir.Name() == stagingDir || dir.Name() == tempDirName || !dir.IsDir() {
1421-
continue
1422-
}
1423-
// Read the "link" file under each layer to get the name of the symlink
1424-
data, err := os.ReadFile(path.Join(d.dir(dir.Name()), "link"))
1425-
if err != nil {
1426-
errs = errors.Join(errs, fmt.Errorf("reading name of symlink for %q: %w", dir.Name(), err))
1427-
continue
1428-
}
1429-
linkPath := path.Join(d.home, linkDir, strings.Trim(string(data), "\n"))
1430-
// Check if the symlink exists, and if it doesn't, create it again with the
1431-
// name we got from the "link" file
1432-
err = fileutils.Lexists(linkPath)
1433-
if err != nil && os.IsNotExist(err) {
1434-
if err := os.Symlink(path.Join("..", dir.Name(), "diff"), linkPath); err != nil {
1435-
errs = errors.Join(errs, err)
1436-
continue
1437-
}
1438-
madeProgress = true
1439-
} else if err != nil {
1440-
errs = errors.Join(errs, err)
1441-
continue
1442-
}
1443-
}
1444-
1445-
// linkDirFullPath is the full path to the linkDir
1446-
linkDirFullPath := filepath.Join(d.home, "l")
1447-
// Now check if we somehow lost a "link" file, by making sure
1448-
// that each symlink we have corresponds to one.
1449-
links, err := os.ReadDir(linkDirFullPath)
1450-
if err != nil {
1451-
errs = errors.Join(errs, err)
1452-
continue
1453-
}
1454-
// Go through all of the symlinks in the "l" directory
1455-
for _, link := range links {
1456-
// Read the symlink's target, which should be "../$layer/diff"
1457-
target, err := os.Readlink(filepath.Join(linkDirFullPath, link.Name()))
1458-
if err != nil {
1459-
errs = errors.Join(errs, err)
1460-
continue
1461-
}
1462-
targetComponents := strings.Split(target, string(os.PathSeparator))
1463-
if len(targetComponents) != 3 || targetComponents[0] != ".." || targetComponents[2] != "diff" {
1464-
errs = errors.Join(errs, fmt.Errorf("link target of %q looks weird: %q", link, target))
1465-
// force the link to be recreated on the next pass
1466-
if err := os.Remove(filepath.Join(linkDirFullPath, link.Name())); err != nil {
1467-
if !os.IsNotExist(err) {
1468-
errs = errors.Join(errs, fmt.Errorf("removing link %q: %w", link, err))
1469-
} // else don’t report any error, but also don’t set madeProgress.
1470-
continue
1471-
}
1472-
madeProgress = true
1473-
continue
1474-
}
1475-
// Reconstruct the name of the target's link file and check that
1476-
// it has the basename of our symlink in it.
1477-
targetID := targetComponents[1]
1478-
linkFile := filepath.Join(d.dir(targetID), "link")
1479-
data, err := os.ReadFile(linkFile)
1480-
if err != nil || string(data) != link.Name() {
1481-
// NOTE: If two or more links point to the same target, we will update linkFile
1482-
// with every value of link.Name(), and set madeProgress = true every time.
1483-
if err := os.WriteFile(linkFile, []byte(link.Name()), 0o644); err != nil {
1484-
errs = errors.Join(errs, fmt.Errorf("correcting link for layer %s: %w", targetID, err))
1485-
continue
1486-
}
1487-
madeProgress = true
1488-
}
1489-
}
1490-
iterations++
1491-
if iterations >= maxIterations {
1492-
errs = errors.Join(errs, fmt.Errorf("reached %d iterations in overlay graph driver’s recreateSymlink, giving up", iterations))
1493-
break
1494-
}
1495-
}
1496-
return errs
1497-
}
1498-
14991415
// Get creates and mounts the required file system for the given id and returns the mount path.
15001416
func (d *Driver) Get(id string, options graphdriver.MountOpts) (string, error) {
15011417
return d.get(id, false, options)
@@ -1589,9 +1505,15 @@ func (d *Driver) get(id string, disableShifting bool, options graphdriver.MountO
15891505
readWrite = false
15901506
}
15911507

1592-
lowers, err := os.ReadFile(path.Join(dir, lowerFile))
1593-
if err != nil && !os.IsNotExist(err) {
1594-
return "", err
1508+
lowers, err := os.ReadFile(path.Join(dir, lowerLayersFile))
1509+
if err != nil {
1510+
if !errors.Is(err, unix.ENOENT) {
1511+
return "", err
1512+
}
1513+
lowers, err = os.ReadFile(path.Join(dir, lowerFile))
1514+
if err != nil && !os.IsNotExist(err) {
1515+
return "", err
1516+
}
15951517
}
15961518
splitLowers := strings.Split(string(lowers), ":")
15971519
if len(splitLowers) > maxDepth {
@@ -1703,16 +1625,7 @@ func (d *Driver) get(id string, disableShifting bool, options graphdriver.MountO
17031625
}
17041626
lower = ""
17051627
}
1706-
// if it is a "not found" error, that means the symlinks were lost in a sudden reboot
1707-
// so call the recreateSymlinks function to go through all the layer dirs and recreate
1708-
// the symlinks with the name from their respective "link" files
1709-
if lower == "" && os.IsNotExist(err) {
1710-
logrus.Warnf("Can't stat lower layer %q because it does not exist. Going through storage to recreate the missing symlinks.", newpath)
1711-
if err := d.recreateSymlinks(); err != nil {
1712-
return "", fmt.Errorf("recreating the missing symlinks: %w", err)
1713-
}
1714-
lower = newpath
1715-
} else if lower == "" {
1628+
if lower == "" {
17161629
return "", fmt.Errorf("can't stat lower layer %q: %w", newpath, err)
17171630
}
17181631
} else {
@@ -1725,7 +1638,12 @@ func (d *Driver) get(id string, disableShifting bool, options graphdriver.MountO
17251638

17261639
linkContent, err := os.Readlink(lower)
17271640
if err != nil {
1728-
return "", err
1641+
if !errors.Is(err, syscall.EINVAL) {
1642+
return "", err
1643+
}
1644+
// Not a symlink: layer ID from lower-layers, append /diff.
1645+
lower = path.Join(lower, "diff")
1646+
linkContent = lower
17291647
}
17301648
lowerID := filepath.Base(filepath.Dir(linkContent))
17311649
composefsMount, err := maybeAddComposefsMount(lowerID, i+1, readWrite)

storage/tests/overlay-recreate.bats

Lines changed: 0 additions & 36 deletions
This file was deleted.

0 commit comments

Comments
 (0)