From eb3385d490523262e26b6cb691a5e18a07a8b26d Mon Sep 17 00:00:00 2001 From: Ophestra Date: Fri, 29 Aug 2025 01:46:54 +0900 Subject: [PATCH] container/initsymlink: unwrap mount errors The mount function now wraps its own errors in a much more descriptive type with proper message formatting. Wrapping them no longer makes any sense. Signed-off-by: Ophestra --- container/init_test.go | 8 +++----- container/initdev.go | 8 +++----- container/initdev_test.go | 4 ++-- container/initoverlay.go | 3 +-- container/initoverlay_test.go | 2 +- container/initproc.go | 3 +-- container/initremount.go | 3 +-- container/mount.go | 17 +++++------------ container/mount_test.go | 6 +++--- 9 files changed, 20 insertions(+), 34 deletions(-) diff --git a/container/init_test.go b/container/init_test.go index de165da..94e165d 100644 --- a/container/init_test.go +++ b/container/init_test.go @@ -377,7 +377,7 @@ func TestInitEntrypoint(t *testing.T) { }, }, nil}, - {"apply", func(k syscallDispatcher) error { initEntrypoint(k, assertPrefix, assertVerboseTrue); return nil }, [][]kexpect{ + {"early unhandled error", func(k syscallDispatcher) error { initEntrypoint(k, assertPrefix, assertVerboseTrue); return nil }, [][]kexpect{ { {"lockOSThread", expectArgs{}, nil, nil}, {"getpid", expectArgs{}, 1, nil}, @@ -759,10 +759,8 @@ func TestInitEntrypoint(t *testing.T) { {"bindMount", expectArgs{"/host", "/sysroot", uintptr(0x4001), false}, nil, nil}, {"verbosef", expectArgs{"%s %s", []any{"mounting", &MountProcOp{Target: MustAbs("/proc/")}}}, nil, nil}, {"mkdirAll", expectArgs{"/sysroot/proc", os.FileMode(0755)}, nil, nil}, - {"mount", expectArgs{"proc", "/sysroot/proc", "proc", uintptr(0xe), ""}, nil, errUnique}, - {"printBaseErr", expectArgs{wrapErrSuffix(errUnique, `cannot mount proc on "/proc/":`), "cannot apply op at index 1:"}, nil, nil}, - {"beforeExit", expectArgs{}, nil, nil}, - {"exit", expectArgs{1}, nil, nil}, + {"mount", expectArgs{"proc", "/sysroot/proc", "proc", uintptr(0xe), ""}, nil, &MountError{"proc", "/sysroot/proc", "proc", uintptr(0xe), "", syscall.ENOTRECOVERABLE}}, + {"fatal", expectArgs{[]any{"cannot mount proc on /sysroot/proc: state not recoverable"}}, nil, nil}, /* end apply */ }, }, nil}, diff --git a/container/initdev.go b/container/initdev.go index 43ef524..9bb9561 100644 --- a/container/initdev.go +++ b/container/initdev.go @@ -82,8 +82,7 @@ func (d *MountDevOp) apply(state *setupState, k syscallDispatcher) error { if err := k.mount(SourceDevpts, devPtsPath, FstypeDevpts, MS_NOSUID|MS_NOEXEC, "newinstance,ptmxmode=0666,mode=620"); err != nil { - return wrapErrSuffix(err, - fmt.Sprintf("cannot mount devpts on %q:", devPtsPath)) + return err } if state.RetainSession { @@ -111,7 +110,7 @@ func (d *MountDevOp) apply(state *setupState, k syscallDispatcher) error { return wrapErrSelf(err) } if err := k.mount(SourceMqueue, mqueueTarget, FstypeMqueue, MS_NOSUID|MS_NOEXEC|MS_NODEV, zeroString); err != nil { - return wrapErrSuffix(err, "cannot mount mqueue:") + return err } } @@ -120,8 +119,7 @@ func (d *MountDevOp) apply(state *setupState, k syscallDispatcher) error { } if err := k.remount(target, MS_RDONLY); err != nil { - return wrapErrSuffix(k.remount(target, MS_RDONLY), - fmt.Sprintf("cannot remount %q:", target)) + return err } return k.mountTmpfs(SourceTmpfs, devShmPath, MS_NOSUID|MS_NODEV, 0, 01777) } diff --git a/container/initdev_test.go b/container/initdev_test.go index de756d0..b7aef45 100644 --- a/container/initdev_test.go +++ b/container/initdev_test.go @@ -390,7 +390,7 @@ func TestMountDevOp(t *testing.T) { {"mkdir", expectArgs{"/sysroot/dev/shm", os.FileMode(0750)}, nil, nil}, {"mkdir", expectArgs{"/sysroot/dev/pts", os.FileMode(0750)}, nil, nil}, {"mount", expectArgs{"devpts", "/sysroot/dev/pts", "devpts", uintptr(0xa), "newinstance,ptmxmode=0666,mode=620"}, nil, errUnique}, - }, wrapErrSuffix(errUnique, `cannot mount devpts on "/sysroot/dev/pts":`)}, + }, errUnique}, {"ensureFile stdout", &Params{ParentPerm: 0750, RetainSession: true}, &MountDevOp{ Target: MustAbs("/dev/"), @@ -550,7 +550,7 @@ func TestMountDevOp(t *testing.T) { {"bindMount", expectArgs{"/host/dev/pts/2", "/sysroot/dev/console", uintptr(0), false}, nil, nil}, {"mkdir", expectArgs{"/sysroot/dev/mqueue", os.FileMode(0750)}, nil, nil}, {"mount", expectArgs{"mqueue", "/sysroot/dev/mqueue", "mqueue", uintptr(0xe), ""}, nil, errUnique}, - }, wrapErrSuffix(errUnique, "cannot mount mqueue:")}, + }, errUnique}, {"success no session", &Params{ParentPerm: 0755}, &MountDevOp{ Target: MustAbs("/dev/"), diff --git a/container/initoverlay.go b/container/initoverlay.go index 44c8348..2e648d8 100644 --- a/container/initoverlay.go +++ b/container/initoverlay.go @@ -167,8 +167,7 @@ func (o *MountOverlayOp) apply(state *setupState, k syscallDispatcher) error { OptionOverlayLowerdir+"="+strings.Join(o.lower, SpecialOverlayPath), OptionOverlayUserxattr) - return wrapErrSuffix(k.mount(SourceOverlay, target, FstypeOverlay, 0, strings.Join(options, SpecialOverlayOption)), - fmt.Sprintf("cannot mount overlay on %q:", o.Target)) + return k.mount(SourceOverlay, target, FstypeOverlay, 0, strings.Join(options, SpecialOverlayOption)) } func (o *MountOverlayOp) Is(op Op) bool { diff --git a/container/initoverlay_test.go b/container/initoverlay_test.go index e12cffe..2c076f2 100644 --- a/container/initoverlay_test.go +++ b/container/initoverlay_test.go @@ -186,7 +186,7 @@ func TestMountOverlayOp(t *testing.T) { }, nil, []kexpect{ {"mkdirAll", expectArgs{"/sysroot/nix/store", os.FileMode(0700)}, nil, nil}, {"mount", expectArgs{"overlay", "/sysroot/nix/store", "overlay", uintptr(0), "upperdir=/host/mnt-root/nix/.rw-store/.upper,workdir=/host/mnt-root/nix/.rw-store/.work,lowerdir=/host/mnt-root/nix/ro-store,userxattr"}, nil, errUnique}, - }, wrapErrSuffix(errUnique, `cannot mount overlay on "/nix/store":`)}, + }, errUnique}, {"success single layer", &Params{ParentPerm: 0700}, &MountOverlayOp{ Target: MustAbs("/nix/store"), diff --git a/container/initproc.go b/container/initproc.go index fe02aeb..d003809 100644 --- a/container/initproc.go +++ b/container/initproc.go @@ -24,8 +24,7 @@ func (p *MountProcOp) apply(state *setupState, k syscallDispatcher) error { if err := k.mkdirAll(target, state.ParentPerm); err != nil { return wrapErrSelf(err) } - return wrapErrSuffix(k.mount(SourceProc, target, FstypeProc, MS_NOSUID|MS_NOEXEC|MS_NODEV, zeroString), - fmt.Sprintf("cannot mount proc on %q:", p.Target.String())) + return k.mount(SourceProc, target, FstypeProc, MS_NOSUID|MS_NOEXEC|MS_NODEV, zeroString) } func (p *MountProcOp) Is(op Op) bool { diff --git a/container/initremount.go b/container/initremount.go index af58c74..6df8a33 100644 --- a/container/initremount.go +++ b/container/initremount.go @@ -22,8 +22,7 @@ type RemountOp struct { func (r *RemountOp) Valid() bool { return r != nil && r.Target != nil } func (*RemountOp) early(*setupState, syscallDispatcher) error { return nil } func (r *RemountOp) apply(_ *setupState, k syscallDispatcher) error { - return wrapErrSuffix(k.remount(toSysroot(r.Target.String()), r.Flags), - fmt.Sprintf("cannot remount %q:", r.Target)) + return k.remount(toSysroot(r.Target.String()), r.Flags) } func (r *RemountOp) Is(op Op) bool { diff --git a/container/mount.go b/container/mount.go index f7d4af3..fc510b4 100644 --- a/container/mount.go +++ b/container/mount.go @@ -106,8 +106,7 @@ func (p *procPaths) bindMount(source, target string, flags uintptr, eq bool) err } if err := p.k.mount(source, target, FstypeNULL, MS_SILENT|MS_BIND|flags&MS_REC, zeroString); err != nil { - return wrapErrSuffix(err, - fmt.Sprintf("cannot mount %q on %q:", source, target)) + return err } return p.k.remount(target, flags) @@ -161,8 +160,7 @@ func (p *procPaths) remount(target string, flags uintptr) error { } if err = remountWithFlags(p.k, n, mf); err != nil { - return wrapErrSuffix(err, - fmt.Sprintf("cannot remount %q:", n.Clean)) + return err } if flags&MS_REC == 0 { return nil @@ -174,11 +172,8 @@ func (p *procPaths) remount(target string, flags uintptr) error { continue } - err = remountWithFlags(p.k, cur, mf) - - if err != nil && !errors.Is(err, EACCES) { - return wrapErrSuffix(err, - fmt.Sprintf("cannot propagate flags to %q:", cur.Clean)) + if err = remountWithFlags(p.k, cur, mf); err != nil && !errors.Is(err, EACCES) { + return err } } @@ -213,9 +208,7 @@ func mountTmpfs(k syscallDispatcher, fsname, target string, flags uintptr, size if size > 0 { opt += fmt.Sprintf(",size=%d", size) } - return wrapErrSuffix( - k.mount(fsname, target, FstypeTmpfs, flags, opt), - fmt.Sprintf("cannot mount tmpfs on %q:", target)) + return k.mount(fsname, target, FstypeTmpfs, flags, opt) } func parentPerm(perm os.FileMode) os.FileMode { diff --git a/container/mount_test.go b/container/mount_test.go index 2b5b1c4..c3d9ad3 100644 --- a/container/mount_test.go +++ b/container/mount_test.go @@ -15,7 +15,7 @@ func TestBindMount(t *testing.T) { }, [][]kexpect{{ {"verbosef", expectArgs{"resolved %q flags %#x", []any{"/sysroot/nix", uintptr(1)}}, nil, nil}, {"mount", expectArgs{"/host/nix", "/sysroot/nix", "", uintptr(0x9000), ""}, nil, errUnique}, - }}, wrapErrSuffix(errUnique, `cannot mount "/host/nix" on "/sysroot/nix":`)}, + }}, errUnique}, {"success ne", func(k syscallDispatcher) error { return newProcPaths(k, hostPath).bindMount("/host/nix", "/sysroot/.host-nix", syscall.MS_RDONLY, false) @@ -139,7 +139,7 @@ func TestRemount(t *testing.T) { {"close", expectArgs{0xdeadbeef}, nil, nil}, {"openNew", expectArgs{"/host/proc/self/mountinfo"}, newConstFile(sampleMountinfoNix), nil}, {"mount", expectArgs{"none", "/sysroot/nix", "", uintptr(0x209027), ""}, nil, errUnique}, - }}, wrapErrSuffix(errUnique, `cannot remount "/sysroot/nix":`)}, + }}, errUnique}, {"mount propagate", func(k syscallDispatcher) error { return newProcPaths(k, hostPath).remount("/sysroot/nix", syscall.MS_REC|syscall.MS_RDONLY|syscall.MS_NODEV) @@ -151,7 +151,7 @@ func TestRemount(t *testing.T) { {"openNew", expectArgs{"/host/proc/self/mountinfo"}, newConstFile(sampleMountinfoNix), nil}, {"mount", expectArgs{"none", "/sysroot/nix", "", uintptr(0x209027), ""}, nil, nil}, {"mount", expectArgs{"none", "/sysroot/nix/.ro-store", "", uintptr(0x209027), ""}, nil, errUnique}, - }}, wrapErrSuffix(errUnique, `cannot propagate flags to "/sysroot/nix/.ro-store":`)}, + }}, errUnique}, {"success toplevel", func(k syscallDispatcher) error { return newProcPaths(k, hostPath).remount("/sysroot/bin", syscall.MS_REC|syscall.MS_RDONLY|syscall.MS_NODEV)