diff --git a/pkg/sentry/fsimpl/gofer/dentry_impl.go b/pkg/sentry/fsimpl/gofer/dentry_impl.go index 8bace5dce..1d5c3c02d 100644 --- a/pkg/sentry/fsimpl/gofer/dentry_impl.go +++ b/pkg/sentry/fsimpl/gofer/dentry_impl.go @@ -15,6 +15,8 @@ package gofer import ( + "fmt" + "golang.org/x/sys/unix" "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/atomicbitops" @@ -511,7 +513,7 @@ func (d *dentry) statfs(ctx context.Context) (linux.Statfs, error) { func (fs *filesystem) restoreRoot(ctx context.Context, opts *vfs.CompleteRestoreOptions) error { rootInode, rootHostFD, err := fs.initClientAndGetRoot(ctx) if err != nil { - return err + return fmt.Errorf("failed to initialize client and get root: %w", err) } // The root is always non-synthetic. @@ -536,14 +538,14 @@ func (d *dentry) restoreFile(ctx context.Context, opts *vfs.CompleteRestoreOptio inode, err := controlFD.Walk(ctx, d.name) if err != nil { if !dt.isDir() || !dt.forMountpoint { - return err + return fmt.Errorf("failed to walk %q of type %x: %w", genericDebugPathname(d), dt.fileType(), err) } // Recreate directories that were created during volume mounting, since // during restore we don't attempt to remount them. inode, err = controlFD.MkdirAt(ctx, d.name, linux.FileMode(d.mode.Load()), lisafs.UID(d.uid.Load()), lisafs.GID(d.gid.Load())) if err != nil { - return err + return fmt.Errorf("failed to create mountpoint directory at %q: %w", genericDebugPathname(d), err) } } return dt.restoreFile(ctx, &inode, opts) @@ -556,13 +558,13 @@ func (d *dentry) restoreFile(ctx context.Context, opts *vfs.CompleteRestoreOptio }) if err != nil { if !dt.isDir() || !dt.forMountpoint { - return err + return fmt.Errorf("failed to walk %q of type %x: %w", genericDebugPathname(d), dt.fileType(), err) } // Recreate directories that were created during volume mounting, since // during restore we don't attempt to remount them. if err := unix.Mkdirat(controlFD, d.name, d.mode.Load()); err != nil { - return err + return fmt.Errorf("failed to create mountpoint directory at %q: %w", genericDebugPathname(d), err) } // Try again... @@ -570,7 +572,7 @@ func (d *dentry) restoreFile(ctx context.Context, opts *vfs.CompleteRestoreOptio return unix.Openat(controlFD, d.name, flags, 0) }) if err != nil { - return err + return fmt.Errorf("failed to open %q: %w", genericDebugPathname(d), err) } } return dt.restoreFile(ctx, childFD, opts) diff --git a/pkg/sentry/fsimpl/gofer/directfs_dentry.go b/pkg/sentry/fsimpl/gofer/directfs_dentry.go index 4691f077b..7cc268289 100644 --- a/pkg/sentry/fsimpl/gofer/directfs_dentry.go +++ b/pkg/sentry/fsimpl/gofer/directfs_dentry.go @@ -645,13 +645,12 @@ func (d *directfsDentry) statfs() (linux.Statfs, error) { func (d *directfsDentry) restoreFile(ctx context.Context, controlFD int, opts *vfs.CompleteRestoreOptions) error { if controlFD < 0 { - log.Warningf("directfsDentry.restoreFile called with invalid controlFD") - return unix.EINVAL + return fmt.Errorf("directfsDentry.restoreFile called with invalid controlFD") } var stat unix.Stat_t if err := unix.Fstat(controlFD, &stat); err != nil { _ = unix.Close(controlFD) - return err + return fmt.Errorf("failed to stat %q: %w", genericDebugPathname(&d.dentry), err) } d.controlFD = controlFD @@ -688,7 +687,7 @@ func (d *directfsDentry) restoreFile(ctx context.Context, controlFD int, opts *v if rw, ok := d.fs.savedDentryRW[&d.dentry]; ok { if err := d.ensureSharedHandle(ctx, rw.read, rw.write, false /* trunc */); err != nil { - return err + return fmt.Errorf("failed to restore file handles (read=%t, write=%t) for %q: %w", rw.read, rw.write, genericDebugPathname(&d.dentry), err) } } diff --git a/pkg/sentry/fsimpl/gofer/lisafs_dentry.go b/pkg/sentry/fsimpl/gofer/lisafs_dentry.go index 245002e0e..28bf85add 100644 --- a/pkg/sentry/fsimpl/gofer/lisafs_dentry.go +++ b/pkg/sentry/fsimpl/gofer/lisafs_dentry.go @@ -568,7 +568,7 @@ func (d *lisafsDentry) restoreFile(ctx context.Context, inode *lisafs.Inode, opt if rw, ok := d.fs.savedDentryRW[&d.dentry]; ok { if err := d.ensureSharedHandle(ctx, rw.read, rw.write, false /* trunc */); err != nil { - return err + return fmt.Errorf("failed to restore file handles (read=%t, write=%t) for %q: %w", rw.read, rw.write, genericDebugPathname(&d.dentry), err) } } diff --git a/pkg/sentry/fsimpl/gofer/save_restore.go b/pkg/sentry/fsimpl/gofer/save_restore.go index e16e11e7e..459ac65fb 100644 --- a/pkg/sentry/fsimpl/gofer/save_restore.go +++ b/pkg/sentry/fsimpl/gofer/save_restore.go @@ -196,7 +196,7 @@ func (fs *filesystem) CompleteRestore(ctx context.Context, opts vfs.CompleteRest fs.inoByKey = make(map[inoKey]uint64) if err := fs.restoreRoot(ctx, &opts); err != nil { - return err + return vfs.PrependErrMsg("failed to restore root", err) } // Restore remaining dentries. @@ -260,7 +260,7 @@ func (fd *specialFileFD) completeRestore(ctx context.Context) error { d := fd.dentry() h, err := d.openHandle(ctx, fd.vfsfd.IsReadable(), fd.vfsfd.IsWritable(), false /* trunc */) if err != nil { - return err + return fmt.Errorf("failed to open handle for specialFileFD for %q: %w", genericDebugPathname(d), err) } fd.handle = h @@ -268,7 +268,7 @@ func (fd *specialFileFD) completeRestore(ctx context.Context) error { fd.haveQueue = (ftype == linux.S_IFIFO || ftype == linux.S_IFSOCK) && fd.handle.fd >= 0 if fd.haveQueue { if err := fdnotifier.AddFD(fd.handle.fd, &fd.queue); err != nil { - return err + return fmt.Errorf("failed to add FD to fdnotified for %q: %w", genericDebugPathname(d), err) } } diff --git a/pkg/sentry/kernel/kernel.go b/pkg/sentry/kernel/kernel.go index 549472629..a92915d36 100644 --- a/pkg/sentry/kernel/kernel.go +++ b/pkg/sentry/kernel/kernel.go @@ -839,7 +839,7 @@ func (k *Kernel) LoadFrom(ctx context.Context, r, pagesMetadata io.Reader, pages pagesFile = nil // transferred to k.loadMemoryFiles() } if mfLoadErr != nil { - return mfLoadErr + return fmt.Errorf("failed to load memory files: %w", mfLoadErr) } if !saveRestoreNet { @@ -861,7 +861,7 @@ func (k *Kernel) LoadFrom(ctx context.Context, r, pagesMetadata io.Reader, pages } if err := k.vfs.CompleteRestore(ctx, vfsOpts); err != nil { - return err + return vfs.PrependErrMsg("vfs.CompleteRestore() failed", err) } tcpip.AsyncLoading.Wait() diff --git a/pkg/sentry/pgalloc/save_restore.go b/pkg/sentry/pgalloc/save_restore.go index 2a12db715..951b86bb8 100644 --- a/pkg/sentry/pgalloc/save_restore.go +++ b/pkg/sentry/pgalloc/save_restore.go @@ -291,7 +291,7 @@ func (f *MemoryFile) LoadFrom(ctx context.Context, r io.Reader, opts *LoadOpts) f.chunks.Store(&chunks) log.Infof("MemoryFile(%p): loaded metadata in %s", f, time.Since(timeMetadataStart)) if err := f.file.Truncate(int64(len(chunks)) * chunkSize); err != nil { - return err + return fmt.Errorf("failed to truncate MemoryFile: %w", err) } // Obtain chunk mappings, then madvise them concurrently with loading data. var ( @@ -385,7 +385,7 @@ func (f *MemoryFile) LoadFrom(ctx context.Context, r io.Reader, opts *LoadOpts) // Verify header. length, object, err := state.ReadHeader(&wr) if err != nil { - return err + return fmt.Errorf("failed to read header: %w", err) } if object { // Not expected. @@ -419,7 +419,7 @@ func (f *MemoryFile) LoadFrom(ctx context.Context, r io.Reader, opts *LoadOpts) _, ioErr = io.ReadFull(r, s) }) if ioErr != nil { - return ioErr + return fmt.Errorf("failed to read pages: %w", ioErr) } } diff --git a/pkg/sentry/vfs/save_restore.go b/pkg/sentry/vfs/save_restore.go index 5b61b8e1e..5ceb4c36d 100644 --- a/pkg/sentry/vfs/save_restore.go +++ b/pkg/sentry/vfs/save_restore.go @@ -36,6 +36,18 @@ func (e ErrCorruption) Error() string { return "restore failed due to external file system state in corruption: " + e.Err.Error() } +// PrependErrMsg prepends the passed prefix to the error while preserving +// special vfs errors as the outer most error. +func PrependErrMsg(prefix string, err error) error { + switch terr := err.(type) { + case ErrCorruption: + terr.Err = fmt.Errorf("%s: %w", prefix, terr.Err) + return terr + default: + return fmt.Errorf("%s: %w", prefix, err) + } +} + // FilesystemImplSaveRestoreExtension is an optional extension to // FilesystemImpl. type FilesystemImplSaveRestoreExtension interface { @@ -68,7 +80,7 @@ func (vfs *VirtualFilesystem) CompleteRestore(ctx context.Context, opts *Complet if ext, ok := fs.impl.(FilesystemImplSaveRestoreExtension); ok { if err := ext.CompleteRestore(ctx, *opts); err != nil { fs.DecRef(ctx) - return err + return PrependErrMsg(fmt.Sprintf("failed to complete restore for filesystem type %q", fs.fsType.Name()), err) } } fs.DecRef(ctx) diff --git a/runsc/boot/loader.go b/runsc/boot/loader.go index f2fe33d23..74d9101a0 100644 --- a/runsc/boot/loader.go +++ b/runsc/boot/loader.go @@ -662,9 +662,7 @@ func New(args Args) (*Loader, error) { enableAutosave(l, args.Conf.TestOnlyAutosaveResume, l.saveFDs) } - if err := l.kernelInitExtra(); err != nil { - return nil, err - } + l.kernelInitExtra() // Create the control server using the provided FD. // diff --git a/runsc/boot/restore.go b/runsc/boot/restore.go index 2c77690de..8e0d3da2c 100644 --- a/runsc/boot/restore.go +++ b/runsc/boot/restore.go @@ -546,7 +546,7 @@ func (r *restorer) restore(l *Loader) error { err = loadOpts.Load(ctx, l.k, nil, oldInetStack, time.NewCalibratedClocks(), &vfs.CompleteRestoreOptions{}, l.saveRestoreNet) r.pagesFile = nil // transferred to loadOpts.Load() if err != nil { - return err + return fmt.Errorf("failed to load kernel: %w", err) } checkpointVersion := popVersionFromCheckpoint(l.k) @@ -557,10 +557,10 @@ func (r *restorer) restore(l *Loader) error { oldSpecs, err := popContainerSpecsFromCheckpoint(l.k) if err != nil { - return err + return fmt.Errorf("failed to pop container specs from checkpoint: %w", err) } if err := validateSpecs(oldSpecs, l.containerSpecs); err != nil { - return err + return fmt.Errorf("failed to validate restore spec: %w", err) } // Since we have a new kernel we also must make a new watchdog. @@ -613,9 +613,7 @@ func (r *restorer) restore(l *Loader) error { l.k.RestoreContainerMapping(l.containerIDs) - if err := l.kernelInitExtra(); err != nil { - return err - } + l.kernelInitExtra() // Refresh the control server with the newly created kernel. l.ctrl.refreshHandlers() @@ -625,7 +623,7 @@ func (r *restorer) restore(l *Loader) error { // r.restoreDone() signals and waits for the sandbox to start. if err := r.restoreDone(); err != nil { - return err + return fmt.Errorf("restorer.restoreDone callback failed: %w", err) } r.stateFile.Close() diff --git a/runsc/boot/restore_impl.go b/runsc/boot/restore_impl.go index 5be050ecb..b49049ea8 100644 --- a/runsc/boot/restore_impl.go +++ b/runsc/boot/restore_impl.go @@ -41,6 +41,4 @@ func newProcInternalData(*specs.Spec) *proc.InternalData { return &proc.InternalData{} } -func (l *Loader) kernelInitExtra() error { - return nil -} +func (l *Loader) kernelInitExtra() {}