Skip to content

Commit 006290d

Browse files
sjmiller609claude
andcommitted
fork: hardlink shared mem-file to dodge restore-alias ELOOP
The previous fork-mem-file symlink looped through withSnapshotSourceDirAlias during firecracker restore: fork/.../memory -> source/.../memory, but the alias dance temporarily symlinks source dir -> fork dir, so resolution ping-pongs back to fork/.../memory and trips ELOOP. Switching to a hardlink makes the fork's mem-file resolve by inode so the temporary directory alias no longer affects it. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 981371a commit 006290d

3 files changed

Lines changed: 54 additions & 33 deletions

File tree

lib/instances/firecracker_test.go

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"os"
1111
"path/filepath"
1212
"strings"
13+
"syscall"
1314
"testing"
1415
"time"
1516

@@ -557,7 +558,8 @@ func TestFirecrackerSnapshotFeature(t *testing.T) {
557558
// Standby, the first fork implicitly promotes it to Template, and the fork:
558559
//
559560
// (a) reaches Running,
560-
// (b) has its mem-file as a symlink into the template's snapshot dir,
561+
// (b) has its mem-file hardlinked to the source's snapshot mem-file
562+
// (the fan-out optimisation),
561563
// (c) bumps the template's ForkCount to 1,
562564
// (d) registers with the per-template uffd page server,
563565
// (e) on delete, decrements the ForkCount and detaches from uffd.
@@ -617,15 +619,20 @@ func TestFirecrackerForkFromTemplate(t *testing.T) {
617619
}
618620
})
619621

620-
// (b) The fork's mem-file must be a symlink into the source's snapshot.
622+
// (b) The fork's mem-file must share the source's inode (hardlink), not
623+
// be a copy. We can't compare paths because the link is by inode; we
624+
// compare st_ino + st_dev between the two instances' mem-files.
621625
forkMemPath := filepath.Join(p.InstanceSnapshotLatest(forkID), templateSharedMemFileName)
622-
info, err := os.Lstat(forkMemPath)
626+
srcMemPath := filepath.Join(p.InstanceSnapshotLatest(sourceID), templateSharedMemFileName)
627+
forkInfo, err := os.Stat(forkMemPath)
623628
require.NoError(t, err, "fork mem-file should exist at snapshot-latest/memory")
624-
assert.Equal(t, os.ModeSymlink, info.Mode()&os.ModeSymlink, "fork mem-file should be a symlink, not a copy")
625-
target, err := os.Readlink(forkMemPath)
629+
assert.True(t, forkInfo.Mode().IsRegular(), "fork mem-file should be a regular file (hardlink), not a symlink")
630+
srcInfo, err := os.Stat(srcMemPath)
626631
require.NoError(t, err)
627-
expectedTarget := filepath.Join(p.InstanceSnapshotLatest(sourceID), templateSharedMemFileName)
628-
assert.Equal(t, expectedTarget, target, "fork mem-file symlink should point at the template's mem-file")
632+
forkSys := forkInfo.Sys().(*syscall.Stat_t)
633+
srcSys := srcInfo.Sys().(*syscall.Stat_t)
634+
assert.Equal(t, srcSys.Ino, forkSys.Ino, "fork mem-file should share the source's inode (hardlink, not copy)")
635+
assert.Equal(t, srcSys.Dev, forkSys.Dev, "fork mem-file should be on the same filesystem as source")
629636

630637
// (c) The source instance is now a Template with ForkCount=1.
631638
sourceMeta, err := mgr.loadMetadata(sourceID)

lib/instances/templates.go

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,22 @@ const (
1111
templateSharedMemFileRelPath = "snapshots/snapshot-latest/memory"
1212
)
1313

14-
// installForkSharedMemFile arranges the fork's snapshot directory so the
15-
// guest mem-file is a symlink into the source template instance's snapshot
16-
// directory instead of a per-fork copy. firecracker mmaps the mem-file
17-
// MAP_PRIVATE during restore, so all forks COW from the same backing file.
14+
// guest mem-file is a hardlink to the source template instance's snapshot
15+
// mem-file instead of a per-fork copy. firecracker mmaps the mem-file
16+
// MAP_PRIVATE during restore, so all forks COW from the same backing inode.
1817
//
1918
// Layout: forkDataDir is the fork's data dir. The snapshot dir is at
2019
// <forkDataDir>/snapshots/snapshot-latest, and the mem-file lives at
21-
// <snapshot dir>/memory. The symlink target is the source instance's
22-
// standby snapshot mem-file.
20+
// <snapshot dir>/memory. The hardlink shares the inode with the source
21+
// instance's standby snapshot mem-file.
22+
//
23+
// We use a hardlink rather than a symlink because RestoreVM temporarily
24+
// aliases the source data dir to the fork data dir while firecracker
25+
// loads the snapshot (see withSnapshotSourceDirAlias). A symlink whose
26+
// target traverses the source dir would resolve back into the fork dir
27+
// during that window and trip ELOOP; a hardlink resolves by inode so
28+
// the alias has no effect on it. Hardlinks require both paths on the
29+
// same filesystem, which holds for our standard data-dir layout.
2330
func (m *manager) installForkSharedMemFile(forkDataDir, sourceInstanceID string) error {
2431
srcMem := filepath.Join(m.paths.InstanceSnapshotLatest(sourceInstanceID), templateSharedMemFileName)
2532
if _, err := os.Stat(srcMem); err != nil {
@@ -31,8 +38,8 @@ func (m *manager) installForkSharedMemFile(forkDataDir, sourceInstanceID string)
3138
}
3239
dstMem := filepath.Join(dstSnapshotDir, templateSharedMemFileName)
3340
_ = os.Remove(dstMem)
34-
if err := os.Symlink(srcMem, dstMem); err != nil {
35-
return fmt.Errorf("symlink shared mem-file: %w", err)
41+
if err := os.Link(srcMem, dstMem); err != nil {
42+
return fmt.Errorf("hardlink shared mem-file: %w", err)
3643
}
3744
return nil
3845
}

lib/instances/templates_shared_memfile_test.go

Lines changed: 25 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4,17 +4,29 @@ import (
44
"context"
55
"os"
66
"path/filepath"
7+
"syscall"
78
"testing"
89

910
"github.com/kernel/hypeman/lib/hypervisor"
1011
"github.com/stretchr/testify/assert"
1112
"github.com/stretchr/testify/require"
1213
)
1314

14-
// TestInstallForkSharedMemFile_SymlinksSourceMemFile verifies that the helper
15-
// creates a symlink at the fork's snapshot mem-file path pointing back to the
16-
// source instance's mem-file.
17-
func TestInstallForkSharedMemFile_SymlinksSourceMemFile(t *testing.T) {
15+
func sameInode(t *testing.T, a, b string) bool {
16+
t.Helper()
17+
ai, err := os.Stat(a)
18+
require.NoError(t, err)
19+
bi, err := os.Stat(b)
20+
require.NoError(t, err)
21+
as := ai.Sys().(*syscall.Stat_t)
22+
bs := bi.Sys().(*syscall.Stat_t)
23+
return as.Ino == bs.Ino && as.Dev == bs.Dev
24+
}
25+
26+
// TestInstallForkSharedMemFile_HardlinksSourceMemFile verifies that the helper
27+
// creates a hardlink at the fork's snapshot mem-file path that shares the
28+
// source instance's mem-file inode.
29+
func TestInstallForkSharedMemFile_HardlinksSourceMemFile(t *testing.T) {
1830
t.Parallel()
1931

2032
mgr, _ := newStorageOnlyManager(t)
@@ -32,16 +44,13 @@ func TestInstallForkSharedMemFile_SymlinksSourceMemFile(t *testing.T) {
3244
forkMem := filepath.Join(forkDir, "snapshots", "snapshot-latest", templateSharedMemFileName)
3345
info, err := os.Lstat(forkMem)
3446
require.NoError(t, err)
35-
assert.NotZero(t, info.Mode()&os.ModeSymlink, "fork mem-file must be a symlink, not a regular file")
36-
37-
target, err := os.Readlink(forkMem)
38-
require.NoError(t, err)
39-
assert.Equal(t, srcMem, target)
47+
assert.True(t, info.Mode().IsRegular(), "fork mem-file must be a regular file (hardlink), not a symlink")
48+
assert.True(t, sameInode(t, srcMem, forkMem), "fork mem-file must share the source's inode")
4049
}
4150

4251
// TestInstallForkSharedMemFile_ErrorsWhenSourceMissing makes sure the helper
43-
// refuses to silently create a dangling symlink when the source mem-file does
44-
// not exist.
52+
// refuses to silently create a dangling link when the source mem-file does not
53+
// exist.
4554
func TestInstallForkSharedMemFile_ErrorsWhenSourceMissing(t *testing.T) {
4655
t.Parallel()
4756

@@ -54,7 +63,7 @@ func TestInstallForkSharedMemFile_ErrorsWhenSourceMissing(t *testing.T) {
5463

5564
// TestForkFirecrackerSharesMemFile_FromTemplate verifies the end-to-end fork
5665
// path: when the source is a Firecracker Template instance, the fork's
57-
// mem-file is a symlink to the source's mem-file instead of a copy. This
66+
// mem-file is a hardlink to the source's mem-file instead of a copy. This
5867
// preserves the firecracker MAP_PRIVATE COW semantics that let multiple forks
5968
// share the heavy backing file.
6069
func TestForkFirecrackerSharesMemFile_FromTemplate(t *testing.T) {
@@ -84,11 +93,8 @@ func TestForkFirecrackerSharesMemFile_FromTemplate(t *testing.T) {
8493
forkMem := filepath.Join(mgr.paths.InstanceSnapshotLatest(forked.Id), templateSharedMemFileName)
8594
info, err := os.Lstat(forkMem)
8695
require.NoError(t, err)
87-
assert.NotZero(t, info.Mode()&os.ModeSymlink, "fork mem-file must be a symlink for firecracker fan-out")
88-
89-
target, err := os.Readlink(forkMem)
90-
require.NoError(t, err)
91-
assert.Equal(t, srcMem, target)
96+
assert.True(t, info.Mode().IsRegular(), "fork mem-file must be a regular file (hardlink) for firecracker fan-out")
97+
assert.True(t, sameInode(t, srcMem, forkMem), "fork mem-file must share the source's inode")
9298
}
9399

94100
// TestForkFirecrackerStandbySourceDoesNotShareMemFile guards the
@@ -121,7 +127,8 @@ func TestForkFirecrackerStandbySourceDoesNotShareMemFile(t *testing.T) {
121127
forkMem := filepath.Join(mgr.paths.InstanceSnapshotLatest(forked.Id), templateSharedMemFileName)
122128
info, err := os.Lstat(forkMem)
123129
require.NoError(t, err)
124-
assert.Zero(t, info.Mode()&os.ModeSymlink, "standby-source fork mem-file must be a regular file copy, not a symlink")
130+
require.True(t, info.Mode().IsRegular(), "standby-source fork mem-file must be a regular file copy")
131+
assert.False(t, sameInode(t, srcMem, forkMem), "standby-source fork mem-file must be a copy, not a hardlink to source")
125132
}
126133

127134
// promoteFixtureToTemplate marks the source's stored metadata as a Template

0 commit comments

Comments
 (0)