From 239be78fbbf14493c6b7657b8a94f98aca08da06 Mon Sep 17 00:00:00 2001 From: Ayush Ranjan Date: Thu, 19 Jan 2023 10:35:57 -0800 Subject: [PATCH] Make gofer.dentry a generic type. This introduces a new gofer.dentry.impl field which can hold dentry implementation specific details. For now there exists only one implementation, which is a lisafs dentry. This work is in preparation for adding a direct host dentry implementation, which will make host syscalls instead of making RPCs. This change should have no change in behavior or performance. PiperOrigin-RevId: 503203994 --- pkg/lisafs/client_file.go | 8 +- pkg/sentry/fsimpl/gofer/BUILD | 2 + pkg/sentry/fsimpl/gofer/dentry_impl.go | 424 ++++++++++++++++ pkg/sentry/fsimpl/gofer/directory.go | 108 +---- pkg/sentry/fsimpl/gofer/filesystem.go | 351 +++++--------- pkg/sentry/fsimpl/gofer/gofer.go | 585 ++++++++--------------- pkg/sentry/fsimpl/gofer/gofer_test.go | 4 +- pkg/sentry/fsimpl/gofer/handle.go | 66 +-- pkg/sentry/fsimpl/gofer/lisafs_dentry.go | 510 ++++++++++++++++++++ pkg/sentry/fsimpl/gofer/regular_file.go | 38 +- pkg/sentry/fsimpl/gofer/revalidate.go | 50 +- pkg/sentry/fsimpl/gofer/save_restore.go | 89 +--- pkg/sentry/fsimpl/gofer/socket.go | 5 +- pkg/sentry/fsimpl/gofer/special_file.go | 30 +- pkg/sentry/fsimpl/gofer/symlink.go | 2 +- 15 files changed, 1404 insertions(+), 868 deletions(-) create mode 100644 pkg/sentry/fsimpl/gofer/dentry_impl.go create mode 100644 pkg/sentry/fsimpl/gofer/lisafs_dentry.go diff --git a/pkg/lisafs/client_file.go b/pkg/lisafs/client_file.go index 1de9c4cb3..fe4dd3416 100644 --- a/pkg/lisafs/client_file.go +++ b/pkg/lisafs/client_file.go @@ -304,7 +304,13 @@ func (f *ClientFD) SetStat(ctx context.Context, stat *linux.Statx) (uint32, erro ctx.UninterruptibleSleepStart(false) err := f.client.SndRcvMessage(SetStat, uint32(req.SizeBytes()), req.MarshalUnsafe, resp.CheckedUnmarshal, nil, req.String, resp.String) ctx.UninterruptibleSleepFinish(false) - return resp.FailureMask, unix.Errno(resp.FailureErrNo), err + if err != nil { + return 0, nil, err + } + if resp.FailureMask == 0 { + return 0, nil, nil + } + return resp.FailureMask, unix.Errno(resp.FailureErrNo), nil } // WalkMultiple makes the Walk RPC with multiple path components. diff --git a/pkg/sentry/fsimpl/gofer/BUILD b/pkg/sentry/fsimpl/gofer/BUILD index 4b9c64c85..4d6b0fb64 100644 --- a/pkg/sentry/fsimpl/gofer/BUILD +++ b/pkg/sentry/fsimpl/gofer/BUILD @@ -53,6 +53,7 @@ go_template_instance( go_library( name = "gofer", srcs = [ + "dentry_impl.go", "dentry_list.go", "directory.go", "filesystem.go", @@ -60,6 +61,7 @@ go_library( "gofer.go", "handle.go", "host_named_pipe.go", + "lisafs_dentry.go", "regular_file.go", "revalidate.go", "save_restore.go", diff --git a/pkg/sentry/fsimpl/gofer/dentry_impl.go b/pkg/sentry/fsimpl/gofer/dentry_impl.go new file mode 100644 index 000000000..3de18e986 --- /dev/null +++ b/pkg/sentry/fsimpl/gofer/dentry_impl.go @@ -0,0 +1,424 @@ +// Copyright 2022 The gVisor Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package gofer + +import ( + "golang.org/x/sys/unix" + "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/context" + "gvisor.dev/gvisor/pkg/log" + "gvisor.dev/gvisor/pkg/sentry/kernel/auth" + "gvisor.dev/gvisor/pkg/sentry/vfs" +) + +// We do *not* define an interface for dentry.impl because making interface +// method calls is almost 2.5x slower than calling the same method on a +// concrete type. Instead, we use type assertions in switch statements. The +// asserted type is a concrete dentry implementation and methods are called +// directly on the concrete type. This helps in the following ways: +// +// 1. This is faster because concrete type assertion just needs to compare the +// itab pointer in the interface value to a constant which is relatively +// cheap. Benchmarking showed that such type switches don't add almost any +// overhead. +// 2. Passing any pointer to an interface method immediately causes the pointed +// object to escape to heap. Making concrete method calls allows escape +// analysis to proceed as usual and avoids heap allocations. +// +// Also note that the default case in these type switch statements panics. We +// do not do panic(fmt.Sprintf("... %T", d.impl)) because somehow it adds a lot +// of overhead to the type switch. So instead we panic with a constant string. + +// Precondition: d.handleMu must be locked. +func (d *dentry) isReadHandleOk() bool { + switch dt := d.impl.(type) { + case *lisafsDentry: + return dt.readFDLisa.Ok() + case nil: // synthetic dentry + return false + default: + panic("unknown dentry implementation") + } +} + +// Precondition: d.handleMu must be locked. +func (d *dentry) isWriteHandleOk() bool { + switch dt := d.impl.(type) { + case *lisafsDentry: + return dt.writeFDLisa.Ok() + case nil: // synthetic dentry + return false + default: + panic("unknown dentry implementation") + } +} + +// Precondition: d.handleMu must be locked. +func (d *dentry) readHandle() handle { + switch dt := d.impl.(type) { + case *lisafsDentry: + return handle{ + fdLisa: dt.readFDLisa, + fd: d.readFD.RacyLoad(), + } + case nil: // synthetic dentry + return noHandle + default: + panic("unknown dentry implementation") + } +} + +// Precondition: d.handleMu must be locked. +func (d *dentry) writeHandle() handle { + switch dt := d.impl.(type) { + case *lisafsDentry: + return handle{ + fdLisa: dt.writeFDLisa, + fd: d.writeFD.RacyLoad(), + } + case nil: // synthetic dentry + return noHandle + default: + panic("unknown dentry implementation") + } +} + +// Precondition: !d.isSynthetic(). +func (d *dentry) openHandle(ctx context.Context, read, write, trunc bool) (handle, error) { + flags := uint32(unix.O_RDONLY) + switch { + case read && write: + flags = unix.O_RDWR + case read: + flags = unix.O_RDONLY + case write: + flags = unix.O_WRONLY + default: + log.Debugf("openHandle called with read = write = false. Falling back to read only FD.") + } + if trunc { + flags |= unix.O_TRUNC + } + switch dt := d.impl.(type) { + case *lisafsDentry: + openFD, hostFD, err := dt.controlFD.OpenAt(ctx, flags) + if err != nil { + return noHandle, err + } + return handle{ + fdLisa: dt.controlFD.Client().NewFD(openFD), + fd: int32(hostFD), + }, nil + default: + panic("unknown dentry implementation") + } +} + +// Preconditions: +// - d.handleMu must be locked. +// - !d.isSynthetic(). +func (d *dentry) updateHandles(ctx context.Context, h handle, readable, writable bool) { + switch dt := d.impl.(type) { + case *lisafsDentry: + dt.updateHandles(ctx, h, readable, writable) + default: + panic("unknown dentry implementation") + } +} + +// updateMetadataLocked updates the dentry's metadata fields. The h parameter +// is optional. If it is not provided, an appropriate FD should be chosen to +// stat the remote file. +// +// Preconditions: +// - !d.isSynthetic(). +// - d.metadataMu is locked. +// +// +checklocks:d.metadataMu +func (d *dentry) updateMetadataLocked(ctx context.Context, h handle) error { + switch dt := d.impl.(type) { + case *lisafsDentry: + return dt.updateMetadataLocked(ctx, h) // +checklocksforce: acquired by precondition. + default: + panic("unknown dentry implementation") + } +} + +func (d *dentry) chmod(ctx context.Context, mode uint16) error { + switch dt := d.impl.(type) { + case *lisafsDentry: + return chmod(ctx, dt.controlFD, mode) + default: + panic("unknown dentry implementation") + } +} + +// Preconditions: +// - !d.isSynthetic(). +// - d.handleMu is locked. +func (d *dentry) setStatLocked(ctx context.Context, stat *linux.Statx) (uint32, error, error) { + switch dt := d.impl.(type) { + case *lisafsDentry: + return dt.controlFD.SetStat(ctx, stat) + default: + panic("unknown dentry implementation") + } +} + +func (d *dentry) destroyImpl(ctx context.Context) { + switch dt := d.impl.(type) { + case *lisafsDentry: + dt.destroy(ctx) + case nil: // synthetic dentry + default: + panic("unknown dentry implementation") + } +} + +// Postcondition: Caller must do dentry caching appropriately. +func (d *dentry) getRemoteChild(ctx context.Context, name string) (*dentry, error) { + switch dt := d.impl.(type) { + case *lisafsDentry: + return dt.getRemoteChild(ctx, name) + default: + panic("unknown dentry implementation") + } +} + +// Preconditions: +// - fs.renameMu must be locked. +// - parent.dirMu must be locked. +// - parent.isDir(). +// - name is not "." or "..". +// - dentry at name must not already exist in dentry tree. +// +// Postcondition: The returned dentry is already cached appropriately. +func (d *dentry) getRemoteChildAndWalkPathLocked(ctx context.Context, rp *vfs.ResolvingPath, ds **[]*dentry) (*dentry, error) { + switch dt := d.impl.(type) { + case *lisafsDentry: + return dt.getRemoteChildAndWalkPathLocked(ctx, rp, ds) + // TODO(b/258687694): For directfs, remember to use fs.getRemoteChildLocked + // so that dentry caching is done properly. + default: + panic("unknown dentry implementation") + } +} + +// Precondition: !d.isSynthetic(). +func (d *dentry) listXattrImpl(ctx context.Context, size uint64) ([]string, error) { + switch dt := d.impl.(type) { + case *lisafsDentry: + return dt.controlFD.ListXattr(ctx, size) + default: + panic("unknown dentry implementation") + } +} + +// Precondition: !d.isSynthetic(). +func (d *dentry) getXattrImpl(ctx context.Context, opts *vfs.GetXattrOptions) (string, error) { + switch dt := d.impl.(type) { + case *lisafsDentry: + return dt.controlFD.GetXattr(ctx, opts.Name, opts.Size) + default: + panic("unknown dentry implementation") + } +} + +// Precondition: !d.isSynthetic(). +func (d *dentry) setXattrImpl(ctx context.Context, opts *vfs.SetXattrOptions) error { + switch dt := d.impl.(type) { + case *lisafsDentry: + return dt.controlFD.SetXattr(ctx, opts.Name, opts.Value, opts.Flags) + default: + panic("unknown dentry implementation") + } +} + +// Precondition: !d.isSynthetic(). +func (d *dentry) removeXattrImpl(ctx context.Context, name string) error { + switch dt := d.impl.(type) { + case *lisafsDentry: + return dt.controlFD.RemoveXattr(ctx, name) + default: + panic("unknown dentry implementation") + } +} + +// Precondition: !d.isSynthetic(). +func (d *dentry) mknod(ctx context.Context, name string, creds *auth.Credentials, opts *vfs.MknodOptions) (*dentry, error) { + switch dt := d.impl.(type) { + case *lisafsDentry: + return dt.mknod(ctx, name, creds, opts) + default: + panic("unknown dentry implementation") + } +} + +// Precondition: !d.isSynthetic(). +func (d *dentry) link(ctx context.Context, target *dentry, name string) (*dentry, error) { + switch dt := d.impl.(type) { + case *lisafsDentry: + return dt.link(ctx, target.impl.(*lisafsDentry), name) + default: + panic("unknown dentry implementation") + } +} + +// Precondition: !d.isSynthetic(). +func (d *dentry) mkdir(ctx context.Context, name string, mode linux.FileMode, uid auth.KUID, gid auth.KGID) (*dentry, error) { + switch dt := d.impl.(type) { + case *lisafsDentry: + return dt.mkdir(ctx, name, mode, uid, gid) + default: + panic("unknown dentry implementation") + } +} + +// Precondition: !d.isSynthetic(). +func (d *dentry) symlink(ctx context.Context, name, target string, creds *auth.Credentials) (*dentry, error) { + switch dt := d.impl.(type) { + case *lisafsDentry: + return dt.symlink(ctx, name, target, creds) + default: + panic("unknown dentry implementation") + } +} + +// Precondition: !d.isSynthetic(). +func (d *dentry) openCreate(ctx context.Context, name string, accessFlags uint32, mode linux.FileMode, uid auth.KUID, gid auth.KGID) (*dentry, handle, error) { + switch dt := d.impl.(type) { + case *lisafsDentry: + return dt.openCreate(ctx, name, accessFlags, mode, uid, gid) + default: + panic("unknown dentry implementation") + } +} + +// Preconditions: +// - d.isDir(). +// - d.handleMu must be locked. +// - !d.isSynthetic(). +func (d *dentry) getDirentsLocked(ctx context.Context, count int, recordDirent func(name string, key inoKey, dType uint8)) error { + switch dt := d.impl.(type) { + case *lisafsDentry: + return dt.getDirentsLocked(ctx, count, recordDirent) + default: + panic("unknown dentry implementation") + } +} + +// Precondition: !d.isSynthetic(). +func (d *dentry) flush(ctx context.Context) error { + d.handleMu.RLock() + defer d.handleMu.RUnlock() + switch dt := d.impl.(type) { + case *lisafsDentry: + return flush(ctx, dt.writeFDLisa) + default: + panic("unknown dentry implementation") + } +} + +// Precondition: !d.isSynthetic(). +func (d *dentry) allocate(ctx context.Context, mode, offset, length uint64) error { + d.handleMu.RLock() + defer d.handleMu.RUnlock() + switch dt := d.impl.(type) { + case *lisafsDentry: + return dt.writeFDLisa.Allocate(ctx, mode, offset, length) + default: + panic("unknown dentry implementation") + } +} + +// Precondition: !d.isSynthetic(). +func (d *dentry) connect(ctx context.Context, sockType linux.SockType) (int, error) { + switch dt := d.impl.(type) { + case *lisafsDentry: + return dt.controlFD.Connect(ctx, sockType) + default: + panic("unknown dentry implementation") + } +} + +// Precondition: !d.isSynthetic(). +func (d *dentry) readlinkImpl(ctx context.Context) (string, error) { + switch dt := d.impl.(type) { + case *lisafsDentry: + return dt.controlFD.ReadLinkAt(ctx) + default: + panic("unknown dentry implementation") + } +} + +// Precondition: !d.isSynthetic(). +func (d *dentry) unlink(ctx context.Context, name string, flags uint32) error { + switch dt := d.impl.(type) { + case *lisafsDentry: + return dt.controlFD.UnlinkAt(ctx, name, flags) + default: + panic("unknown dentry implementation") + } +} + +// Precondition: !d.isSynthetic(). +func (d *dentry) rename(ctx context.Context, oldName string, newParent *dentry, newName string) error { + switch dt := d.impl.(type) { + case *lisafsDentry: + return dt.controlFD.RenameAt(ctx, oldName, newParent.impl.(*lisafsDentry).controlFD.ID(), newName) + default: + panic("unknown dentry implementation") + } +} + +// Precondition: !d.isSynthetic(). +func (d *dentry) statfs(ctx context.Context) (linux.Statfs, error) { + switch dt := d.impl.(type) { + case *lisafsDentry: + return dt.statfs(ctx) + default: + panic("unknown dentry implementation") + } +} + +func (fs *filesystem) restoreRoot(ctx context.Context, opts *vfs.CompleteRestoreOptions) error { + // The root is always non-synthetic. + switch dt := fs.root.impl.(type) { + case *lisafsDentry: + rootInode, err := fs.initClient(ctx) + if err != nil { + return err + } + return dt.restoreFile(ctx, &rootInode, opts) + default: + panic("unknown dentry implementation") + } +} + +// Preconditions: +// - !d.isSynthetic(). +// - d.parent != nil and has been restored. +func (d *dentry) restoreFile(ctx context.Context, opts *vfs.CompleteRestoreOptions) error { + switch dt := d.impl.(type) { + case *lisafsDentry: + inode, err := d.parent.impl.(*lisafsDentry).controlFD.Walk(ctx, d.name) + if err != nil { + return err + } + return dt.restoreFile(ctx, &inode, opts) + default: + panic("unknown dentry implementation") + } +} diff --git a/pkg/sentry/fsimpl/gofer/directory.go b/pkg/sentry/fsimpl/gofer/directory.go index 38dd836f2..7c87d3d72 100644 --- a/pkg/sentry/fsimpl/gofer/directory.go +++ b/pkg/sentry/fsimpl/gofer/directory.go @@ -22,9 +22,6 @@ import ( "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/errors/linuxerr" "gvisor.dev/gvisor/pkg/hostarch" - "gvisor.dev/gvisor/pkg/lisafs" - "gvisor.dev/gvisor/pkg/log" - "gvisor.dev/gvisor/pkg/refs" "gvisor.dev/gvisor/pkg/sentry/kernel/auth" "gvisor.dev/gvisor/pkg/sentry/kernel/pipe" "gvisor.dev/gvisor/pkg/sentry/socket/unix/transport" @@ -36,28 +33,6 @@ func (d *dentry) isDir() bool { return d.fileType() == linux.S_IFDIR } -// Preconditions: -// - filesystem.renameMu must be locked. -// - d.dirMu must be locked. -// - d.isDir(). -// - child must be a newly-created dentry that has never had a parent. -func (d *dentry) insertCreatedChildLocked(ctx context.Context, childIno *lisafs.Inode, childName string, updateChild func(child *dentry), ds **[]*dentry) error { - child, err := d.fs.newDentry(ctx, childIno) - if err != nil { - if err := d.controlFDLisa.UnlinkAt(ctx, childName, 0 /* flags */); err != nil { - log.Warningf("failed to clean up created child %s after newDentry() failed: %v", childName, err) - } - d.fs.client.CloseFD(ctx, childIno.ControlFD, false /* flush */) - return err - } - d.cacheNewChildLocked(child, childName) - appendNewChildDentry(ds, d, child) - if updateChild != nil { - updateChild(child) - } - return nil -} - // Preconditions: // - filesystem.renameMu must be locked. // - d.dirMu must be locked. @@ -129,19 +104,13 @@ type createSyntheticOpts struct { pipe *pipe.VFSPipe } -// createSyntheticChildLocked creates a synthetic file with the given name -// in d. -// -// Preconditions: -// - d.dirMu must be locked. -// - d.isDir(). -// - d does not already contain a child with the given name. -func (d *dentry) createSyntheticChildLocked(opts *createSyntheticOpts) { - now := d.fs.clock.Now().Nanoseconds() +// newSyntheticDentry creates a synthetic file with the given name. +func (fs *filesystem) newSyntheticDentry(opts *createSyntheticOpts) *dentry { + now := fs.clock.Now().Nanoseconds() child := &dentry{ - refs: atomicbitops.FromInt64(1), // held by d - fs: d.fs, - ino: d.fs.nextIno(), + refs: atomicbitops.FromInt64(1), // held by parent. + fs: fs, + ino: fs.nextIno(), mode: atomicbitops.FromUint32(uint32(opts.mode)), uid: atomicbitops.FromUint32(uint32(opts.kuid)), gid: atomicbitops.FromUint32(uint32(opts.kgid)), @@ -155,7 +124,6 @@ func (d *dentry) createSyntheticChildLocked(opts *createSyntheticOpts) { mmapFD: atomicbitops.FromInt32(-1), nlink: atomicbitops.FromUint32(2), } - refs.Register(child) switch opts.mode.FileType() { case linux.S_IFDIR: // Nothing else needs to be done. @@ -166,13 +134,8 @@ func (d *dentry) createSyntheticChildLocked(opts *createSyntheticOpts) { default: panic(fmt.Sprintf("failed to create synthetic file of unrecognized type: %v", opts.mode.FileType())) } - child.pf.dentry = child - child.cacheEntry.d = child - child.syncableListEntry.d = child - child.vfsd.Init(child) - - d.cacheNewChildLocked(child, opts.name) - d.syntheticChildren++ + child.init(nil /* impl */) + return child } // Preconditions: @@ -274,53 +237,28 @@ func (d *dentry) getDirents(ctx context.Context) ([]vfs.Dirent, error) { // duplicate entries for synthetic children. realChildren = make(map[string]struct{}) } - const count = 64 * 1024 // for consistency with the vfs1 client d.handleMu.RLock() - if !d.isReadFileOk() { + if !d.isReadHandleOk() { // This should not be possible because a readable handle should // have been opened when the calling directoryFD was opened. - d.handleMu.RUnlock() panic("gofer.dentry.getDirents called without a readable handle") } - // shouldSeek0 indicates whether the server should SEEK to 0 before reading - // directory entries. - shouldSeek0 := true - for { - countLisa := int32(count) - if shouldSeek0 { - // See lisafs.Getdents64Req.Count. - countLisa = -countLisa - shouldSeek0 = false + const count = 64 * 1024 // for consistency with the vfs1 client + err := d.getDirentsLocked(ctx, count, func(name string, key inoKey, dType uint8) { + dirent := vfs.Dirent{ + Name: name, + Ino: d.fs.inoFromKey(key), + NextOff: int64(len(dirents) + 1), + Type: dType, } - direntsLisa, err := d.readFDLisa.Getdents64(ctx, countLisa) - if err != nil { - d.handleMu.RUnlock() - return nil, err - } - if len(direntsLisa) == 0 { - d.handleMu.RUnlock() - break - } - for i := range direntsLisa { - name := string(direntsLisa[i].Name) - if name == "." || name == ".." { - continue - } - dirent := vfs.Dirent{ - Name: name, - Ino: d.fs.inoFromKey(inoKey{ - ino: uint64(direntsLisa[i].Ino), - devMinor: uint32(direntsLisa[i].DevMinor), - devMajor: uint32(direntsLisa[i].DevMajor), - }), - NextOff: int64(len(dirents) + 1), - Type: uint8(direntsLisa[i].Type), - } - dirents = append(dirents, dirent) - if realChildren != nil { - realChildren[name] = struct{}{} - } + dirents = append(dirents, dirent) + if realChildren != nil { + realChildren[name] = struct{}{} } + }) + d.handleMu.RUnlock() + if err != nil { + return nil, err } } // Emit entries for synthetic children. diff --git a/pkg/sentry/fsimpl/gofer/filesystem.go b/pkg/sentry/fsimpl/gofer/filesystem.go index fff1398f4..a156d6c1c 100644 --- a/pkg/sentry/fsimpl/gofer/filesystem.go +++ b/pkg/sentry/fsimpl/gofer/filesystem.go @@ -26,7 +26,6 @@ import ( "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/errors/linuxerr" "gvisor.dev/gvisor/pkg/fspath" - "gvisor.dev/gvisor/pkg/lisafs" "gvisor.dev/gvisor/pkg/sentry/fsimpl/host" "gvisor.dev/gvisor/pkg/sentry/fsmetric" "gvisor.dev/gvisor/pkg/sentry/kernel" @@ -224,105 +223,6 @@ func (fs *filesystem) stepLocked(ctx context.Context, rp *vfs.ResolvingPath, d * return child, false, nil } -// Preconditions: -// - fs.renameMu must be locked. -// - parent.dirMu must be locked. -// - parent.isDir(). -// - parent and the dentry at name have been revalidated. -func (fs *filesystem) getChildAndWalkPathLocked(ctx context.Context, parent *dentry, rp *vfs.ResolvingPath, ds **[]*dentry) (*dentry, error) { - // Note that pit is a copy of the iterator that does not affect rp. - pit := rp.Pit() - first := pit.String() - if len(first) > MaxFilenameLen { - return nil, linuxerr.ENAMETOOLONG - } - if child, ok := parent.children[first]; ok || parent.isSynthetic() { - if child == nil { - return nil, linuxerr.ENOENT - } - return child, nil - } - - if parent.childrenSet != nil { - // Is the first child even there? Don't make RPC if not. - if _, ok := parent.childrenSet[first]; !ok { - return nil, linuxerr.ENOENT - } - } - - // Walk as much of the path as possible in 1 RPC. - names := []string{first} - for pit = pit.Next(); pit.Ok(); pit = pit.Next() { - name := pit.String() - if name == "." { - continue - } - if name == ".." { - break - } - names = append(names, name) - } - status, inodes, err := parent.controlFDLisa.WalkMultiple(ctx, names) - if err != nil { - return nil, err - } - if len(inodes) == 0 { - parent.cacheNegativeLookupLocked(first) - return nil, linuxerr.ENOENT - } - - // Add the walked inodes into the dentry tree. - curParent := parent - curParentDirMuLock := func() { - if curParent != parent { - curParent.dirMu.Lock() - } - } - curParentDirMuUnlock := func() { - if curParent != parent { - curParent.dirMu.Unlock() // +checklocksforce: locked via curParentDirMuLock(). - } - } - var ret *dentry - var dentryCreationErr error - for i := range inodes { - if dentryCreationErr != nil { - fs.client.CloseFD(ctx, inodes[i].ControlFD, false /* flush */) - continue - } - - child, err := fs.newDentry(ctx, &inodes[i]) - if err != nil { - fs.client.CloseFD(ctx, inodes[i].ControlFD, false /* flush */) - dentryCreationErr = err - continue - } - curParentDirMuLock() - curParent.cacheNewChildLocked(child, names[i]) - curParentDirMuUnlock() - // For now, child has 0 references, so our caller should call - // child.checkCachingLocked(). curParent gained a ref so we should also - // call curParent.checkCachingLocked() so it can be removed from the cache - // if needed. We only do that for the first iteration because all - // subsequent parents would have already been added to ds. - if i == 0 { - *ds = appendDentry(*ds, curParent) - } - *ds = appendDentry(*ds, child) - curParent = child - if i == 0 { - ret = child - } - } - - if status == lisafs.WalkComponentDoesNotExist && curParent.isDir() { - curParentDirMuLock() - curParent.cacheNegativeLookupLocked(names[len(inodes)]) - curParentDirMuUnlock() - } - return ret, dentryCreationErr -} - // getChildLocked returns a dentry representing the child of parent with the // given name. Returns ENOENT if the child doesn't exist. // @@ -333,41 +233,67 @@ func (fs *filesystem) getChildAndWalkPathLocked(ctx context.Context, parent *den // - name is not "." or "..". // - parent and the dentry at name have been revalidated. func (fs *filesystem) getChildLocked(ctx context.Context, parent *dentry, name string, ds **[]*dentry) (*dentry, error) { - if len(name) > MaxFilenameLen { - return nil, linuxerr.ENAMETOOLONG - } - if child, ok := parent.children[name]; ok || parent.isSynthetic() { - if child == nil { - return nil, linuxerr.ENOENT - } - return child, nil + if child, err := parent.getCachedChildLocked(name); child != nil || err != nil { + return child, err } + return fs.getRemoteChildLocked(ctx, parent, name, ds) +} - if parent.childrenSet != nil { - // Is the child even there? Don't make RPC if not. - if _, ok := parent.childrenSet[name]; !ok { - return nil, linuxerr.ENOENT - } - } - - childInode, err := parent.controlFDLisa.Walk(ctx, name) +// getRemoteChildLocked is similar to getChildLocked, with the additional +// precondition that the child identified by name does not exist in cache. +func (fs *filesystem) getRemoteChildLocked(ctx context.Context, parent *dentry, name string, ds **[]*dentry) (*dentry, error) { + child, err := parent.getRemoteChild(ctx, name) + // Cache the result appropriately in the dentry tree. if err != nil { if linuxerr.Equals(linuxerr.ENOENT, err) { parent.cacheNegativeLookupLocked(name) } return nil, err } - // Create a new dentry representing the file. - child, err := fs.newDentry(ctx, &childInode) - if err != nil { - fs.client.CloseFD(ctx, childInode.ControlFD, false /* flush */) - return nil, err - } parent.cacheNewChildLocked(child, name) appendNewChildDentry(ds, parent, child) return child, nil } +// getChildAndWalkPathLocked is the same as getChildLocked, except that it +// may prefetch the entire path represented by rp. +func (fs *filesystem) getChildAndWalkPathLocked(ctx context.Context, parent *dentry, rp *vfs.ResolvingPath, ds **[]*dentry) (*dentry, error) { + if child, err := parent.getCachedChildLocked(rp.Component()); child != nil || err != nil { + return child, err + } + // dentry.getRemoteChildAndWalkPathLocked already handles dentry caching. + return parent.getRemoteChildAndWalkPathLocked(ctx, rp, ds) +} + +// getCachedChildLocked returns a child dentry if it was cached earlier. If no +// cached child dentry exists, (nil, nil) is returned. +// +// Preconditions: +// - fs.renameMu must be locked. +// - d.dirMu must be locked. +// - d.isDir(). +// - name is not "." or "..". +// - d and the dentry at name have been revalidated. +func (d *dentry) getCachedChildLocked(name string) (*dentry, error) { + if len(name) > MaxFilenameLen { + return nil, linuxerr.ENAMETOOLONG + } + if child, ok := d.children[name]; ok || d.isSynthetic() { + if child == nil { + return nil, linuxerr.ENOENT + } + return child, nil + } + + if d.childrenSet != nil { + // Is the child even there? Don't make RPC if not. + if _, ok := d.childrenSet[name]; !ok { + return nil, linuxerr.ENOENT + } + } + return nil, nil +} + // walkParentDirLocked resolves all but the last path component of rp to an // existing directory, starting from the given directory (which is usually // rp.Start().Impl().(*dentry)). It does not check that the returned directory @@ -437,7 +363,7 @@ func (fs *filesystem) resolveLocked(ctx context.Context, rp *vfs.ResolvingPath, // Preconditions: // - !rp.Done(). // - For the final path component in rp, !rp.ShouldFollowSymlink(). -func (fs *filesystem) doCreateAt(ctx context.Context, rp *vfs.ResolvingPath, dir bool, createInRemoteDir func(parent *dentry, name string, ds **[]*dentry) error, createInSyntheticDir func(parent *dentry, name string) error) error { +func (fs *filesystem) doCreateAt(ctx context.Context, rp *vfs.ResolvingPath, dir bool, createInRemoteDir func(parent *dentry, name string, ds **[]*dentry) (*dentry, error), createInSyntheticDir func(parent *dentry, name string) (*dentry, error)) error { var ds *[]*dentry fs.renameMu.RLock() defer fs.renameMuRUnlockAndCheckCaching(ctx, &ds) @@ -514,9 +440,12 @@ func (fs *filesystem) doCreateAt(ctx context.Context, rp *vfs.ResolvingPath, dir if createInSyntheticDir == nil { return linuxerr.EPERM } - if err := createInSyntheticDir(parent, name); err != nil { + child, err := createInSyntheticDir(parent, name) + if err != nil { return err } + parent.cacheNewChildLocked(child, name) + parent.syntheticChildren++ parent.touchCMtime() parent.clearDirentsLocked() ev := linux.IN_CREATE @@ -529,9 +458,17 @@ func (fs *filesystem) doCreateAt(ctx context.Context, rp *vfs.ResolvingPath, dir // No cached dentry exists; however, in InteropModeShared there might still be // an existing file at name. Just attempt the file creation RPC anyways. If a // file does exist, the RPC will fail with EEXIST like we would have. - if err := createInRemoteDir(parent, name, &ds); err != nil { + child, err := createInRemoteDir(parent, name, &ds) + if err != nil { return err } + parent.cacheNewChildLocked(child, name) + if child.isSynthetic() { + parent.syntheticChildren++ + ds = appendDentry(ds, parent) + } else { + appendNewChildDentry(&ds, parent, child) + } if fs.opts.interop != InteropModeShared { if child, ok := parent.children[name]; ok && child == nil { // Delete the now-stale negative dentry. @@ -686,7 +623,7 @@ func (fs *filesystem) unlinkAt(ctx context.Context, rp *vfs.ResolvingPath, dir b return linuxerr.ENOENT } } else if child == nil || !child.isSynthetic() { - if err := parent.controlFDLisa.UnlinkAt(ctx, name, flags); err != nil { + if err := parent.unlink(ctx, name, flags); err != nil { if child != nil { vfsObj.AbortDeleteDentry(&child.vfsd) // +checklocksforce: see above. } @@ -784,31 +721,27 @@ func (fs *filesystem) GetParentDentryAt(ctx context.Context, rp *vfs.ResolvingPa // LinkAt implements vfs.FilesystemImpl.LinkAt. func (fs *filesystem) LinkAt(ctx context.Context, rp *vfs.ResolvingPath, vd vfs.VirtualDentry) error { - err := fs.doCreateAt(ctx, rp, false /* dir */, func(parent *dentry, childName string, ds **[]*dentry) error { + err := fs.doCreateAt(ctx, rp, false /* dir */, func(parent *dentry, name string, ds **[]*dentry) (*dentry, error) { if rp.Mount() != vd.Mount() { - return linuxerr.EXDEV + return nil, linuxerr.EXDEV } d := vd.Dentry().Impl().(*dentry) if d.isDir() { - return linuxerr.EPERM + return nil, linuxerr.EPERM } gid := auth.KGID(d.gid.Load()) uid := auth.KUID(d.uid.Load()) mode := linux.FileMode(d.mode.Load()) if err := vfs.MayLink(rp.Credentials(), mode, uid, gid); err != nil { - return err + return nil, err } if d.nlink.Load() == 0 { - return linuxerr.ENOENT + return nil, linuxerr.ENOENT } if d.nlink.Load() == math.MaxUint32 { - return linuxerr.EMLINK + return nil, linuxerr.EMLINK } - linkInode, err := parent.controlFDLisa.LinkAt(ctx, d.controlFDLisa.ID(), childName) - if err != nil { - return err - } - return parent.insertCreatedChildLocked(ctx, &linkInode, childName, nil, ds) + return parent.link(ctx, d, name) }, nil) if err == nil { @@ -821,7 +754,7 @@ func (fs *filesystem) LinkAt(ctx context.Context, rp *vfs.ResolvingPath, vd vfs. // MkdirAt implements vfs.FilesystemImpl.MkdirAt. func (fs *filesystem) MkdirAt(ctx context.Context, rp *vfs.ResolvingPath, opts vfs.MkdirOptions) error { creds := rp.Credentials() - return fs.doCreateAt(ctx, rp, true /* dir */, func(parent *dentry, name string, ds **[]*dentry) error { + return fs.doCreateAt(ctx, rp, true /* dir */, func(parent *dentry, name string, ds **[]*dentry) (*dentry, error) { // If the parent is a setgid directory, use the parent's GID // rather than the caller's and enable setgid. kgid := creds.EffectiveKGID @@ -830,56 +763,53 @@ func (fs *filesystem) MkdirAt(ctx context.Context, rp *vfs.ResolvingPath, opts v kgid = auth.KGID(parent.gid.Load()) mode |= linux.S_ISGID } - childDirInode, err := parent.controlFDLisa.MkdirAt(ctx, name, mode, lisafs.UID(creds.EffectiveKUID), lisafs.GID(kgid)) + + child, err := parent.mkdir(ctx, name, mode, creds.EffectiveKUID, kgid) if err == nil { - if err = parent.insertCreatedChildLocked(ctx, &childDirInode, name, nil, ds); err != nil { - return err - } if fs.opts.interop != InteropModeShared { parent.incLinks() } - return nil + return child, nil } if !opts.ForSyntheticMountpoint || linuxerr.Equals(linuxerr.EEXIST, err) { - return err + return nil, err } ctx.Infof("Failed to create remote directory %q: %v; falling back to synthetic directory", name, err) - parent.createSyntheticChildLocked(&createSyntheticOpts{ + child = fs.newSyntheticDentry(&createSyntheticOpts{ name: name, mode: linux.S_IFDIR | opts.Mode, kuid: creds.EffectiveKUID, kgid: creds.EffectiveKGID, }) - *ds = appendDentry(*ds, parent) if fs.opts.interop != InteropModeShared { parent.incLinks() } - return nil - }, func(parent *dentry, name string) error { + return child, nil + }, func(parent *dentry, name string) (*dentry, error) { if !opts.ForSyntheticMountpoint { // Can't create non-synthetic files in synthetic directories. - return linuxerr.EPERM + return nil, linuxerr.EPERM } - parent.createSyntheticChildLocked(&createSyntheticOpts{ + child := fs.newSyntheticDentry(&createSyntheticOpts{ name: name, mode: linux.S_IFDIR | opts.Mode, kuid: creds.EffectiveKUID, kgid: creds.EffectiveKGID, }) parent.incLinks() - return nil + return child, nil }) } // MknodAt implements vfs.FilesystemImpl.MknodAt. func (fs *filesystem) MknodAt(ctx context.Context, rp *vfs.ResolvingPath, opts vfs.MknodOptions) error { - return fs.doCreateAt(ctx, rp, false /* dir */, func(parent *dentry, name string, ds **[]*dentry) error { + return fs.doCreateAt(ctx, rp, false /* dir */, func(parent *dentry, name string, ds **[]*dentry) (*dentry, error) { creds := rp.Credentials() - if err := parent.mknodLocked(ctx, name, creds, opts, ds); err == nil { - return nil + if child, err := parent.mknod(ctx, name, creds, &opts); err == nil { + return child, nil } else if !linuxerr.Equals(linuxerr.EPERM, err) { - return err + return nil, err } // EPERM means that gofer does not allow creating a socket or pipe. Fallback @@ -890,36 +820,32 @@ func (fs *filesystem) MknodAt(ctx context.Context, rp *vfs.ResolvingPath, opts v switch { case err == nil: // Step succeeded, another file exists. - return linuxerr.EEXIST + return nil, linuxerr.EEXIST case !linuxerr.Equals(linuxerr.ENOENT, err): // Unexpected error. - return err + return nil, err } switch opts.Mode.FileType() { case linux.S_IFSOCK: - parent.createSyntheticChildLocked(&createSyntheticOpts{ + return fs.newSyntheticDentry(&createSyntheticOpts{ name: name, mode: opts.Mode, kuid: creds.EffectiveKUID, kgid: creds.EffectiveKGID, endpoint: opts.Endpoint, - }) - *ds = appendDentry(*ds, parent) - return nil + }), nil case linux.S_IFIFO: - parent.createSyntheticChildLocked(&createSyntheticOpts{ + return fs.newSyntheticDentry(&createSyntheticOpts{ name: name, mode: opts.Mode, kuid: creds.EffectiveKUID, kgid: creds.EffectiveKGID, pipe: pipe.NewVFSPipe(true /* isNamed */, pipe.DefaultPipeSize), - }) - *ds = appendDentry(*ds, parent) - return nil + }), nil } // Retain error from gofer if synthetic file cannot be created internally. - return linuxerr.EPERM + return nil, linuxerr.EPERM }, nil) } @@ -957,7 +883,7 @@ func (fs *filesystem) OpenAt(ctx context.Context, rp *vfs.ResolvingPath, opts vf } if !start.cachedMetadataAuthoritative() { // Refresh dentry's attributes before opening. - if err := start.updateMetadata(ctx, nil); err != nil { + if err := start.updateMetadata(ctx); err != nil { return nil, err } } @@ -1128,7 +1054,7 @@ func (d *dentry) openSocketByConnecting(ctx context.Context, opts *vfs.OpenOptio } // Note that special value of linux.SockType = 0 is interpreted by lisafs // as "do not care about the socket type". Analogous to p9.AnonymousSocket. - sockFD, err := d.controlFDLisa.Connect(ctx, 0 /* sockType */) + sockFD, err := d.connect(ctx, 0 /* sockType */) if err != nil { return nil, err } @@ -1158,7 +1084,7 @@ func (d *dentry) openSpecialFile(ctx context.Context, mnt *vfs.Mount, opts *vfs. // since closed its end. isBlockingOpenOfNamedPipe := d.fileType() == linux.S_IFIFO && opts.Flags&linux.O_NONBLOCK == 0 retry: - h, err := openHandle(ctx, d.controlFDLisa, ats.MayRead(), ats.MayWrite(), opts.Flags&linux.O_TRUNC != 0) + h, err := d.openHandle(ctx, ats.MayRead(), ats.MayWrite(), opts.Flags&linux.O_TRUNC != 0) if err != nil { if isBlockingOpenOfNamedPipe && ats == vfs.MayWrite && linuxerr.Equals(linuxerr.ENXIO, err) { // An attempt to open a named pipe with O_WRONLY|O_NONBLOCK fails @@ -1211,35 +1137,28 @@ func (d *dentry) createAndOpenChildLocked(ctx context.Context, rp *vfs.Resolving kgid = auth.KGID(d.gid.Load()) } - ino, openFD, hostFD, err := d.controlFDLisa.OpenCreateAt(ctx, name, opts.Flags&linux.O_ACCMODE, opts.Mode, lisafs.UID(creds.EffectiveKUID), lisafs.GID(kgid)) + child, h, err := d.openCreate(ctx, name, opts.Flags&linux.O_ACCMODE, opts.Mode, creds.EffectiveKUID, kgid) if err != nil { return nil, err } - child, err := d.fs.newDentry(ctx, &ino) - if err != nil { - d.fs.client.CloseFD(ctx, ino.ControlFD, false /* flush */) - d.fs.client.CloseFD(ctx, openFD, false /* flush */) - if hostFD >= 0 { - unix.Close(hostFD) - } - return nil, err - } // Incorporate the fid that was opened by lcreate. useRegularFileFD := child.fileType() == linux.S_IFREG && !d.fs.opts.regularFilesUseSpecialFileFD if useRegularFileFD { + var readable, writable bool child.handleMu.Lock() if vfs.MayReadFileWithOpenFlags(opts.Flags) { - child.readFDLisa = d.fs.client.NewFD(openFD) - if hostFD != -1 { - child.readFD = atomicbitops.FromInt32(int32(hostFD)) - child.mmapFD = atomicbitops.FromInt32(int32(hostFD)) + readable = true + if h.fd != -1 { + child.readFD = atomicbitops.FromInt32(h.fd) + child.mmapFD = atomicbitops.FromInt32(h.fd) } } if vfs.MayWriteFileWithOpenFlags(opts.Flags) { - child.writeFDLisa = d.fs.client.NewFD(openFD) - child.writeFD = atomicbitops.FromInt32(int32(hostFD)) + writable = true + child.writeFD = atomicbitops.FromInt32(h.fd) } + child.updateHandles(ctx, h, readable, writable) child.handleMu.Unlock() } // Insert the dentry into the tree. @@ -1259,10 +1178,6 @@ func (d *dentry) createAndOpenChildLocked(ctx context.Context, rp *vfs.Resolving } childVFSFD = &fd.vfsfd } else { - h := handle{ - fdLisa: d.fs.client.NewFD(openFD), - fd: int32(hostFD), - } fd, err := newSpecialFileFD(h, mnt, child, opts.Flags) if err != nil { h.close(ctx) @@ -1330,7 +1245,7 @@ func (fs *filesystem) RenameAt(ctx context.Context, rp *vfs.ResolvingPath, oldPa oldParent := oldParentVD.Dentry().Impl().(*dentry) if !oldParent.cachedMetadataAuthoritative() { - if err := oldParent.updateMetadata(ctx, nil); err != nil { + if err := oldParent.updateMetadata(ctx); err != nil { return err } } @@ -1418,7 +1333,7 @@ func (fs *filesystem) RenameAt(ctx context.Context, rp *vfs.ResolvingPath, oldPa // Update the remote filesystem. if !renamed.isSynthetic() { - if err := oldParent.controlFDLisa.RenameAt(ctx, oldName, newParent.controlFDLisa.ID(), newName); err != nil { + if err := oldParent.rename(ctx, oldName, newParent, newName); err != nil { vfsObj.AbortRenameDentry(&renamed.vfsd, replacedVFSD) return err } @@ -1429,7 +1344,7 @@ func (fs *filesystem) RenameAt(ctx context.Context, rp *vfs.ResolvingPath, oldPa if replaced.isDir() { flags = linux.AT_REMOVEDIR } - if err := newParent.controlFDLisa.UnlinkAt(ctx, newName, flags); err != nil { + if err := newParent.unlink(ctx, newName, flags); err != nil { vfsObj.AbortRenameDentry(&renamed.vfsd, replacedVFSD) return err } @@ -1536,45 +1451,37 @@ func (fs *filesystem) StatFSAt(ctx context.Context, rp *vfs.ResolvingPath) (linu for d.isSynthetic() { d = d.parent } - var statFS lisafs.StatFS - if err := d.controlFDLisa.StatFSTo(ctx, &statFS); err != nil { + statfs, err := d.statfs(ctx) + if err != nil { return linux.Statfs{}, err } - if statFS.NameLength == 0 || statFS.NameLength > MaxFilenameLen { - statFS.NameLength = MaxFilenameLen + if statfs.NameLength == 0 || statfs.NameLength > MaxFilenameLen { + statfs.NameLength = MaxFilenameLen } - return linux.Statfs{ - // This is primarily for distinguishing a gofer file system in - // tests. Testing is important, so instead of defining - // something completely random, use a standard value. - Type: linux.V9FS_MAGIC, - BlockSize: statFS.BlockSize, - FragmentSize: statFS.BlockSize, - Blocks: statFS.Blocks, - BlocksFree: statFS.BlocksFree, - BlocksAvailable: statFS.BlocksAvailable, - Files: statFS.Files, - FilesFree: statFS.FilesFree, - NameLength: statFS.NameLength, - }, nil + // This is primarily for distinguishing a gofer file system in + // tests. Testing is important, so instead of defining + // something completely random, use a standard value. + statfs.Type = linux.V9FS_MAGIC + return statfs, nil } // SymlinkAt implements vfs.FilesystemImpl.SymlinkAt. func (fs *filesystem) SymlinkAt(ctx context.Context, rp *vfs.ResolvingPath, target string) error { - return fs.doCreateAt(ctx, rp, false /* dir */, func(parent *dentry, name string, ds **[]*dentry) error { - creds := rp.Credentials() - symlinkInode, err := parent.controlFDLisa.SymlinkAt(ctx, name, target, lisafs.UID(creds.EffectiveKUID), lisafs.GID(creds.EffectiveKGID)) + return fs.doCreateAt(ctx, rp, false /* dir */, func(parent *dentry, name string, ds **[]*dentry) (*dentry, error) { + child, err := parent.symlink(ctx, name, target, rp.Credentials()) if err != nil { - return err + return nil, err } - return parent.insertCreatedChildLocked(ctx, &symlinkInode, name, func(child *dentry) { - if fs.opts.interop != InteropModeShared { - // Cache the symlink target on creation. In practice, this - // helps avoid a lot of ReadLink RPCs. - child.haveTarget = true - child.target = target - } - }, ds) + if parent.fs.opts.interop != InteropModeShared { + // Cache the symlink target on creation. In practice, this helps avoid a + // lot of ReadLink RPCs. Note that when InteropModeShared is in effect, + // we are forced to make Readlink RPCs. Because in this mode, we use host + // timestamps, not timestamps based on our internal clock. And readlink + // updates the atime on the host. + child.haveTarget = true + child.target = target + } + return child, nil }, nil) } diff --git a/pkg/sentry/fsimpl/gofer/gofer.go b/pkg/sentry/fsimpl/gofer/gofer.go index 3e130b098..650470873 100644 --- a/pkg/sentry/fsimpl/gofer/gofer.go +++ b/pkg/sentry/fsimpl/gofer/gofer.go @@ -506,6 +506,7 @@ func (fstype FilesystemType) GetFilesystem(ctx context.Context, vfsObj *vfs.Virt fs.vfsfs.Init(vfsObj, &fstype, fs) + // TODO(b/258687694): Handle directfs. if err := fs.initClientAndRoot(ctx); err != nil { fs.vfsfs.DecRef(ctx) return nil, nil, err @@ -519,9 +520,8 @@ func (fs *filesystem) initClientAndRoot(ctx context.Context) error { if err != nil { return err } - fs.root, err = fs.newDentry(ctx, &rootInode) + fs.root, err = fs.newLisafsDentry(ctx, &rootInode) if err != nil { - fs.client.CloseFD(ctx, rootInode.ControlFD, false /* flush */) return err } @@ -626,8 +626,9 @@ func (fs *filesystem) Release(ctx context.Context) { d := elem.d d.handleMu.Lock() d.dataMu.Lock() - if h := d.writeHandleLocked(); h.isOpen() { + if d.isWriteHandleOk() { // Write dirty cached data to the remote file. + h := d.writeHandle() if err := fsutil.SyncDirtyAll(ctx, &d.cache, &d.dirty, d.size.Load(), mf, h.writeFromBlocksAt); err != nil { log.Warningf("gofer.filesystem.Release: failed to flush dentry: %v", err) } @@ -715,7 +716,7 @@ type inoKey struct { devMajor uint32 } -func inoKeyFromStat(stat *linux.Statx) inoKey { +func inoKeyFromStatx(stat *linux.Statx) inoKey { return inoKey{ ino: stat.Ino, devMinor: stat.DevMinor, @@ -753,14 +754,6 @@ type dentry struct { // inoKey is used to identify this dentry's inode. inoKey inoKey - // controlFDLisa is used by lisafs to perform path based operations on this - // dentry. - // - // if !controlFD.Ok(), this dentry represents a synthetic file, i.e. a - // file that does not exist on the remote filesystem. As of this writing, the - // only files that can be synthetic are sockets, pipes, and directories. - controlFDLisa lisafs.ClientFD `state:"nosave"` - // If deleted is non-zero, the file represented by this dentry has been // deleted is accessed using atomic memory operations. deleted atomicbitops.Uint32 @@ -861,15 +854,12 @@ type dentry struct { // the file into memmap.MappingSpaces. mappings is protected by mapsMu. mappings memmap.MappingSet - // - If this dentry represents a regular file or directory, readFDLisa is - // a LISAFS FD used for reads by all regularFileFDs/directoryFDs - // representing this dentry, and readFD (if not -1) is a host FD - // equivalent to readFDLisa used as a faster alternative. + // - If this dentry represents a regular file or directory, readFD (if not + // -1) is a host FD used for reads by all regularFileFDs/directoryFDs + // representing this dentry. // - // - If this dentry represents a regular file, writeFDLisa is the LISAFS FD - // used for writes by all regularFileFDs representing this dentry, and - // writeFD (if not -1) is a host FD equivalent to writeFDLisa used as a - // faster alternative. + // - If this dentry represents a regular file, writeFD (if not -1) is a host + // FD used for writes by all regularFileFDs representing this dentry. // // - If this dentry represents a regular file, mmapFD is the host FD used // for memory mappings. If mmapFD is -1, no such FD is available, and the @@ -879,21 +869,17 @@ type dentry struct { // additionally written using atomic memory operations, allowing them to be // read (albeit racily) with atomic.LoadInt32() without locking handleMu. // - // readFDLisa and writeFDLisa may or may not represent the same LISAFS FD. - // Once either transitions from closed (Ok() == false) to open - // (Ok() == true), it may be mutated with handleMu locked, but cannot - // be closed until the dentry is destroyed. + // readFD and writeFD may or may not be the same file descriptor. Once either + // transitions from closed (-1) to open, it may be mutated with handleMu + // locked, but cannot be closed until the dentry is destroyed. // // readFD and writeFD may or may not be the same file descriptor. mmapFD is - // always either -1 or equal to readFD; if writeFDLisa.Ok() (the file has - // been opened for writing), it is additionally either -1 or equal to - // writeFD. - handleMu sync.RWMutex `state:"nosave"` - readFDLisa lisafs.ClientFD `state:"nosave"` - writeFDLisa lisafs.ClientFD `state:"nosave"` - readFD atomicbitops.Int32 `state:"nosave"` - writeFD atomicbitops.Int32 `state:"nosave"` - mmapFD atomicbitops.Int32 `state:"nosave"` + // always either -1 or equal to readFD; if the file has been opened for + // writing, it is additionally either -1 or equal to writeFD. + handleMu sync.RWMutex `state:"nosave"` + readFD atomicbitops.Int32 `state:"nosave"` + writeFD atomicbitops.Int32 `state:"nosave"` + mmapFD atomicbitops.Int32 `state:"nosave"` dataMu sync.RWMutex `state:"nosave"` @@ -935,6 +921,14 @@ type dentry struct { // same underlying file (see the gofer filesystem section fo vfs/inotify.md for // a more in-depth discussion on this matter). watches vfs.Watches + + // impl is the specific dentry implementation for non-synthetic dentries. + // impl is immutable. + // + // If impl is nil, this dentry represents a synthetic file, i.e. a + // file that does not exist on the host filesystem. As of this writing, the + // only files that can be synthetic are sockets, pipes, and directories. + impl any } // +stateify savable @@ -951,81 +945,6 @@ type dentryListElem struct { dentryEntry } -func (fs *filesystem) newDentry(ctx context.Context, ino *lisafs.Inode) (*dentry, error) { - if ino.Stat.Mask&linux.STATX_TYPE == 0 { - ctx.Warningf("can't create gofer.dentry without file type") - return nil, linuxerr.EIO - } - if ino.Stat.Mode&linux.FileTypeMask == linux.ModeRegular && ino.Stat.Mask&linux.STATX_SIZE == 0 { - ctx.Warningf("can't create regular file gofer.dentry without file size") - return nil, linuxerr.EIO - } - - inoKey := inoKeyFromStat(&ino.Stat) - d := &dentry{ - fs: fs, - inoKey: inoKey, - ino: fs.inoFromKey(inoKey), - mode: atomicbitops.FromUint32(uint32(ino.Stat.Mode)), - uid: atomicbitops.FromUint32(uint32(fs.opts.dfltuid)), - gid: atomicbitops.FromUint32(uint32(fs.opts.dfltgid)), - blockSize: atomicbitops.FromUint32(hostarch.PageSize), - readFD: atomicbitops.FromInt32(-1), - writeFD: atomicbitops.FromInt32(-1), - mmapFD: atomicbitops.FromInt32(-1), - controlFDLisa: fs.client.NewFD(ino.ControlFD), - } - d.pf.dentry = d - d.cacheEntry.d = d - d.syncableListEntry.d = d - if ino.Stat.Mask&linux.STATX_UID != 0 { - d.uid = atomicbitops.FromUint32(dentryUID(lisafs.UID(ino.Stat.UID))) - } - if ino.Stat.Mask&linux.STATX_GID != 0 { - d.gid = atomicbitops.FromUint32(dentryGID(lisafs.GID(ino.Stat.GID))) - } - if ino.Stat.Mask&linux.STATX_SIZE != 0 { - d.size = atomicbitops.FromUint64(ino.Stat.Size) - } - if ino.Stat.Blksize != 0 { - d.blockSize = atomicbitops.FromUint32(ino.Stat.Blksize) - } - if ino.Stat.Mask&linux.STATX_ATIME != 0 { - d.atime = atomicbitops.FromInt64(dentryTimestamp(ino.Stat.Atime)) - } else { - d.atime = atomicbitops.FromInt64(fs.clock.Now().Nanoseconds()) - } - if ino.Stat.Mask&linux.STATX_MTIME != 0 { - d.mtime = atomicbitops.FromInt64(dentryTimestamp(ino.Stat.Mtime)) - } else { - d.mtime = atomicbitops.FromInt64(fs.clock.Now().Nanoseconds()) - } - if ino.Stat.Mask&linux.STATX_CTIME != 0 { - d.ctime = atomicbitops.FromInt64(dentryTimestamp(ino.Stat.Ctime)) - } else { - // Approximate ctime with mtime if ctime isn't available. - d.ctime = atomicbitops.FromInt64(d.mtime.Load()) - } - if ino.Stat.Mask&linux.STATX_BTIME != 0 { - d.btime = atomicbitops.FromInt64(dentryTimestamp(ino.Stat.Btime)) - } - if ino.Stat.Mask&linux.STATX_NLINK != 0 { - d.nlink = atomicbitops.FromUint32(ino.Stat.Nlink) - } else { - if ino.Stat.Mode&linux.FileTypeMask == linux.ModeDirectory { - d.nlink = atomicbitops.FromUint32(2) - } else { - d.nlink = atomicbitops.FromUint32(1) - } - } - d.vfsd.Init(d) - refs.Register(d) - fs.syncMu.Lock() - fs.syncableDentries.PushBack(&d.syncableListEntry) - fs.syncMu.Unlock() - return d, nil -} - func (fs *filesystem) inoFromKey(key inoKey) uint64 { fs.inoMu.Lock() defer fs.inoMu.Unlock() @@ -1042,19 +961,35 @@ func (fs *filesystem) nextIno() uint64 { return fs.lastIno.Add(1) } +// init must be called before first use of d. +func (d *dentry) init(impl any) { + d.pf.dentry = d + d.cacheEntry.d = d + d.syncableListEntry.d = d + // Nested impl-inheritance pattern. In memory it looks like: + // [[[ vfs.Dentry ] dentry ] dentryImpl ] + // All 3 abstractions are allocated in one allocation. We achieve this by + // making each outer dentry implementation hold the inner dentry by value. + // Then the outer most dentry is allocated and we initialize fields inward. + // Each inner dentry has a pointer to the next level of implementation. + d.impl = impl + d.vfsd.Init(d) + refs.Register(d) +} + func (d *dentry) isSynthetic() bool { - return !d.isControlFileOk() + return d.impl == nil } func (d *dentry) cachedMetadataAuthoritative() bool { return d.fs.opts.interop != InteropModeShared || d.isSynthetic() } -// updateMetadataFromStatLocked is called to update d's metadata after an update +// updateMetadataFromStatxLocked is called to update d's metadata after an update // from the remote filesystem. // Precondition: d.metadataMu must be locked. // +checklocks:d.metadataMu -func (d *dentry) updateMetadataFromStatLocked(stat *linux.Statx) { +func (d *lisafsDentry) updateMetadataFromStatxLocked(stat *linux.Statx) { if stat.Mask&linux.STATX_TYPE != 0 { if got, want := stat.Mode&linux.FileTypeMask, d.fileType(); uint32(got) != want { panic(fmt.Sprintf("gofer.dentry file type changed from %#o to %#o", want, got)) @@ -1103,10 +1038,11 @@ func (d *dentry) refreshSizeLocked(ctx context.Context) error { // Can use RacyLoad() because handleMu is locked. if d.writeFD.RacyLoad() < 0 { d.handleMu.RUnlock() - // Ask the gofer if we don't have a host FD. - return d.updateMetadataLocked(ctx, nil) + // Use a suitable FD if we don't have a writable host FD. + return d.updateMetadataLocked(ctx, noHandle) } + // Using statx(2) with a minimal mask is faster than fstat(2). var stat unix.Statx_t // Can use RacyLoad() because handleMu is locked. err := unix.Statx(int(d.writeFD.RacyLoad()), "", unix.AT_EMPTY_PATH, unix.STATX_SIZE, &stat) @@ -1119,52 +1055,12 @@ func (d *dentry) refreshSizeLocked(ctx context.Context) error { } // Preconditions: !d.isSynthetic(). -func (d *dentry) updateMetadata(ctx context.Context, fd *lisafs.ClientFD) error { - // d.metadataMu must be locked *before* we getAttr so that we do not end up - // updating stale attributes in d.updateFromP9AttrsLocked(). +func (d *dentry) updateMetadata(ctx context.Context) error { + // d.metadataMu must be locked *before* we stat so that we do not end up + // updating stale attributes in d.updateMetadataFromStatLocked(). d.metadataMu.Lock() defer d.metadataMu.Unlock() - return d.updateMetadataLocked(ctx, fd) -} - -// Preconditions: -// - !d.isSynthetic(). -// - d.metadataMu is locked. -// -// +checklocks:d.metadataMu -func (d *dentry) updateMetadataLocked(ctx context.Context, fd *lisafs.ClientFD) error { - handleMuRLocked := false - if fd == nil { - // Use open FDs in preferenece to the control FD. This may be significantly - // more efficient in some implementations. Prefer a writable FD over a - // readable one since some filesystem implementations may update a writable - // FD's metadata after writes, without making metadata updates immediately - // visible to read-only FDs representing the same file. - d.handleMu.RLock() - switch { - case d.writeFDLisa.Ok(): - fd = &d.writeFDLisa - handleMuRLocked = true - case d.readFDLisa.Ok(): - fd = &d.readFDLisa - handleMuRLocked = true - default: - fd = &d.controlFDLisa - d.handleMu.RUnlock() - } - } - - var stat linux.Statx - err := fd.StatTo(ctx, &stat) - if handleMuRLocked { - // handleMu must be released before updateMetadataFromStatLocked(). - d.handleMu.RUnlock() // +checklocksforce: complex case. - } - if err != nil { - return err - } - d.updateMetadataFromStatLocked(&stat) - return nil + return d.updateMetadataLocked(ctx, noHandle) } func (d *dentry) fileType() uint32 { @@ -1279,6 +1175,7 @@ func (d *dentry) setStat(ctx context.Context, creds *auth.Credentials, opts *vfs var failureErr error if !d.isSynthetic() { if stat.Mask != 0 { + d.handleMu.RLock() if stat.Mask&linux.STATX_SIZE != 0 { // d.dataMu must be held around the update to both the remote // file's size and d.size to serialize with writeback (which @@ -1287,7 +1184,8 @@ func (d *dentry) setStat(ctx context.Context, creds *auth.Credentials, opts *vfs d.dataMu.Lock() } var err error - failureMask, failureErr, err = d.controlFDLisa.SetStat(ctx, stat) + failureMask, failureErr, err = d.setStatLocked(ctx, stat) + d.handleMu.RUnlock() if err != nil { if stat.Mask&linux.STATX_SIZE != 0 { d.dataMu.Unlock() // +checklocksforce: locked conditionally above @@ -1343,44 +1241,6 @@ func (d *dentry) setStat(ctx context.Context, creds *auth.Credentials, opts *vfs return nil } -// Preconditions: -// - filesystem.renameMu must be locked. -// - d.dirMu must be locked. -// - d.isDir(). -func (d *dentry) mknodLocked(ctx context.Context, name string, creds *auth.Credentials, opts vfs.MknodOptions, ds **[]*dentry) error { - if _, ok := opts.Endpoint.(transport.HostBoundEndpoint); !ok { - childInode, err := d.controlFDLisa.MknodAt(ctx, name, opts.Mode, lisafs.UID(creds.EffectiveKUID), lisafs.GID(creds.EffectiveKGID), opts.DevMinor, opts.DevMajor) - if err != nil { - return err - } - return d.insertCreatedChildLocked(ctx, &childInode, name, nil, ds) - } - - // This mknod(2) is coming from unix bind(2), as opts.Endpoint is set. - sockType := opts.Endpoint.(transport.Endpoint).Type() - childInode, boundSocketFD, err := d.controlFDLisa.BindAt(ctx, sockType, name, opts.Mode, lisafs.UID(creds.EffectiveKUID), lisafs.GID(creds.EffectiveKGID)) - if err != nil { - return err - } - hbep := opts.Endpoint.(transport.HostBoundEndpoint) - if err := hbep.SetBoundSocketFD(boundSocketFD); err != nil { - boundSocketFD.Close(ctx) - if err := d.controlFDLisa.UnlinkAt(ctx, name, 0 /* flags */); err != nil { - log.Warningf("failed to clean up socket which was created by BindAt RPC: %v", err) - } - d.fs.client.CloseFD(ctx, childInode.ControlFD, false /* flush */) - return err - } - if err := d.insertCreatedChildLocked(ctx, &childInode, name, func(child *dentry) { - // Set the endpoint on the newly created child dentry. - child.endpoint = opts.Endpoint - }, ds); err != nil { - hbep.ResetBoundSocketFD(ctx) - return err - } - return nil -} - // doAllocate performs an allocate operation on d. Note that d.metadataMu will // be held when allocate is called. func (d *dentry) doAllocate(ctx context.Context, offset, length uint64, allocate func() error) error { @@ -1831,8 +1691,9 @@ func (d *dentry) destroyLocked(ctx context.Context) { mf := d.fs.mfp.MemoryFile() d.handleMu.Lock() d.dataMu.Lock() - if h := d.writeHandleLocked(); h.isOpen() { + if d.isWriteHandleOk() { // Write dirty pages back to the remote filesystem. + h := d.writeHandle() if err := fsutil.SyncDirtyAll(ctx, &d.cache, &d.dirty, d.size.Load(), mf, h.writeFromBlocksAt); err != nil { log.Warningf("gofer.dentry.destroyLocked: failed to write dirty data back: %v", err) } @@ -1844,12 +1705,7 @@ func (d *dentry) destroyLocked(ctx context.Context) { d.dirty.RemoveAll() } d.dataMu.Unlock() - if d.readFDLisa.Ok() && d.readFDLisa.ID() != d.writeFDLisa.ID() { - d.readFDLisa.Close(ctx, false /* flush */) - } - if d.writeFDLisa.Ok() { - d.writeFDLisa.Close(ctx, false /* flush */) - } + d.destroyImpl(ctx) // Can use RacyLoad() because handleMu is locked. if d.readFD.RacyLoad() >= 0 { _ = unix.Close(int(d.readFD.RacyLoad())) @@ -1872,12 +1728,6 @@ func (d *dentry) destroyLocked(ctx context.Context) { // this turns out to be too expensive in many cases, so for now we // don't do this. - // Close the control FD. Propagate the Close RPCs immediately to the server - // if the dentry being destroyed is a deleted regular file. This is to - // release the disk space on remote immediately. - flushClose := d.isDeleted() && d.isRegularFile() - d.controlFDLisa.Close(ctx, flushClose) - // Remove d from the set of syncable dentries. d.fs.syncMu.Lock() d.fs.syncableDentries.Remove(&d.syncableListEntry) @@ -1902,50 +1752,42 @@ func (d *dentry) setDeleted() { d.deleted.Store(1) } -func (d *dentry) isControlFileOk() bool { - return d.controlFDLisa.Ok() -} - -func (d *dentry) isReadFileOk() bool { - return d.readFDLisa.Ok() -} - func (d *dentry) listXattr(ctx context.Context, size uint64) ([]string, error) { - if !d.isControlFileOk() { + if d.isSynthetic() { return nil, nil } - return d.controlFDLisa.ListXattr(ctx, size) + return d.listXattrImpl(ctx, size) } func (d *dentry) getXattr(ctx context.Context, creds *auth.Credentials, opts *vfs.GetXattrOptions) (string, error) { - if !d.isControlFileOk() { + if d.isSynthetic() { return "", linuxerr.ENODATA } if err := d.checkXattrPermissions(creds, opts.Name, vfs.MayRead); err != nil { return "", err } - return d.controlFDLisa.GetXattr(ctx, opts.Name, opts.Size) + return d.getXattrImpl(ctx, opts) } func (d *dentry) setXattr(ctx context.Context, creds *auth.Credentials, opts *vfs.SetXattrOptions) error { - if !d.isControlFileOk() { + if d.isSynthetic() { return linuxerr.EPERM } if err := d.checkXattrPermissions(creds, opts.Name, vfs.MayWrite); err != nil { return err } - return d.controlFDLisa.SetXattr(ctx, opts.Name, opts.Value, opts.Flags) + return d.setXattrImpl(ctx, opts) } func (d *dentry) removeXattr(ctx context.Context, creds *auth.Credentials, name string) error { - if !d.isControlFileOk() { + if d.isSynthetic() { return linuxerr.EPERM } if err := d.checkXattrPermissions(creds, name, vfs.MayWrite); err != nil { return err } - return d.controlFDLisa.RemoveXattr(ctx, name) + return d.removeXattrImpl(ctx, name) } // Preconditions: @@ -1956,7 +1798,7 @@ func (d *dentry) ensureSharedHandle(ctx context.Context, read, write, trunc bool // O_TRUNC). if !trunc { d.handleMu.RLock() - canReuseCurHandle := (!read || d.readFDLisa.Ok()) && (!write || d.writeFDLisa.Ok()) + canReuseCurHandle := (!read || d.isReadHandleOk()) && (!write || d.isWriteHandleOk()) d.handleMu.RUnlock() if canReuseCurHandle { // Current handles are sufficient. @@ -1964,142 +1806,129 @@ func (d *dentry) ensureSharedHandle(ctx context.Context, read, write, trunc bool } } + d.handleMu.Lock() + needNewHandle := (read && !d.isReadHandleOk()) || (write && !d.isWriteHandleOk()) || trunc + if !needNewHandle { + d.handleMu.Unlock() + return nil + } + var fdsToCloseArr [2]int32 fdsToClose := fdsToCloseArr[:0] invalidateTranslations := false - d.handleMu.Lock() - if (read && !d.readFDLisa.Ok()) || (write && !d.writeFDLisa.Ok()) || trunc { - // Get a new handle. If this file has been opened for both reading and - // writing, try to get a single handle that is usable for both: - // - // - Writable memory mappings of a host FD require that the host FD is - // opened for both reading and writing. - // - // - NOTE(b/141991141): Some filesystems may not ensure coherence - // between multiple handles for the same file. - openReadable := d.readFDLisa.Ok() || read - openWritable := d.writeFDLisa.Ok() || write - h, err := openHandle(ctx, d.controlFDLisa, openReadable, openWritable, trunc) - if linuxerr.Equals(linuxerr.EACCES, err) && (openReadable != read || openWritable != write) { - // It may not be possible to use a single handle for both - // reading and writing, since permissions on the file may have - // changed to e.g. disallow reading after previously being - // opened for reading. In this case, we have no choice but to - // use separate handles for reading and writing. - ctx.Debugf("gofer.dentry.ensureSharedHandle: bifurcating read/write handles for dentry %p", d) - openReadable = read - openWritable = write - h, err = openHandle(ctx, d.controlFDLisa, openReadable, openWritable, trunc) - } - if err != nil { - d.handleMu.Unlock() - return err - } + // Get a new handle. If this file has been opened for both reading and + // writing, try to get a single handle that is usable for both: + // + // - Writable memory mappings of a host FD require that the host FD is + // opened for both reading and writing. + // + // - NOTE(b/141991141): Some filesystems may not ensure coherence + // between multiple handles for the same file. + openReadable := d.isReadHandleOk() || read + openWritable := d.isWriteHandleOk() || write + h, err := d.openHandle(ctx, openReadable, openWritable, trunc) + if linuxerr.Equals(linuxerr.EACCES, err) && (openReadable != read || openWritable != write) { + // It may not be possible to use a single handle for both + // reading and writing, since permissions on the file may have + // changed to e.g. disallow reading after previously being + // opened for reading. In this case, we have no choice but to + // use separate handles for reading and writing. + ctx.Debugf("gofer.dentry.ensureSharedHandle: bifurcating read/write handles for dentry %p", d) + openReadable = read + openWritable = write + h, err = d.openHandle(ctx, openReadable, openWritable, trunc) + } + if err != nil { + d.handleMu.Unlock() + return err + } - // Update d.readFD and d.writeFD - if h.fd >= 0 { - if openReadable && openWritable && (d.readFD.RacyLoad() < 0 || d.writeFD.RacyLoad() < 0 || d.readFD.RacyLoad() != d.writeFD.RacyLoad()) { - // Replace existing FDs with this one. - if d.readFD.RacyLoad() >= 0 { - // We already have a readable FD that may be in use by - // concurrent callers of d.pf.FD(). - if d.fs.opts.overlayfsStaleRead { - // If overlayfsStaleRead is in effect, then the new FD - // may not be coherent with the existing one, so we - // have no choice but to switch to mappings of the new - // FD in both the application and sentry. - if err := d.pf.hostFileMapper.RegenerateMappings(int(h.fd)); err != nil { - d.handleMu.Unlock() - ctx.Warningf("gofer.dentry.ensureSharedHandle: failed to replace sentry mappings of old FD with mappings of new FD: %v", err) - h.close(ctx) - return err - } - fdsToClose = append(fdsToClose, d.readFD.RacyLoad()) - invalidateTranslations = true - d.readFD.Store(h.fd) - } else { - // Otherwise, we want to avoid invalidating existing - // memmap.Translations (which is expensive); instead, use - // dup3 to make the old file descriptor refer to the new - // file description, then close the new file descriptor - // (which is no longer needed). Racing callers of d.pf.FD() - // may use the old or new file description, but this - // doesn't matter since they refer to the same file, and - // any racing mappings must be read-only. - if err := unix.Dup3(int(h.fd), int(d.readFD.RacyLoad()), unix.O_CLOEXEC); err != nil { - oldFD := d.readFD.RacyLoad() - d.handleMu.Unlock() - ctx.Warningf("gofer.dentry.ensureSharedHandle: failed to dup fd %d to fd %d: %v", h.fd, oldFD, err) - h.close(ctx) - return err - } - fdsToClose = append(fdsToClose, h.fd) - h.fd = d.readFD.RacyLoad() + // Update d.readFD and d.writeFD + if h.fd >= 0 { + if openReadable && openWritable && (d.readFD.RacyLoad() < 0 || d.writeFD.RacyLoad() < 0 || d.readFD.RacyLoad() != d.writeFD.RacyLoad()) { + // Replace existing FDs with this one. + if d.readFD.RacyLoad() >= 0 { + // We already have a readable FD that may be in use by + // concurrent callers of d.pf.FD(). + if d.fs.opts.overlayfsStaleRead { + // If overlayfsStaleRead is in effect, then the new FD + // may not be coherent with the existing one, so we + // have no choice but to switch to mappings of the new + // FD in both the application and sentry. + if err := d.pf.hostFileMapper.RegenerateMappings(int(h.fd)); err != nil { + d.handleMu.Unlock() + ctx.Warningf("gofer.dentry.ensureSharedHandle: failed to replace sentry mappings of old FD with mappings of new FD: %v", err) + h.close(ctx) + return err } - } else { - d.readFD.Store(h.fd) - } - if d.writeFD.RacyLoad() != h.fd && d.writeFD.RacyLoad() >= 0 { - fdsToClose = append(fdsToClose, d.writeFD.RacyLoad()) - } - d.writeFD.Store(h.fd) - d.mmapFD.Store(h.fd) - } else if openReadable && d.readFD.RacyLoad() < 0 { - d.readFD.Store(h.fd) - // If the file has not been opened for writing, the new FD may - // be used for read-only memory mappings. If the file was - // previously opened for reading (without an FD), then existing - // translations of the file may use the internal page cache; - // invalidate those mappings. - if !d.writeFDLisa.Ok() { - invalidateTranslations = d.readFDLisa.Ok() - d.mmapFD.Store(h.fd) - } - } else if openWritable && d.writeFD.RacyLoad() < 0 { - d.writeFD.Store(h.fd) - if d.readFD.RacyLoad() >= 0 { - // We have an existing read-only FD, but the file has just - // been opened for writing, so we need to start supporting - // writable memory mappings. However, the new FD is not - // readable, so we have no FD that can be used to create - // writable memory mappings. Switch to using the internal - // page cache. + fdsToClose = append(fdsToClose, d.readFD.RacyLoad()) invalidateTranslations = true - d.mmapFD.Store(-1) + d.readFD.Store(h.fd) + } else { + // Otherwise, we want to avoid invalidating existing + // memmap.Translations (which is expensive); instead, use + // dup3 to make the old file descriptor refer to the new + // file description, then close the new file descriptor + // (which is no longer needed). Racing callers of d.pf.FD() + // may use the old or new file description, but this + // doesn't matter since they refer to the same file, and + // any racing mappings must be read-only. + if err := unix.Dup3(int(h.fd), int(d.readFD.RacyLoad()), unix.O_CLOEXEC); err != nil { + oldFD := d.readFD.RacyLoad() + d.handleMu.Unlock() + ctx.Warningf("gofer.dentry.ensureSharedHandle: failed to dup fd %d to fd %d: %v", h.fd, oldFD, err) + h.close(ctx) + return err + } + fdsToClose = append(fdsToClose, h.fd) + h.fd = d.readFD.RacyLoad() } } else { - // The new FD is not useful. - fdsToClose = append(fdsToClose, h.fd) + d.readFD.Store(h.fd) } - } else if openWritable && d.writeFD.RacyLoad() < 0 && d.mmapFD.RacyLoad() >= 0 { - // We have an existing read-only FD, but the file has just been - // opened for writing, so we need to start supporting writable - // memory mappings. However, we have no writable host FD. Switch to - // using the internal page cache. - invalidateTranslations = true - d.mmapFD.Store(-1) - } - - // Switch to new fids/FDs. - oldReadFD := lisafs.InvalidFDID - if openReadable { - oldReadFD = d.readFDLisa.ID() - d.readFDLisa = h.fdLisa - } - oldWriteFD := lisafs.InvalidFDID - if openWritable { - oldWriteFD = d.writeFDLisa.ID() - d.writeFDLisa = h.fdLisa - } - // NOTE(b/141991141): Close old FDs before making new fids visible (by - // unlocking d.handleMu). - if oldReadFD.Ok() { - d.fs.client.CloseFD(ctx, oldReadFD, false /* flush */) - } - if oldWriteFD.Ok() && oldReadFD != oldWriteFD { - d.fs.client.CloseFD(ctx, oldWriteFD, false /* flush */) + if d.writeFD.RacyLoad() != h.fd && d.writeFD.RacyLoad() >= 0 { + fdsToClose = append(fdsToClose, d.writeFD.RacyLoad()) + } + d.writeFD.Store(h.fd) + d.mmapFD.Store(h.fd) + } else if openReadable && d.readFD.RacyLoad() < 0 { + readHandleWasOk := d.isReadHandleOk() + d.readFD.Store(h.fd) + // If the file has not been opened for writing, the new FD may + // be used for read-only memory mappings. If the file was + // previously opened for reading (without an FD), then existing + // translations of the file may use the internal page cache; + // invalidate those mappings. + if !d.isWriteHandleOk() { + invalidateTranslations = readHandleWasOk + d.mmapFD.Store(h.fd) + } + } else if openWritable && d.writeFD.RacyLoad() < 0 { + d.writeFD.Store(h.fd) + if d.readFD.RacyLoad() >= 0 { + // We have an existing read-only FD, but the file has just + // been opened for writing, so we need to start supporting + // writable memory mappings. However, the new FD is not + // readable, so we have no FD that can be used to create + // writable memory mappings. Switch to using the internal + // page cache. + invalidateTranslations = true + d.mmapFD.Store(-1) + } + } else { + // The new FD is not useful. + fdsToClose = append(fdsToClose, h.fd) } + } else if openWritable && d.writeFD.RacyLoad() < 0 && d.mmapFD.RacyLoad() >= 0 { + // We have an existing read-only FD, but the file has just been + // opened for writing, so we need to start supporting writable + // memory mappings. However, we have no writable host FD. Switch to + // using the internal page cache. + invalidateTranslations = true + d.mmapFD.Store(-1) } + + d.updateHandles(ctx, h, openReadable, openWritable) d.handleMu.Unlock() if invalidateTranslations { @@ -2118,22 +1947,6 @@ func (d *dentry) ensureSharedHandle(ctx context.Context, read, write, trunc bool return nil } -// Preconditions: d.handleMu must be locked. -func (d *dentry) readHandleLocked() handle { - return handle{ - fdLisa: d.readFDLisa, - fd: d.readFD.RacyLoad(), - } -} - -// Preconditions: d.handleMu must be locked. -func (d *dentry) writeHandleLocked() handle { - return handle{ - fdLisa: d.writeFDLisa, - fd: d.writeFD.RacyLoad(), - } -} - func (d *dentry) syncRemoteFile(ctx context.Context) error { d.handleMu.RLock() defer d.handleMu.RUnlock() @@ -2142,38 +1955,23 @@ func (d *dentry) syncRemoteFile(ctx context.Context) error { // Preconditions: d.handleMu must be locked. func (d *dentry) syncRemoteFileLocked(ctx context.Context) error { - // If we have a host FD, fsyncing it is likely to be faster than an fsync - // RPC. Prefer syncing write handles over read handles, since some remote + // Prefer syncing write handles over read handles, since some remote // filesystem implementations may not sync changes made through write // handles otherwise. - if d.writeFD.RacyLoad() >= 0 { - ctx.UninterruptibleSleepStart(false) - err := unix.Fsync(int(d.writeFD.RacyLoad())) - ctx.UninterruptibleSleepFinish(false) - return err - } - if d.writeFDLisa.Ok() { - return d.writeFDLisa.Sync(ctx) - } - if d.readFD.RacyLoad() >= 0 { - ctx.UninterruptibleSleepStart(false) - err := unix.Fsync(int(d.readFD.RacyLoad())) - ctx.UninterruptibleSleepFinish(false) - return err - } - if d.readFDLisa.Ok() { - return d.readFDLisa.Sync(ctx) - } + wh := d.writeHandle() + wh.sync(ctx) + rh := d.readHandle() + rh.sync(ctx) return nil } func (d *dentry) syncCachedFile(ctx context.Context, forFilesystemSync bool) error { d.handleMu.RLock() defer d.handleMu.RUnlock() - h := d.writeHandleLocked() - if h.isOpen() { + if d.isWriteHandleOk() { // Write back dirty pages to the remote file. d.dataMu.Lock() + h := d.writeHandle() err := fsutil.SyncDirtyAll(ctx, &d.cache, &d.dirty, d.size.Load(), d.fs.mfp.MemoryFile(), h.writeFromBlocksAt) d.dataMu.Unlock() if err != nil { @@ -2186,7 +1984,7 @@ func (d *dentry) syncCachedFile(ctx context.Context, forFilesystemSync bool) err } // Only return err if we can reasonably have expected sync to succeed // (d is a regular file and was opened for writing). - if d.isRegularFile() && h.isOpen() { + if d.isRegularFile() && d.isWriteHandleOk() { return err } ctx.Debugf("gofer.dentry.syncCachedFile: syncing non-writable or non-regular-file dentry failed: %v", err) @@ -2239,11 +2037,12 @@ func (fd *fileDescription) Stat(ctx context.Context, opts vfs.StatOptions) (linu if !d.cachedMetadataAuthoritative() && opts.Mask&validMask != 0 && opts.Sync != linux.AT_STATX_DONT_SYNC { // Use specialFileFD.handle.fileLisa for the Stat if available, for the // same reason that we try to use open FD in updateMetadataLocked(). - var fdLisa *lisafs.ClientFD + var err error if sffd, ok := fd.vfsfd.Impl().(*specialFileFD); ok { - fdLisa = &sffd.handle.fdLisa + err = sffd.updateMetadata(ctx) + } else { + err = d.updateMetadata(ctx) } - err := d.updateMetadata(ctx, fdLisa) if err != nil { return linux.Statx{}, err } diff --git a/pkg/sentry/fsimpl/gofer/gofer_test.go b/pkg/sentry/fsimpl/gofer/gofer_test.go index 45bf99a4f..0c4b5b05c 100644 --- a/pkg/sentry/fsimpl/gofer/gofer_test.go +++ b/pkg/sentry/fsimpl/gofer/gofer_test.go @@ -42,7 +42,7 @@ func TestDestroyIdempotent(t *testing.T) { Mode: linux.S_IFDIR | 0666, }, } - parent, err := fs.newDentry(ctx, &parentInode) + parent, err := fs.newLisafsDentry(ctx, &parentInode) if err != nil { t.Fatalf("fs.newDentry(): %v", err) } @@ -55,7 +55,7 @@ func TestDestroyIdempotent(t *testing.T) { Size: 0, }, } - child, err := fs.newDentry(ctx, &childInode) + child, err := fs.newLisafsDentry(ctx, &childInode) if err != nil { t.Fatalf("fs.newDentry(): %v", err) } diff --git a/pkg/sentry/fsimpl/gofer/handle.go b/pkg/sentry/fsimpl/gofer/handle.go index 44a18014b..fb56e8061 100644 --- a/pkg/sentry/fsimpl/gofer/handle.go +++ b/pkg/sentry/fsimpl/gofer/handle.go @@ -18,12 +18,16 @@ import ( "golang.org/x/sys/unix" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/lisafs" - "gvisor.dev/gvisor/pkg/log" "gvisor.dev/gvisor/pkg/safemem" "gvisor.dev/gvisor/pkg/sentry/hostfd" "gvisor.dev/gvisor/pkg/sync" ) +var noHandle = handle{ + fdLisa: lisafs.ClientFD{}, // zero value is fine. + fd: -1, +} + // handle represents a remote "open file descriptor", consisting of an opened // lisafs FD and optionally a host file descriptor. // @@ -33,39 +37,10 @@ type handle struct { fd int32 // -1 if unavailable } -// Preconditions: read || write. -func openHandle(ctx context.Context, fdLisa lisafs.ClientFD, read, write, trunc bool) (handle, error) { - flags := uint32(unix.O_RDONLY) - switch { - case read && write: - flags = unix.O_RDWR - case read: - flags = unix.O_RDONLY - case write: - flags = unix.O_WRONLY - default: - log.Debugf("openHandle called with read = write = false. Falling back to read only FD.") - } - if trunc { - flags |= unix.O_TRUNC - } - openFD, hostFD, err := fdLisa.OpenAt(ctx, flags) - if err != nil { - return handle{fd: -1}, err - } - h := handle{ - fdLisa: fdLisa.Client().NewFD(openFD), - fd: int32(hostFD), - } - return h, nil -} - -func (h *handle) isOpen() bool { - return h.fdLisa.Ok() -} - func (h *handle) close(ctx context.Context) { - h.fdLisa.Close(ctx, true /* flush */) + if h.fdLisa.Ok() { + h.fdLisa.Close(ctx, true /* flush */) + } if h.fd >= 0 { unix.Close(int(h.fd)) h.fd = -1 @@ -102,6 +77,31 @@ func (h *handle) writeFromBlocksAt(ctx context.Context, srcs safemem.BlockSeq, o return safemem.FromIOWriter{rw}.WriteFromBlocks(srcs) } +func (h *handle) allocate(ctx context.Context, mode, offset, length uint64) error { + if h.fdLisa.Ok() { + return h.fdLisa.Allocate(ctx, mode, offset, length) + } + if h.fd >= 0 { + return unix.Fallocate(int(h.fd), uint32(mode), int64(offset), int64(length)) + } + return nil +} + +func (h *handle) sync(ctx context.Context) error { + // If we have a host FD, fsyncing it is likely to be faster than an fsync + // RPC. + if h.fd >= 0 { + ctx.UninterruptibleSleepStart(false) + err := unix.Fsync(int(h.fd)) + ctx.UninterruptibleSleepFinish(false) + return err + } + if h.fdLisa.Ok() { + return h.fdLisa.Sync(ctx) + } + return nil +} + type handleReadWriter struct { ctx context.Context h *handle diff --git a/pkg/sentry/fsimpl/gofer/lisafs_dentry.go b/pkg/sentry/fsimpl/gofer/lisafs_dentry.go new file mode 100644 index 000000000..fd1df694c --- /dev/null +++ b/pkg/sentry/fsimpl/gofer/lisafs_dentry.go @@ -0,0 +1,510 @@ +// Copyright 2022 The gVisor Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package gofer + +import ( + "fmt" + + "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/atomicbitops" + "gvisor.dev/gvisor/pkg/context" + "gvisor.dev/gvisor/pkg/errors/linuxerr" + "gvisor.dev/gvisor/pkg/hostarch" + "gvisor.dev/gvisor/pkg/lisafs" + "gvisor.dev/gvisor/pkg/log" + "gvisor.dev/gvisor/pkg/sentry/kernel/auth" + "gvisor.dev/gvisor/pkg/sentry/socket/unix/transport" + "gvisor.dev/gvisor/pkg/sentry/vfs" +) + +// lisafsDentry is a gofer dentry implementation. It represents a dentry backed +// by a lisafs connection. +// +// +stateify savable +type lisafsDentry struct { + dentry + + // controlFD is used by lisafs to perform path based operations on this + // dentry. controlFD is immutable. + // + // if !controlFD.Ok(), this dentry represents a synthetic file, i.e. a + // file that does not exist on the remote filesystem. As of this writing, the + // only files that can be synthetic are sockets, pipes, and directories. + controlFD lisafs.ClientFD `state:"nosave"` + + // If this dentry represents a regular file or directory, readFDLisa is a + // LISAFS FD used for reads by all regularFileFDs/directoryFDs representing + // this dentry. readFDLisa is protected by dentry.handleMu. + readFDLisa lisafs.ClientFD `state:"nosave"` + + // If this dentry represents a regular file, writeFDLisa is the LISAFS FD + // used for writes by all regularFileFDs representing this dentry. + // readFDLisa and writeFDLisa may or may not represent the same LISAFS FD. + // Once either transitions from closed (Ok() == false) to open + // (Ok() == true), it may be mutated with dentry.handleMu locked, but cannot + // be closed until the dentry is destroyed. writeFDLisa is protected by + // dentry.handleMu. + writeFDLisa lisafs.ClientFD `state:"nosave"` +} + +// newLisafsDentry creates a new dentry representing the given file. The dentry +// initially has no references, but is not cached; it is the caller's +// responsibility to set the dentry's reference count and/or call +// dentry.checkCachingLocked() as appropriate. +// newLisafsDentry takes ownership of ino. +func (fs *filesystem) newLisafsDentry(ctx context.Context, ino *lisafs.Inode) (*dentry, error) { + if ino.Stat.Mask&linux.STATX_TYPE == 0 { + ctx.Warningf("can't create gofer.dentry without file type") + fs.client.CloseFD(ctx, ino.ControlFD, false /* flush */) + return nil, linuxerr.EIO + } + if ino.Stat.Mode&linux.FileTypeMask == linux.ModeRegular && ino.Stat.Mask&linux.STATX_SIZE == 0 { + ctx.Warningf("can't create regular file gofer.dentry without file size") + fs.client.CloseFD(ctx, ino.ControlFD, false /* flush */) + return nil, linuxerr.EIO + } + + inoKey := inoKeyFromStatx(&ino.Stat) + d := &lisafsDentry{ + dentry: dentry{ + fs: fs, + inoKey: inoKey, + ino: fs.inoFromKey(inoKey), + mode: atomicbitops.FromUint32(uint32(ino.Stat.Mode)), + uid: atomicbitops.FromUint32(uint32(fs.opts.dfltuid)), + gid: atomicbitops.FromUint32(uint32(fs.opts.dfltgid)), + blockSize: atomicbitops.FromUint32(hostarch.PageSize), + readFD: atomicbitops.FromInt32(-1), + writeFD: atomicbitops.FromInt32(-1), + mmapFD: atomicbitops.FromInt32(-1), + }, + controlFD: fs.client.NewFD(ino.ControlFD), + } + if ino.Stat.Mask&linux.STATX_UID != 0 { + d.uid = atomicbitops.FromUint32(dentryUID(lisafs.UID(ino.Stat.UID))) + } + if ino.Stat.Mask&linux.STATX_GID != 0 { + d.gid = atomicbitops.FromUint32(dentryGID(lisafs.GID(ino.Stat.GID))) + } + if ino.Stat.Mask&linux.STATX_SIZE != 0 { + d.size = atomicbitops.FromUint64(ino.Stat.Size) + } + if ino.Stat.Blksize != 0 { + d.blockSize = atomicbitops.FromUint32(ino.Stat.Blksize) + } + if ino.Stat.Mask&linux.STATX_ATIME != 0 { + d.atime = atomicbitops.FromInt64(dentryTimestamp(ino.Stat.Atime)) + } else { + d.atime = atomicbitops.FromInt64(fs.clock.Now().Nanoseconds()) + } + if ino.Stat.Mask&linux.STATX_MTIME != 0 { + d.mtime = atomicbitops.FromInt64(dentryTimestamp(ino.Stat.Mtime)) + } else { + d.mtime = atomicbitops.FromInt64(fs.clock.Now().Nanoseconds()) + } + if ino.Stat.Mask&linux.STATX_CTIME != 0 { + d.ctime = atomicbitops.FromInt64(dentryTimestamp(ino.Stat.Ctime)) + } else { + // Approximate ctime with mtime if ctime isn't available. + d.ctime = atomicbitops.FromInt64(d.mtime.Load()) + } + if ino.Stat.Mask&linux.STATX_BTIME != 0 { + d.btime = atomicbitops.FromInt64(dentryTimestamp(ino.Stat.Btime)) + } + if ino.Stat.Mask&linux.STATX_NLINK != 0 { + d.nlink = atomicbitops.FromUint32(ino.Stat.Nlink) + } else { + if ino.Stat.Mode&linux.FileTypeMask == linux.ModeDirectory { + d.nlink = atomicbitops.FromUint32(2) + } else { + d.nlink = atomicbitops.FromUint32(1) + } + } + d.dentry.init(d) + fs.syncMu.Lock() + fs.syncableDentries.PushBack(&d.syncableListEntry) + fs.syncMu.Unlock() + return &d.dentry, nil +} + +func (d *lisafsDentry) updateHandles(ctx context.Context, h handle, readable, writable bool) { + // Switch to new LISAFS FDs. Note that the read, write and mmap host FDs are + // updated separately. + oldReadFD := lisafs.InvalidFDID + if readable { + oldReadFD = d.readFDLisa.ID() + d.readFDLisa = h.fdLisa + } + oldWriteFD := lisafs.InvalidFDID + if writable { + oldWriteFD = d.writeFDLisa.ID() + d.writeFDLisa = h.fdLisa + } + // NOTE(b/141991141): Close old FDs before making new fids visible (by + // unlocking d.handleMu). + if oldReadFD.Ok() { + d.fs.client.CloseFD(ctx, oldReadFD, false /* flush */) + } + if oldWriteFD.Ok() && oldReadFD != oldWriteFD { + d.fs.client.CloseFD(ctx, oldWriteFD, false /* flush */) + } +} + +// Precondition: d.metadataMu must be locked. +// +// +checklocks:d.metadataMu +func (d *lisafsDentry) updateMetadataLocked(ctx context.Context, h handle) error { + handleMuRLocked := false + if !h.fdLisa.Ok() { + // Use open FDs in preferenece to the control FD. This may be significantly + // more efficient in some implementations. Prefer a writable FD over a + // readable one since some filesystem implementations may update a writable + // FD's metadata after writes, without making metadata updates immediately + // visible to read-only FDs representing the same file. + d.handleMu.RLock() + switch { + case d.writeFDLisa.Ok(): + h.fdLisa = d.writeFDLisa + handleMuRLocked = true + case d.readFDLisa.Ok(): + h.fdLisa = d.readFDLisa + handleMuRLocked = true + default: + h.fdLisa = d.controlFD + d.handleMu.RUnlock() + } + } + + var stat linux.Statx + err := h.fdLisa.StatTo(ctx, &stat) + if handleMuRLocked { + // handleMu must be released before updateMetadataFromStatLocked(). + d.handleMu.RUnlock() // +checklocksforce: complex case. + } + if err != nil { + return err + } + d.updateMetadataFromStatxLocked(&stat) + return nil +} + +func chmod(ctx context.Context, controlFD lisafs.ClientFD, mode uint16) error { + setStat := linux.Statx{ + Mask: linux.STATX_MODE, + Mode: mode, + } + _, failureErr, err := controlFD.SetStat(ctx, &setStat) + if err != nil { + return err + } + return failureErr +} + +func (d *lisafsDentry) destroy(ctx context.Context) { + if d.readFDLisa.Ok() && d.readFDLisa.ID() != d.writeFDLisa.ID() { + d.readFDLisa.Close(ctx, false /* flush */) + } + if d.writeFDLisa.Ok() { + d.writeFDLisa.Close(ctx, false /* flush */) + } + if d.controlFD.Ok() { + // Close the control FD. Propagate the Close RPCs immediately to the server + // if the dentry being destroyed is a deleted regular file. This is to + // release the disk space on remote immediately. This will flush the above + // read/write lisa FDs as well. + flushClose := d.isDeleted() && d.isRegularFile() + d.controlFD.Close(ctx, flushClose) + } +} + +func (d *lisafsDentry) getRemoteChild(ctx context.Context, name string) (*dentry, error) { + childInode, err := d.controlFD.Walk(ctx, name) + if err != nil { + return nil, err + } + return d.fs.newLisafsDentry(ctx, &childInode) +} + +// Preconditions: +// - fs.renameMu must be locked. +// - parent.dirMu must be locked. +// - parent.isDir(). +// - name is not "." or "..". +// - dentry at name must not already exist in dentry tree. +func (d *lisafsDentry) getRemoteChildAndWalkPathLocked(ctx context.Context, rp *vfs.ResolvingPath, ds **[]*dentry) (*dentry, error) { + // Walk as much of the path as possible in 1 RPC. + // Note that pit is a copy of the iterator that does not affect rp. + var names []string + for pit := rp.Pit(); pit.Ok(); pit = pit.Next() { + name := pit.String() + if name == "." { + continue + } + if name == ".." { + break + } + names = append(names, name) + } + status, inodes, err := d.controlFD.WalkMultiple(ctx, names) + if err != nil { + return nil, err + } + if len(inodes) == 0 { + d.cacheNegativeLookupLocked(names[0]) + return nil, linuxerr.ENOENT + } + + // Add the walked inodes into the dentry tree. + startParent := &d.dentry + curParent := startParent + curParentDirMuLock := func() { + if curParent != startParent { + curParent.dirMu.Lock() + } + } + curParentDirMuUnlock := func() { + if curParent != startParent { + curParent.dirMu.Unlock() // +checklocksforce: locked via curParentDirMuLock(). + } + } + var ret *dentry + var dentryCreationErr error + for i := range inodes { + if dentryCreationErr != nil { + d.fs.client.CloseFD(ctx, inodes[i].ControlFD, false /* flush */) + continue + } + + child, err := d.fs.newLisafsDentry(ctx, &inodes[i]) + if err != nil { + dentryCreationErr = err + continue + } + curParentDirMuLock() + curParent.cacheNewChildLocked(child, names[i]) + curParentDirMuUnlock() + // For now, child has 0 references, so our caller should call + // child.checkCachingLocked(). curParent gained a ref so we should also + // call curParent.checkCachingLocked() so it can be removed from the cache + // if needed. We only do that for the first iteration because all + // subsequent parents would have already been added to ds. + if i == 0 { + *ds = appendDentry(*ds, curParent) + } + *ds = appendDentry(*ds, child) + curParent = child + if i == 0 { + ret = child + } + } + + if status == lisafs.WalkComponentDoesNotExist && curParent.isDir() { + curParentDirMuLock() + curParent.cacheNegativeLookupLocked(names[len(inodes)]) + curParentDirMuUnlock() + } + return ret, dentryCreationErr +} + +func (d *lisafsDentry) newChildDentry(ctx context.Context, childIno *lisafs.Inode, childName string) (*dentry, error) { + child, err := d.fs.newLisafsDentry(ctx, childIno) + if err != nil { + if err := d.controlFD.UnlinkAt(ctx, childName, 0 /* flags */); err != nil { + log.Warningf("failed to clean up created child %s after newLisafsDentry() failed: %v", childName, err) + } + } + return child, err +} + +func (d *lisafsDentry) mknod(ctx context.Context, name string, creds *auth.Credentials, opts *vfs.MknodOptions) (*dentry, error) { + if _, ok := opts.Endpoint.(transport.HostBoundEndpoint); !ok { + childInode, err := d.controlFD.MknodAt(ctx, name, opts.Mode, lisafs.UID(creds.EffectiveKUID), lisafs.GID(creds.EffectiveKGID), opts.DevMinor, opts.DevMajor) + if err != nil { + return nil, err + } + return d.newChildDentry(ctx, &childInode, name) + } + + // This mknod(2) is coming from unix bind(2), as opts.Endpoint is set. + sockType := opts.Endpoint.(transport.Endpoint).Type() + childInode, boundSocketFD, err := d.controlFD.BindAt(ctx, sockType, name, opts.Mode, lisafs.UID(creds.EffectiveKUID), lisafs.GID(creds.EffectiveKGID)) + if err != nil { + return nil, err + } + hbep := opts.Endpoint.(transport.HostBoundEndpoint) + if err := hbep.SetBoundSocketFD(boundSocketFD); err != nil { + boundSocketFD.Close(ctx) + if err := d.controlFD.UnlinkAt(ctx, name, 0 /* flags */); err != nil { + log.Warningf("failed to clean up socket which was created by BindAt RPC: %v", err) + } + d.fs.client.CloseFD(ctx, childInode.ControlFD, false /* flush */) + return nil, err + } + child, err := d.newChildDentry(ctx, &childInode, name) + if err != nil { + hbep.ResetBoundSocketFD(ctx) + return nil, err + } + // Set the endpoint on the newly created child dentry. + child.endpoint = opts.Endpoint + return child, nil +} + +func (d *lisafsDentry) link(ctx context.Context, target *lisafsDentry, name string) (*dentry, error) { + linkInode, err := d.controlFD.LinkAt(ctx, target.controlFD.ID(), name) + if err != nil { + return nil, err + } + return d.newChildDentry(ctx, &linkInode, name) +} + +func (d *lisafsDentry) mkdir(ctx context.Context, name string, mode linux.FileMode, uid auth.KUID, gid auth.KGID) (*dentry, error) { + childDirInode, err := d.controlFD.MkdirAt(ctx, name, mode, lisafs.UID(uid), lisafs.GID(gid)) + if err != nil { + return nil, err + } + return d.newChildDentry(ctx, &childDirInode, name) +} + +func (d *lisafsDentry) symlink(ctx context.Context, name, target string, creds *auth.Credentials) (*dentry, error) { + symlinkInode, err := d.controlFD.SymlinkAt(ctx, name, target, lisafs.UID(creds.EffectiveKUID), lisafs.GID(creds.EffectiveKGID)) + if err != nil { + return nil, err + } + return d.newChildDentry(ctx, &symlinkInode, name) +} + +func (d *lisafsDentry) openCreate(ctx context.Context, name string, flags uint32, mode linux.FileMode, uid auth.KUID, gid auth.KGID) (*dentry, handle, error) { + ino, openFD, hostFD, err := d.controlFD.OpenCreateAt(ctx, name, flags, mode, lisafs.UID(uid), lisafs.GID(gid)) + if err != nil { + return nil, noHandle, err + } + + h := handle{ + fdLisa: d.fs.client.NewFD(openFD), + fd: int32(hostFD), + } + child, err := d.fs.newLisafsDentry(ctx, &ino) + if err != nil { + h.close(ctx) + return nil, noHandle, err + } + return child, h, nil +} + +func (d *lisafsDentry) getDirentsLocked(ctx context.Context, count int, recordDirent func(name string, key inoKey, dType uint8)) error { + // shouldSeek0 indicates whether the server should SEEK to 0 before reading + // directory entries. + shouldSeek0 := true + for { + countLisa := int32(count) + if shouldSeek0 { + // See lisafs.Getdents64Req.Count. + countLisa = -countLisa + shouldSeek0 = false + } + dirents, err := d.readFDLisa.Getdents64(ctx, countLisa) + if err != nil { + return err + } + if len(dirents) == 0 { + return nil + } + for i := range dirents { + name := string(dirents[i].Name) + if name == "." || name == ".." { + continue + } + recordDirent(name, inoKey{ + ino: uint64(dirents[i].Ino), + devMinor: uint32(dirents[i].DevMinor), + devMajor: uint32(dirents[i].DevMajor), + }, uint8(dirents[i].Type)) + } + } +} + +func flush(ctx context.Context, fd lisafs.ClientFD) error { + if fd.Ok() { + return fd.Flush(ctx) + } + return nil +} + +func (d *lisafsDentry) statfs(ctx context.Context) (linux.Statfs, error) { + var statFS lisafs.StatFS + if err := d.controlFD.StatFSTo(ctx, &statFS); err != nil { + return linux.Statfs{}, err + } + return linux.Statfs{ + BlockSize: statFS.BlockSize, + FragmentSize: statFS.BlockSize, + Blocks: statFS.Blocks, + BlocksFree: statFS.BlocksFree, + BlocksAvailable: statFS.BlocksAvailable, + Files: statFS.Files, + FilesFree: statFS.FilesFree, + NameLength: statFS.NameLength, + }, nil +} + +func (d *lisafsDentry) restoreFile(ctx context.Context, inode *lisafs.Inode, opts *vfs.CompleteRestoreOptions) error { + d.controlFD = d.fs.client.NewFD(inode.ControlFD) + + // Gofers do not preserve inoKey across checkpoint/restore, so: + // + // - We must assume that the remote filesystem did not change in a way that + // would invalidate dentries, since we can't revalidate dentries by + // checking inoKey. + // + // - We need to associate the new inoKey with the existing d.ino. + d.inoKey = inoKeyFromStatx(&inode.Stat) + d.fs.inoMu.Lock() + d.fs.inoByKey[d.inoKey] = d.ino + d.fs.inoMu.Unlock() + + // Check metadata stability before updating metadata. + d.metadataMu.Lock() + defer d.metadataMu.Unlock() + if d.isRegularFile() { + if opts.ValidateFileSizes { + if inode.Stat.Mask&linux.STATX_SIZE == 0 { + return vfs.ErrCorruption{fmt.Errorf("gofer.dentry(%q).restoreFile: file size validation failed: file size not available", genericDebugPathname(&d.dentry))} + } + if d.size.RacyLoad() != inode.Stat.Size { + return vfs.ErrCorruption{fmt.Errorf("gofer.dentry(%q).restoreFile: file size validation failed: size changed from %d to %d", genericDebugPathname(&d.dentry), d.size.Load(), inode.Stat.Size)} + } + } + if opts.ValidateFileModificationTimestamps { + if inode.Stat.Mask&linux.STATX_MTIME == 0 { + return vfs.ErrCorruption{fmt.Errorf("gofer.dentry(%q).restoreFile: mtime validation failed: mtime not available", genericDebugPathname(&d.dentry))} + } + if want := dentryTimestamp(inode.Stat.Mtime); d.mtime.RacyLoad() != want { + return vfs.ErrCorruption{fmt.Errorf("gofer.dentry(%q).restoreFile: mtime validation failed: mtime changed from %+v to %+v", genericDebugPathname(&d.dentry), linux.NsecToStatxTimestamp(d.mtime.RacyLoad()), linux.NsecToStatxTimestamp(want))} + } + } + } + if !d.cachedMetadataAuthoritative() { + d.updateMetadataFromStatxLocked(&inode.Stat) + } + + 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 nil +} diff --git a/pkg/sentry/fsimpl/gofer/regular_file.go b/pkg/sentry/fsimpl/gofer/regular_file.go index cecb59e75..9fd1859b8 100644 --- a/pkg/sentry/fsimpl/gofer/regular_file.go +++ b/pkg/sentry/fsimpl/gofer/regular_file.go @@ -94,21 +94,14 @@ func (fd *regularFileFD) OnClose(ctx context.Context) error { return nil } } - d.handleMu.RLock() - defer d.handleMu.RUnlock() - if !d.writeFDLisa.Ok() { - return nil - } - return d.writeFDLisa.Flush(ctx) + return d.flush(ctx) } // Allocate implements vfs.FileDescriptionImpl.Allocate. func (fd *regularFileFD) Allocate(ctx context.Context, mode, offset, length uint64) error { d := fd.dentry() return d.doAllocate(ctx, offset, length, func() error { - d.handleMu.RLock() - defer d.handleMu.RUnlock() - return d.writeFDLisa.Allocate(ctx, mode, offset, length) + return d.allocate(ctx, mode, offset, length) }) } @@ -279,15 +272,10 @@ func (fd *regularFileFD) pwrite(ctx context.Context, src usermem.IOSequence, off // If setuid or setgid were set, update d.mode and propagate // changes to the host. if newMode := vfs.ClearSUIDAndSGID(oldMode); newMode != oldMode { - d.mode.Store(newMode) - stat := linux.Statx{Mask: linux.STATX_MODE, Mode: uint16(newMode)} - failureMask, failureErr, err := d.controlFDLisa.SetStat(ctx, &stat) - if err != nil { + if err := d.chmod(ctx, uint16(newMode)); err != nil { return 0, offset, err } - if failureMask != 0 { - return 0, offset, failureErr - } + d.mode.Store(newMode) } } @@ -379,9 +367,9 @@ func (rw *dentryReadWriter) ReadToBlocks(dsts safemem.BlockSeq) (uint64, error) // coherence with memory-mapped I/O), or if InteropModeShared is in effect // (which prevents us from caching file contents and makes dentry.size // unreliable), or if the file was opened O_DIRECT, read directly from - // dentry.readHandleLocked() without locking dentry.dataMu. + // readHandle() without locking dentry.dataMu. rw.d.handleMu.RLock() - h := rw.d.readHandleLocked() + h := rw.d.readHandle() if (rw.d.mmapFD.RacyLoad() >= 0 && !rw.d.fs.opts.forcePageCache) || rw.d.fs.opts.interop == InteropModeShared || rw.direct { n, err := h.readToBlocksAt(rw.ctx, dsts, rw.off) rw.d.handleMu.RUnlock() @@ -498,10 +486,10 @@ func (rw *dentryReadWriter) WriteFromBlocks(srcs safemem.BlockSeq) (uint64, erro // If we have a mmappable host FD (which must be used here to ensure // coherence with memory-mapped I/O), or if InteropModeShared is in effect // (which prevents us from caching file contents), or if the file was - // opened with O_DIRECT, write directly to dentry.writeHandleLocked() + // opened with O_DIRECT, write directly to dentry.writeHandle() // without locking dentry.dataMu. rw.d.handleMu.RLock() - h := rw.d.writeHandleLocked() + h := rw.d.writeHandle() if (rw.d.mmapFD.RacyLoad() >= 0 && !rw.d.fs.opts.forcePageCache) || rw.d.fs.opts.interop == InteropModeShared || rw.direct { n, err := h.writeFromBlocksAt(rw.ctx, srcs, rw.off) rw.off += n @@ -609,7 +597,7 @@ func (d *dentry) writeback(ctx context.Context, offset, size int64) error { } d.handleMu.RLock() defer d.handleMu.RUnlock() - h := d.writeHandleLocked() + h := d.writeHandle() d.dataMu.Lock() defer d.dataMu.Unlock() // Compute the range of valid bytes (overflow-checked). @@ -649,7 +637,7 @@ func regularFileSeekLocked(ctx context.Context, d *dentry, fdOffset, offset int6 case linux.SEEK_END, linux.SEEK_DATA, linux.SEEK_HOLE: // Ensure file size is up to date. if !d.cachedMetadataAuthoritative() { - if err := d.updateMetadata(ctx, nil); err != nil { + if err := d.updateMetadata(ctx); err != nil { return 0, err } } @@ -809,7 +797,7 @@ func (d *dentry) Translate(ctx context.Context, required, optional memmap.Mappab } mf := d.fs.mfp.MemoryFile() - h := d.readHandleLocked() + h := d.readHandle() _, cerr := d.cache.Fill(ctx, required, maxFillRange(required, optional), d.size.Load(), mf, usage.PageCache, true /* populate */, h.readToBlocksAt) var ts []memmap.Translation @@ -881,7 +869,7 @@ func (d *dentry) InvalidateUnsavable(ctx context.Context) error { mf := d.fs.mfp.MemoryFile() d.handleMu.RLock() defer d.handleMu.RUnlock() - h := d.writeHandleLocked() + h := d.writeHandle() d.dataMu.Lock() defer d.dataMu.Unlock() if err := fsutil.SyncDirtyAll(ctx, &d.cache, &d.dirty, d.size.Load(), mf, h.writeFromBlocksAt); err != nil { @@ -905,7 +893,7 @@ func (d *dentry) Evict(ctx context.Context, er pgalloc.EvictableRange) { defer d.mapsMu.Unlock() d.handleMu.RLock() defer d.handleMu.RUnlock() - h := d.writeHandleLocked() + h := d.writeHandle() d.dataMu.Lock() defer d.dataMu.Unlock() diff --git a/pkg/sentry/fsimpl/gofer/revalidate.go b/pkg/sentry/fsimpl/gofer/revalidate.go index e22075a10..82cb62a6b 100644 --- a/pkg/sentry/fsimpl/gofer/revalidate.go +++ b/pkg/sentry/fsimpl/gofer/revalidate.go @@ -15,6 +15,7 @@ package gofer import ( + "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/sentry/vfs" "gvisor.dev/gvisor/pkg/sync" @@ -235,27 +236,57 @@ func (fs *filesystem) revalidateHelper(ctx context.Context, vfsObj *vfs.VirtualF // Lock metadata on all dentries *before* getting attributes for them. state.lockAllMetadata() - stats, err := state.start.controlFDLisa.WalkStat(ctx, state.names) - if err != nil { - return err + var stats []linux.Statx + switch dt := state.start.impl.(type) { + case *lisafsDentry: + var err error + stats, err = dt.controlFD.WalkStat(ctx, state.names) + if err != nil { + return err + } + default: + panic("unknown dentry implementation") } i := -1 for d := state.popFront(); d != nil; d = state.popFront() { i++ - found := i < len(stats) + var found bool + switch state.start.impl.(type) { + case *lisafsDentry: + found = i < len(stats) + default: + panic("unknown dentry implementation") + } if i == 0 && len(state.names[0]) == 0 { if found && !d.isSynthetic() { // First dentry is where the search is starting, just update attributes // since it cannot be replaced. - d.updateMetadataFromStatLocked(&stats[i]) // +checklocksforce: acquired by lockAllMetadata. + switch dt := d.impl.(type) { + case *lisafsDentry: + dt.updateMetadataFromStatxLocked(&stats[i]) // +checklocksforce: acquired by lockAllMetadata. + default: + panic("unknown dentry implementation") + } } d.metadataMu.Unlock() // +checklocksforce: see above. continue } + var fileChanged bool + if found { + switch d.impl.(type) { + case *lisafsDentry: + fileChanged = d.inoKey != inoKeyFromStatx(&stats[i]) + case nil: + // A remote file was found to replace this synthetic file. + fileChanged = true + default: + panic("unknown dentry implementation") + } + } // Note that synthetic dentries will always fail this comparison check. - if !found || d.inoKey != inoKeyFromStat(&stats[i]) { + if !found || fileChanged { d.metadataMu.Unlock() // +checklocksforce: see above. if !found && d.isSynthetic() { // We have a synthetic file, and no remote file has arisen to replace @@ -306,7 +337,12 @@ func (fs *filesystem) revalidateHelper(ctx context.Context, vfsObj *vfs.VirtualF } // The file at this path hasn't changed. Just update cached metadata. - d.updateMetadataFromStatLocked(&stats[i]) // +checklocksforce: see above. + switch dt := d.impl.(type) { + case *lisafsDentry: + dt.updateMetadataFromStatxLocked(&stats[i]) // +checklocksforce: see above. + default: + panic("unknown dentry implementation") + } d.metadataMu.Unlock() } diff --git a/pkg/sentry/fsimpl/gofer/save_restore.go b/pkg/sentry/fsimpl/gofer/save_restore.go index d1751304d..cd2e62797 100644 --- a/pkg/sentry/fsimpl/gofer/save_restore.go +++ b/pkg/sentry/fsimpl/gofer/save_restore.go @@ -24,7 +24,6 @@ import ( "gvisor.dev/gvisor/pkg/errors/linuxerr" "gvisor.dev/gvisor/pkg/fdnotifier" "gvisor.dev/gvisor/pkg/hostarch" - "gvisor.dev/gvisor/pkg/lisafs" "gvisor.dev/gvisor/pkg/refs" "gvisor.dev/gvisor/pkg/safemem" "gvisor.dev/gvisor/pkg/sentry/vfs" @@ -108,14 +107,14 @@ func (d *dentry) prepareSaveRecursive(ctx context.Context) error { if d.isRegularFile() && !d.cachedMetadataAuthoritative() { // Get updated metadata for d in case we need to perform metadata // validation during restore. - if err := d.updateMetadata(ctx, nil); err != nil { + if err := d.updateMetadata(ctx); err != nil { return err } } - if d.readFDLisa.Ok() || d.writeFDLisa.Ok() { + if d.isReadHandleOk() || d.isWriteHandleOk() { d.fs.savedDentryRW[d] = savedDentryRW{ - read: d.readFDLisa.Ok(), - write: d.writeFDLisa.Ok(), + read: d.isReadHandleOk(), + write: d.isWriteHandleOk(), } } d.dirMu.Lock() @@ -179,11 +178,7 @@ func (fs *filesystem) CompleteRestore(ctx context.Context, opts vfs.CompleteRest fs.opts.fd = fd fs.inoByKey = make(map[inoKey]uint64) - rootInode, err := fs.initClient(ctx) - if err != nil { - return err - } - if err := fs.root.restoreFile(ctx, &rootInode, &opts); err != nil { + if err := fs.restoreRoot(ctx, &opts); err != nil { return err } @@ -223,90 +218,28 @@ func (fs *filesystem) CompleteRestore(ctx context.Context, opts vfs.CompleteRest return nil } -func (d *dentry) restoreFile(ctx context.Context, inode *lisafs.Inode, opts *vfs.CompleteRestoreOptions) error { - d.controlFDLisa = d.fs.client.NewFD(inode.ControlFD) - - // Gofers do not preserve inoKey across checkpoint/restore, so: - // - // - We must assume that the remote filesystem did not change in a way that - // would invalidate dentries, since we can't revalidate dentries by - // checking inoKey. - // - // - We need to associate the new inoKey with the existing d.ino. - d.inoKey = inoKeyFromStat(&inode.Stat) - d.fs.inoMu.Lock() - d.fs.inoByKey[d.inoKey] = d.ino - d.fs.inoMu.Unlock() - - // Check metadata stability before updating metadata. - d.metadataMu.Lock() - defer d.metadataMu.Unlock() - if d.isRegularFile() { - if opts.ValidateFileSizes { - if inode.Stat.Mask&linux.STATX_SIZE == 0 { - return vfs.ErrCorruption{fmt.Errorf("gofer.dentry(%q).restoreFile: file size validation failed: file size not available", genericDebugPathname(d))} - } - if d.size.RacyLoad() != inode.Stat.Size { - return vfs.ErrCorruption{fmt.Errorf("gofer.dentry(%q).restoreFile: file size validation failed: size changed from %d to %d", genericDebugPathname(d), d.size.Load(), inode.Stat.Size)} - } - } - if opts.ValidateFileModificationTimestamps { - if inode.Stat.Mask&linux.STATX_MTIME != 0 { - return vfs.ErrCorruption{fmt.Errorf("gofer.dentry(%q).restoreFile: mtime validation failed: mtime not available", genericDebugPathname(d))} - } - if want := dentryTimestamp(inode.Stat.Mtime); d.mtime.RacyLoad() != want { - return vfs.ErrCorruption{fmt.Errorf("gofer.dentry(%q).restoreFile: mtime validation failed: mtime changed from %+v to %+v", genericDebugPathname(d), linux.NsecToStatxTimestamp(d.mtime.RacyLoad()), linux.NsecToStatxTimestamp(want))} - } - } - } - if !d.cachedMetadataAuthoritative() { - d.updateMetadataFromStatLocked(&inode.Stat) - } - - if rw, ok := d.fs.savedDentryRW[d]; ok { - if err := d.ensureSharedHandle(ctx, rw.read, rw.write, false /* trunc */); err != nil { - return err - } - } - - return nil -} - // Preconditions: d is not synthetic. func (d *dentry) restoreDescendantsRecursive(ctx context.Context, opts *vfs.CompleteRestoreOptions) error { for _, child := range d.children { if child == nil { continue } - // child is synthetic if it does not exist in fs.syncableDentries. - if child.syncableListEntry.Next() == nil && child.syncableListEntry.Prev() == nil && d.fs.syncableDentries.Front() != &child.syncableListEntry { + if child.isSynthetic() { continue } - if err := child.restoreRecursive(ctx, opts); err != nil { + if err := child.restoreFile(ctx, opts); err != nil { + return err + } + if err := child.restoreDescendantsRecursive(ctx, opts); err != nil { return err } } return nil } -// Preconditions: d is not synthetic (but note that since this function -// restores d.file, d.file.isNil() is always true at this point, so this can -// only be detected by checking filesystem.syncableDentries). d.parent has been -// restored. -func (d *dentry) restoreRecursive(ctx context.Context, opts *vfs.CompleteRestoreOptions) error { - inode, err := d.parent.controlFDLisa.Walk(ctx, d.name) - if err != nil { - return err - } - if err := d.restoreFile(ctx, &inode, opts); err != nil { - return err - } - return d.restoreDescendantsRecursive(ctx, opts) -} - func (fd *specialFileFD) completeRestore(ctx context.Context) error { d := fd.dentry() - h, err := openHandle(ctx, d.controlFDLisa, fd.vfsfd.IsReadable(), fd.vfsfd.IsWritable(), false /* trunc */) + h, err := d.openHandle(ctx, fd.vfsfd.IsReadable(), fd.vfsfd.IsWritable(), false /* trunc */) if err != nil { return err } diff --git a/pkg/sentry/fsimpl/gofer/socket.go b/pkg/sentry/fsimpl/gofer/socket.go index 3d47719da..4b7012b30 100644 --- a/pkg/sentry/fsimpl/gofer/socket.go +++ b/pkg/sentry/fsimpl/gofer/socket.go @@ -36,7 +36,8 @@ func (d *dentry) isSocket() bool { // // +stateify savable type endpoint struct { - // dentry is the filesystem dentry which produced this endpoint. + // dentry is the filesystem dentry which produced this endpoint. dentry is + // not synthetic. dentry *dentry // path is the sentry path where this endpoint is bound. @@ -93,7 +94,7 @@ func (e *endpoint) UnidirectionalConnect(ctx context.Context) (transport.Connect } func (e *endpoint) newConnectedEndpoint(ctx context.Context, sockType linux.SockType, queue *waiter.Queue) (*transport.SCMConnectedEndpoint, *syserr.Error) { - hostSockFD, err := e.dentry.controlFDLisa.Connect(ctx, sockType) + hostSockFD, err := e.dentry.connect(ctx, sockType) if err != nil { return nil, syserr.ErrConnectionRefused } diff --git a/pkg/sentry/fsimpl/gofer/special_file.go b/pkg/sentry/fsimpl/gofer/special_file.go index 73ad148e2..abba77ffd 100644 --- a/pkg/sentry/fsimpl/gofer/special_file.go +++ b/pkg/sentry/fsimpl/gofer/special_file.go @@ -17,7 +17,6 @@ package gofer import ( "fmt" - "golang.org/x/sys/unix" "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/context" @@ -150,7 +149,7 @@ func (fd *specialFileFD) OnClose(ctx context.Context) error { if !fd.vfsfd.IsWritable() { return nil } - return fd.handle.fdLisa.Flush(ctx) + return flush(ctx, fd.handle.fdLisa) } // Readiness implements waiter.Waitable.Readiness. @@ -198,7 +197,7 @@ func (fd *specialFileFD) Allocate(ctx context.Context, mode, offset, length uint if fd.isRegularFile { d := fd.dentry() return d.doAllocate(ctx, offset, length, func() error { - return fd.handle.fdLisa.Allocate(ctx, mode, offset, length) + return fd.handle.allocate(ctx, mode, offset, length) }) } return fd.FileDescriptionDefaultImpl.Allocate(ctx, mode, offset, length) @@ -304,7 +303,7 @@ func (fd *specialFileFD) pwrite(ctx context.Context, src usermem.IOSequence, off // size is updated. There is a possible race here if size is modified // externally after metadata cache is updated. if fd.vfsfd.StatusFlags()&linux.O_APPEND != 0 && !d.cachedMetadataAuthoritative() { - if err := d.updateMetadata(ctx, nil); err != nil { + if err := d.updateMetadata(ctx); err != nil { return 0, offset, err } } @@ -416,21 +415,7 @@ func (fd *specialFileFD) sync(ctx context.Context, forFilesystemSync bool) error fd.releaseMu.RLock() defer fd.releaseMu.RUnlock() - if !fd.handle.isOpen() { - return nil - } - err := func() error { - // If we have a host FD, fsyncing it is likely to be faster than an fsync - // RPC. - if fd.handle.fd >= 0 { - ctx.UninterruptibleSleepStart(false) - err := unix.Fsync(int(fd.handle.fd)) - ctx.UninterruptibleSleepFinish(false) - return err - } - return fd.handle.fdLisa.Sync(ctx) - }() - if err != nil { + if err := fd.handle.sync(ctx); err != nil { if !forFilesystemSync { return err } @@ -533,3 +518,10 @@ func (fd *specialFileFD) requireHostFD() { panic("gofer.specialFileFD can no longer be memory-mapped without a host FD") } } + +func (fd *specialFileFD) updateMetadata(ctx context.Context) error { + d := fd.dentry() + d.metadataMu.Lock() + defer d.metadataMu.Unlock() + return d.updateMetadataLocked(ctx, fd.handle) +} diff --git a/pkg/sentry/fsimpl/gofer/symlink.go b/pkg/sentry/fsimpl/gofer/symlink.go index c4f095a8e..c446f1629 100644 --- a/pkg/sentry/fsimpl/gofer/symlink.go +++ b/pkg/sentry/fsimpl/gofer/symlink.go @@ -35,7 +35,7 @@ func (d *dentry) readlink(ctx context.Context, mnt *vfs.Mount) (string, error) { return target, nil } } - target, err := d.controlFDLisa.ReadLinkAt(ctx) + target, err := d.readlinkImpl(ctx) if d.fs.opts.interop != InteropModeShared { if err == nil { d.haveTarget = true