Add O_APPEND support in vfs2 gofer.

Helps in fixing open syscall tests: AppendConcurrentWrite and AppendOnly.

We also now update the file size for seekable special files (regular files)
which we were not doing earlier.

Updates #2923

PiperOrigin-RevId: 322670843
This commit is contained in:
Ayush Ranjan
2020-07-22 15:42:33 -07:00
committed by gVisor bot
parent 71bf90c55b
commit 39525d64cb
3 changed files with 86 additions and 21 deletions
+37 -10
View File
@@ -155,26 +155,53 @@ func (fd *regularFileFD) Read(ctx context.Context, dst usermem.IOSequence, opts
// PWrite implements vfs.FileDescriptionImpl.PWrite.
func (fd *regularFileFD) PWrite(ctx context.Context, src usermem.IOSequence, offset int64, opts vfs.WriteOptions) (int64, error) {
n, _, err := fd.pwrite(ctx, src, offset, opts)
return n, err
}
// pwrite returns the number of bytes written, final offset, error. The final
// offset should be ignored by PWrite.
func (fd *regularFileFD) pwrite(ctx context.Context, src usermem.IOSequence, offset int64, opts vfs.WriteOptions) (written, finalOff int64, err error) {
if offset < 0 {
return 0, syserror.EINVAL
return 0, offset, syserror.EINVAL
}
// Check that flags are supported.
//
// TODO(gvisor.dev/issue/2601): Support select pwritev2 flags.
if opts.Flags&^linux.RWF_HIPRI != 0 {
return 0, syserror.EOPNOTSUPP
return 0, offset, syserror.EOPNOTSUPP
}
limit, err := vfs.CheckLimit(ctx, offset, src.NumBytes())
if err != nil {
return 0, err
}
src = src.TakeFirst64(limit)
d := fd.dentry()
// If the fd was opened with O_APPEND, make sure the file 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.updateFromGetattr(ctx); err != nil {
return 0, offset, err
}
}
d.metadataMu.Lock()
defer d.metadataMu.Unlock()
// Set offset to file size if the fd was opened with O_APPEND.
if fd.vfsfd.StatusFlags()&linux.O_APPEND != 0 {
// Holding d.metadataMu is sufficient for reading d.size.
offset = int64(d.size)
}
limit, err := vfs.CheckLimit(ctx, offset, src.NumBytes())
if err != nil {
return 0, offset, err
}
src = src.TakeFirst64(limit)
n, err := fd.pwriteLocked(ctx, src, offset, opts)
return n, offset + n, err
}
// Preconditions: fd.dentry().metatdataMu must be locked.
func (fd *regularFileFD) pwriteLocked(ctx context.Context, src usermem.IOSequence, offset int64, opts vfs.WriteOptions) (int64, error) {
d := fd.dentry()
if d.fs.opts.interop != InteropModeShared {
// Compare Linux's mm/filemap.c:__generic_file_write_iter() =>
// file_update_time(). This is d.touchCMtime(), but without locking
@@ -237,8 +264,8 @@ func (fd *regularFileFD) PWrite(ctx context.Context, src usermem.IOSequence, off
// Write implements vfs.FileDescriptionImpl.Write.
func (fd *regularFileFD) Write(ctx context.Context, src usermem.IOSequence, opts vfs.WriteOptions) (int64, error) {
fd.mu.Lock()
n, err := fd.PWrite(ctx, src, fd.off, opts)
fd.off += n
n, off, err := fd.pwrite(ctx, src, fd.off, opts)
fd.off = off
fd.mu.Unlock()
return n, err
}
+47 -9
View File
@@ -16,6 +16,7 @@ package gofer
import (
"sync"
"sync/atomic"
"gvisor.dev/gvisor/pkg/abi/linux"
"gvisor.dev/gvisor/pkg/context"
@@ -144,7 +145,7 @@ func (fd *specialFileFD) PRead(ctx context.Context, dst usermem.IOSequence, offs
// mmap due to lock ordering; MM locks precede dentry.dataMu. That doesn't
// hold here since specialFileFD doesn't client-cache data. Just buffer the
// read instead.
if d := fd.dentry(); d.fs.opts.interop != InteropModeShared {
if d := fd.dentry(); d.cachedMetadataAuthoritative() {
d.touchAtime(fd.vfsfd.Mount())
}
buf := make([]byte, dst.NumBytes())
@@ -176,39 +177,76 @@ func (fd *specialFileFD) Read(ctx context.Context, dst usermem.IOSequence, opts
// PWrite implements vfs.FileDescriptionImpl.PWrite.
func (fd *specialFileFD) PWrite(ctx context.Context, src usermem.IOSequence, offset int64, opts vfs.WriteOptions) (int64, error) {
n, _, err := fd.pwrite(ctx, src, offset, opts)
return n, err
}
// pwrite returns the number of bytes written, final offset, error. The final
// offset should be ignored by PWrite.
func (fd *specialFileFD) pwrite(ctx context.Context, src usermem.IOSequence, offset int64, opts vfs.WriteOptions) (written, finalOff int64, err error) {
if fd.seekable && offset < 0 {
return 0, syserror.EINVAL
return 0, offset, syserror.EINVAL
}
// Check that flags are supported.
//
// TODO(gvisor.dev/issue/2601): Support select pwritev2 flags.
if opts.Flags&^linux.RWF_HIPRI != 0 {
return 0, syserror.EOPNOTSUPP
return 0, offset, syserror.EOPNOTSUPP
}
d := fd.dentry()
// If the regular file fd was opened with O_APPEND, make sure the file size
// is updated. There is a possible race here if size is modified externally
// after metadata cache is updated.
if fd.seekable && fd.vfsfd.StatusFlags()&linux.O_APPEND != 0 && !d.cachedMetadataAuthoritative() {
if err := d.updateFromGetattr(ctx); err != nil {
return 0, offset, err
}
}
if fd.seekable {
// We need to hold the metadataMu *while* writing to a regular file.
d.metadataMu.Lock()
defer d.metadataMu.Unlock()
// Set offset to file size if the regular file was opened with O_APPEND.
if fd.vfsfd.StatusFlags()&linux.O_APPEND != 0 {
// Holding d.metadataMu is sufficient for reading d.size.
offset = int64(d.size)
}
limit, err := vfs.CheckLimit(ctx, offset, src.NumBytes())
if err != nil {
return 0, err
return 0, offset, err
}
src = src.TakeFirst64(limit)
}
// Do a buffered write. See rationale in PRead.
if d := fd.dentry(); d.fs.opts.interop != InteropModeShared {
if d.cachedMetadataAuthoritative() {
d.touchCMtime()
}
buf := make([]byte, src.NumBytes())
// Don't do partial writes if we get a partial read from src.
if _, err := src.CopyIn(ctx, buf); err != nil {
return 0, err
return 0, offset, err
}
n, err := fd.handle.writeFromBlocksAt(ctx, safemem.BlockSeqOf(safemem.BlockFromSafeSlice(buf)), uint64(offset))
if err == syserror.EAGAIN {
err = syserror.ErrWouldBlock
}
return int64(n), err
finalOff = offset
// Update file size for regular files.
if fd.seekable {
finalOff += int64(n)
// d.metadataMu is already locked at this point.
if uint64(finalOff) > d.size {
d.dataMu.Lock()
defer d.dataMu.Unlock()
atomic.StoreUint64(&d.size, uint64(finalOff))
}
}
return int64(n), finalOff, err
}
// Write implements vfs.FileDescriptionImpl.Write.
@@ -218,8 +256,8 @@ func (fd *specialFileFD) Write(ctx context.Context, src usermem.IOSequence, opts
}
fd.mu.Lock()
n, err := fd.PWrite(ctx, src, fd.off, opts)
fd.off += n
n, off, err := fd.pwrite(ctx, src, fd.off, opts)
fd.off = off
fd.mu.Unlock()
return n, err
}
+2 -2
View File
@@ -235,7 +235,7 @@ TEST_F(OpenTest, AppendOnly) {
ASSERT_NO_ERRNO_AND_VALUE(Open(test_file_name_, O_RDWR | O_APPEND));
EXPECT_THAT(lseek(fd2.get(), 0, SEEK_CUR), SyscallSucceedsWithValue(0));
// Then try to write to the first file and make sure the bytes are appended.
// Then try to write to the first fd and make sure the bytes are appended.
EXPECT_THAT(WriteFd(fd1.get(), buf.data(), buf.size()),
SyscallSucceedsWithValue(buf.size()));
@@ -247,7 +247,7 @@ TEST_F(OpenTest, AppendOnly) {
EXPECT_THAT(lseek(fd1.get(), 0, SEEK_CUR),
SyscallSucceedsWithValue(kBufSize * 2));
// Then try to write to the second file and make sure the bytes are appended.
// Then try to write to the second fd and make sure the bytes are appended.
EXPECT_THAT(WriteFd(fd2.get(), buf.data(), buf.size()),
SyscallSucceedsWithValue(buf.size()));