Skip to content

Commit 823a7b8

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 01e7561 commit 823a7b8

2 files changed

Lines changed: 66 additions & 184 deletions

File tree

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

@@ -1178,6 +1184,35 @@ func (d *Driver) create(id, parent string, opts *graphdriver.CreateOpts, readOnl
11781184
}
11791185
}
11801186

1187+
// Write a lower-layers file referencing layers by ID instead of
1188+
// l/ symlink references. The reading side appends "/diff" itself.
1189+
parentDir := d.dir(parent)
1190+
layerLower := parent
1191+
parentLower, err := os.ReadFile(path.Join(parentDir, lowerLayersFile))
1192+
if err == nil {
1193+
layerLower += ":" + string(parentLower)
1194+
} else if !errors.Is(err, unix.ENOENT) {
1195+
return err
1196+
} else {
1197+
// Parent has no lower-layers file. Convert old-format lower
1198+
// entries (l/ symlinks) to layer IDs.
1199+
oldLower, err := os.ReadFile(path.Join(parentDir, lowerFile))
1200+
if err == nil {
1201+
for _, s := range strings.Split(string(oldLower), ":") {
1202+
target, err := os.Readlink(d.dir(s))
1203+
if err != nil {
1204+
return fmt.Errorf("reading symlink for lower %q: %w", s, err)
1205+
}
1206+
layerLower += ":" + filepath.Base(filepath.Dir(target))
1207+
}
1208+
} else if !errors.Is(err, unix.ENOENT) {
1209+
return err
1210+
}
1211+
}
1212+
if err := os.WriteFile(path.Join(dir, lowerLayersFile), []byte(layerLower), 0o666); err != nil {
1213+
return err
1214+
}
1215+
11811216
return nil
11821217
}
11831218

@@ -1215,20 +1250,9 @@ func (d *Driver) getLower(parent string) (string, error) {
12151250
return "", err
12161251
}
12171252

1218-
// Read Parent link fileA
12191253
parentLink, err := os.ReadFile(path.Join(parentDir, "link"))
12201254
if err != nil {
1221-
if !errors.Is(err, fs.ErrNotExist) {
1222-
return "", err
1223-
}
1224-
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"))
1225-
if err := d.recreateSymlinks(); err != nil {
1226-
return "", fmt.Errorf("recreating the links: %w", err)
1227-
}
1228-
parentLink, err = os.ReadFile(path.Join(parentDir, "link"))
1229-
if err != nil {
1230-
return "", err
1231-
}
1255+
return "", err
12321256
}
12331257
lowers := []string{path.Join(linkDir, string(parentLink))}
12341258

@@ -1283,27 +1307,25 @@ func (d *Driver) dir2(id string, useImageStore bool) (string, string, bool) {
12831307

12841308
func (d *Driver) getLowerDirs(id string) ([]string, error) {
12851309
var lowersArray []string
1286-
lowers, err := os.ReadFile(path.Join(d.dir(id), lowerFile))
1310+
dir := d.dir(id)
1311+
lowers, err := os.ReadFile(path.Join(dir, lowerLayersFile))
1312+
if err != nil {
1313+
if !errors.Is(err, unix.ENOENT) {
1314+
return nil, err
1315+
}
1316+
lowers, err = os.ReadFile(path.Join(dir, lowerFile))
1317+
}
12871318
if err == nil {
12881319
for s := range strings.SplitSeq(string(lowers), ":") {
12891320
lower := d.dir(s)
12901321
lp, err := os.Readlink(lower)
1291-
// if the link does not exist, we lost the symlinks during a sudden reboot.
1292-
// Let's go ahead and recreate those symlinks.
12931322
if err != nil {
1294-
if errors.Is(err, fs.ErrNotExist) {
1295-
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)
1296-
if err := d.recreateSymlinks(); err != nil {
1297-
return nil, fmt.Errorf("recreating the missing symlinks: %w", err)
1298-
}
1299-
// let's call Readlink on lower again now that we have recreated the missing symlinks
1300-
lp, err = os.Readlink(lower)
1301-
if err != nil {
1302-
return nil, err
1303-
}
1304-
} else {
1305-
return nil, err
1323+
if errors.Is(err, syscall.EINVAL) {
1324+
// Not a symlink: layer ID, append /diff.
1325+
lowersArray = append(lowersArray, path.Join(lower, "diff"))
1326+
continue
13061327
}
1328+
return nil, err
13071329
}
13081330
lowersArray = append(lowersArray, path.Clean(d.dir(path.Join("link", lp))))
13091331
}
@@ -1415,112 +1437,6 @@ func (d *Driver) DeferredRemove(id string) (tempdir.CleanupTempDirFunc, error) {
14151437
return t.Cleanup, nil
14161438
}
14171439

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

1618-
lowers, err := os.ReadFile(path.Join(dir, lowerFile))
1619-
if err != nil && !errors.Is(err, fs.ErrNotExist) {
1620-
return "", err
1534+
lowers, err := os.ReadFile(path.Join(dir, lowerLayersFile))
1535+
if err != nil {
1536+
if !errors.Is(err, unix.ENOENT) {
1537+
return "", err
1538+
}
1539+
lowers, err = os.ReadFile(path.Join(dir, lowerFile))
1540+
if err != nil && !os.IsNotExist(err) {
1541+
return "", err
1542+
}
16211543
}
16221544
splitLowers := strings.Split(string(lowers), ":")
16231545
if len(splitLowers) > maxDepth {
@@ -1729,16 +1651,7 @@ func (d *Driver) get(id string, disableShifting bool, options graphdriver.MountO
17291651
}
17301652
lower = ""
17311653
}
1732-
// if it is a "not found" error, that means the symlinks were lost in a sudden reboot
1733-
// so call the recreateSymlinks function to go through all the layer dirs and recreate
1734-
// the symlinks with the name from their respective "link" files
1735-
if lower == "" && errors.Is(err, fs.ErrNotExist) {
1736-
logrus.Warnf("Can't stat lower layer %q because it does not exist. Going through storage to recreate the missing symlinks.", newpath)
1737-
if err := d.recreateSymlinks(); err != nil {
1738-
return "", fmt.Errorf("recreating the missing symlinks: %w", err)
1739-
}
1740-
lower = newpath
1741-
} else if lower == "" {
1654+
if lower == "" {
17421655
return "", fmt.Errorf("can't stat lower layer %q: %w", newpath, err)
17431656
}
17441657
} else {
@@ -1751,7 +1664,12 @@ func (d *Driver) get(id string, disableShifting bool, options graphdriver.MountO
17511664

17521665
linkContent, err := os.Readlink(lower)
17531666
if err != nil {
1754-
return "", err
1667+
if !errors.Is(err, syscall.EINVAL) {
1668+
return "", err
1669+
}
1670+
// Not a symlink: layer ID from lower-layers, append /diff.
1671+
lower = path.Join(lower, "diff")
1672+
linkContent = lower
17551673
}
17561674
lowerID := filepath.Base(filepath.Dir(linkContent))
17571675
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)