Skip to content

Commit e345324

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 54775bd commit e345324

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

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

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

@@ -1191,20 +1226,9 @@ func (d *Driver) getLower(parent string) (string, error) {
11911226
return "", err
11921227
}
11931228

1194-
// Read Parent link fileA
11951229
parentLink, err := os.ReadFile(path.Join(parentDir, "link"))
11961230
if err != nil {
1197-
if !errors.Is(err, fs.ErrNotExist) {
1198-
return "", err
1199-
}
1200-
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"))
1201-
if err := d.recreateSymlinks(); err != nil {
1202-
return "", fmt.Errorf("recreating the links: %w", err)
1203-
}
1204-
parentLink, err = os.ReadFile(path.Join(parentDir, "link"))
1205-
if err != nil {
1206-
return "", err
1207-
}
1231+
return "", err
12081232
}
12091233
lowers := []string{path.Join(linkDir, string(parentLink))}
12101234

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

12601284
func (d *Driver) getLowerDirs(id string) ([]string, error) {
12611285
var lowersArray []string
1262-
lowers, err := os.ReadFile(path.Join(d.dir(id), lowerFile))
1286+
dir := d.dir(id)
1287+
lowers, err := os.ReadFile(path.Join(dir, lowerLayersFile))
1288+
if err != nil {
1289+
if !errors.Is(err, unix.ENOENT) {
1290+
return nil, err
1291+
}
1292+
lowers, err = os.ReadFile(path.Join(dir, lowerFile))
1293+
}
12631294
if err == nil {
12641295
for s := range strings.SplitSeq(string(lowers), ":") {
12651296
lower := d.dir(s)
12661297
lp, err := os.Readlink(lower)
1267-
// if the link does not exist, we lost the symlinks during a sudden reboot.
1268-
// Let's go ahead and recreate those symlinks.
12691298
if err != nil {
1270-
if errors.Is(err, fs.ErrNotExist) {
1271-
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)
1272-
if err := d.recreateSymlinks(); err != nil {
1273-
return nil, fmt.Errorf("recreating the missing symlinks: %w", err)
1274-
}
1275-
// let's call Readlink on lower again now that we have recreated the missing symlinks
1276-
lp, err = os.Readlink(lower)
1277-
if err != nil {
1278-
return nil, err
1279-
}
1280-
} else {
1281-
return nil, err
1299+
if errors.Is(err, syscall.EINVAL) {
1300+
// Not a symlink: layer ID, append /diff.
1301+
lowersArray = append(lowersArray, path.Join(lower, "diff"))
1302+
continue
12821303
}
1304+
return nil, err
12831305
}
12841306
lowersArray = append(lowersArray, path.Clean(d.dir(path.Join("link", lp))))
12851307
}
@@ -1391,112 +1413,6 @@ func (d *Driver) DeferredRemove(id string) (tempdir.CleanupTempDirFunc, error) {
13911413
return t.Cleanup, nil
13921414
}
13931415

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

1594-
lowers, err := os.ReadFile(path.Join(dir, lowerFile))
1595-
if err != nil && !errors.Is(err, fs.ErrNotExist) {
1596-
return "", err
1510+
lowers, err := os.ReadFile(path.Join(dir, lowerLayersFile))
1511+
if err != nil {
1512+
if !errors.Is(err, unix.ENOENT) {
1513+
return "", err
1514+
}
1515+
lowers, err = os.ReadFile(path.Join(dir, lowerFile))
1516+
if err != nil && !os.IsNotExist(err) {
1517+
return "", err
1518+
}
15971519
}
15981520
splitLowers := strings.Split(string(lowers), ":")
15991521
if len(splitLowers) > maxDepth {
@@ -1705,16 +1627,7 @@ func (d *Driver) get(id string, disableShifting bool, options graphdriver.MountO
17051627
}
17061628
lower = ""
17071629
}
1708-
// if it is a "not found" error, that means the symlinks were lost in a sudden reboot
1709-
// so call the recreateSymlinks function to go through all the layer dirs and recreate
1710-
// the symlinks with the name from their respective "link" files
1711-
if lower == "" && errors.Is(err, fs.ErrNotExist) {
1712-
logrus.Warnf("Can't stat lower layer %q because it does not exist. Going through storage to recreate the missing symlinks.", newpath)
1713-
if err := d.recreateSymlinks(); err != nil {
1714-
return "", fmt.Errorf("recreating the missing symlinks: %w", err)
1715-
}
1716-
lower = newpath
1717-
} else if lower == "" {
1630+
if lower == "" {
17181631
return "", fmt.Errorf("can't stat lower layer %q: %w", newpath, err)
17191632
}
17201633
} else {
@@ -1727,7 +1640,12 @@ func (d *Driver) get(id string, disableShifting bool, options graphdriver.MountO
17271640

17281641
linkContent, err := os.Readlink(lower)
17291642
if err != nil {
1730-
return "", err
1643+
if !errors.Is(err, syscall.EINVAL) {
1644+
return "", err
1645+
}
1646+
// Not a symlink: layer ID from lower-layers, append /diff.
1647+
lower = path.Join(lower, "diff")
1648+
linkContent = lower
17311649
}
17321650
lowerID := filepath.Base(filepath.Dir(linkContent))
17331651
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)