12483: Support O_APPEND. Check for invalid/unsupported file modes.

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg@veritasgenetics.com>

diff --git a/sdk/go/arvados/collection_fs.go b/sdk/go/arvados/collection_fs.go
index 328d68f8975fc51e3253c5c0ac5f892e949b912d..3cae9cbbb5d7bc2d28f5f4d647b59ba61867878b 100644 (file)
--- a/sdk/go/arvados/collection_fs.go
+++ b/sdk/go/arvados/collection_fs.go
@@ -25,6 +25,8 @@ var (
        ErrFileExists        = errors.New("file exists")
        ErrInvalidOperation  = errors.New("invalid operation")
        ErrDirectoryNotEmpty = errors.New("directory not empty")
+       ErrWriteOnlyMode     = errors.New("file is O_WRONLY")
+       ErrSyncNotSupported  = errors.New("O_SYNC flag is not supported")
        ErrPermission        = os.ErrPermission
 
        maxBlockSize = 1 << 26
@@ -231,8 +233,6 @@ func (fn *filenode) Readdir() []os.FileInfo {
 }
 
 func (fn *filenode) Read(p []byte, startPtr filenodePtr) (n int, ptr filenodePtr, err error) {
-       fn.RLock()
-       defer fn.RUnlock()
        ptr = fn.seek(startPtr)
        if ptr.off < 0 {
                err = ErrNegativeOffset
@@ -319,8 +319,6 @@ func (fn *filenode) Truncate(size int64) error {
 }
 
 func (fn *filenode) Write(p []byte, startPtr filenodePtr) (n int, ptr filenodePtr, err error) {
-       fn.Lock()
-       defer fn.Unlock()
        ptr = fn.seek(startPtr)
        if ptr.off < 0 {
                err = ErrNegativeOffset
@@ -497,11 +495,17 @@ type file struct {
        inode
        ptr        filenodePtr
        append     bool
+       readable   bool
        writable   bool
        unreaddirs []os.FileInfo
 }
 
 func (f *file) Read(p []byte) (n int, err error) {
+       if !f.readable {
+               return 0, ErrWriteOnlyMode
+       }
+       f.inode.RLock()
+       defer f.inode.RUnlock()
        n, f.ptr, err = f.inode.Read(p, f.ptr)
        return
 }
@@ -540,6 +544,16 @@ func (f *file) Write(p []byte) (n int, err error) {
        if !f.writable {
                return 0, ErrReadOnlyFile
        }
+       f.inode.Lock()
+       defer f.inode.Unlock()
+       if fn, ok := f.inode.(*filenode); ok && f.append {
+               f.ptr = filenodePtr{
+                       off:       fn.fileinfo.size,
+                       extentIdx: len(fn.extents),
+                       extentOff: 0,
+                       repacked:  fn.repacked,
+               }
+       }
        n, f.ptr, err = f.inode.Write(p, f.ptr)
        return
 }
@@ -971,13 +985,27 @@ func (dn *dirnode) lookupPath(path string) (node inode) {
 }
 
 func (dn *dirnode) OpenFile(name string, flag int, perm os.FileMode) (*file, error) {
+       if flag&os.O_SYNC != 0 {
+               return nil, ErrSyncNotSupported
+       }
        dirname, name := path.Split(name)
        dn, ok := dn.lookupPath(dirname).(*dirnode)
        if !ok {
                return nil, os.ErrNotExist
        }
-       writeMode := flag&(os.O_RDWR|os.O_WRONLY|os.O_CREATE) != 0
-       if !writeMode {
+       var readable, writable bool
+       switch flag & (os.O_RDWR | os.O_RDONLY | os.O_WRONLY) {
+       case os.O_RDWR:
+               readable = true
+               writable = true
+       case os.O_RDONLY:
+               readable = true
+       case os.O_WRONLY:
+               writable = true
+       default:
+               return nil, fmt.Errorf("invalid flags 0x%x", flag)
+       }
+       if !writable {
                // A directory can be opened via "foo/", "foo/.", or
                // "foo/..".
                switch name {
@@ -1030,7 +1058,8 @@ func (dn *dirnode) OpenFile(name string, flag int, perm os.FileMode) (*file, err
        return &file{
                inode:    n,
                append:   flag&os.O_APPEND != 0,
-               writable: flag&(os.O_WRONLY|os.O_RDWR) != 0,
+               readable: readable,
+               writable: writable,
        }, nil
 }
 

diff --git a/sdk/go/arvados/collection_fs_test.go b/sdk/go/arvados/collection_fs_test.go
index ecb1f8928003e3d6bc891f6bbea50e97e0891c09..324ece11ceff3ddd9ccdc578fe518bf5a229cf80 100644 (file)
--- a/sdk/go/arvados/collection_fs_test.go
+++ b/sdk/go/arvados/collection_fs_test.go
@@ -626,6 +626,87 @@ func (s *CollectionFSSuite) TestPersistEmptyFiles(c *check.C) {
        }
 }
 
+func (s *CollectionFSSuite) TestFileModes(c *check.C) {
+       fs, err := (&Collection{}).FileSystem(s.client, s.kc)
+       c.Assert(err, check.IsNil)
+
+       f, err := fs.OpenFile("missing", os.O_WRONLY, 0)
+       c.Check(f, check.IsNil)
+       c.Check(err, check.ErrorMatches, `file does not exist`)
+
+       f, err = fs.OpenFile("new", os.O_CREATE|os.O_RDONLY, 0)
+       c.Assert(err, check.IsNil)
+       defer f.Close()
+       n, err := f.Write([]byte{1, 2, 3})
+       c.Check(n, check.Equals, 0)
+       c.Check(err, check.ErrorMatches, `read-only file`)
+       n, err = f.Read(make([]byte, 1))
+       c.Check(err, check.Equals, io.EOF)
+       f, err = fs.OpenFile("new", os.O_RDWR, 0)
+       c.Assert(err, check.IsNil)
+       defer f.Close()
+       _, err = f.Write([]byte{4, 5, 6})
+       c.Check(err, check.IsNil)
+       fs.Remove("new")
+
+       buf := make([]byte, 64)
+       f, err = fs.OpenFile("append", os.O_EXCL|os.O_CREATE|os.O_RDWR|os.O_APPEND, 0)
+       c.Assert(err, check.IsNil)
+       f.Write([]byte{1, 2, 3})
+       f.Seek(0, os.SEEK_SET)
+       n, _ = f.Read(buf[:1])
+       c.Check(n, check.Equals, 1)
+       c.Check(buf[:1], check.DeepEquals, []byte{1})
+       pos, err := f.Seek(0, os.SEEK_CUR)
+       c.Check(pos, check.Equals, int64(1))
+       f.Write([]byte{4, 5, 6})
+       pos, err = f.Seek(0, os.SEEK_CUR)
+       c.Check(pos, check.Equals, int64(6))
+       f.Seek(0, os.SEEK_SET)
+       n, err = f.Read(buf)
+       c.Check(buf[:n], check.DeepEquals, []byte{1, 2, 3, 4, 5, 6})
+       c.Check(err, check.Equals, io.EOF)
+       f.Close()
+
+       f, err = fs.OpenFile("append", os.O_RDWR|os.O_APPEND, 0)
+       c.Assert(err, check.IsNil)
+       pos, err = f.Seek(0, os.SEEK_CUR)
+       c.Check(pos, check.Equals, int64(0))
+       c.Check(err, check.IsNil)
+       f.Read(buf[:3])
+       pos, _ = f.Seek(0, os.SEEK_CUR)
+       c.Check(pos, check.Equals, int64(3))
+       f.Write([]byte{7, 8, 9})
+       pos, err = f.Seek(0, os.SEEK_CUR)
+       c.Check(pos, check.Equals, int64(9))
+       f.Close()
+
+       f, err = fs.OpenFile("wronly", os.O_CREATE|os.O_WRONLY, 0)
+       c.Assert(err, check.IsNil)
+       n, err = f.Write([]byte{3, 2, 1})
+       c.Check(n, check.Equals, 3)
+       c.Check(err, check.IsNil)
+       pos, _ = f.Seek(0, os.SEEK_CUR)
+       c.Check(pos, check.Equals, int64(3))
+       pos, _ = f.Seek(0, os.SEEK_SET)
+       c.Check(pos, check.Equals, int64(0))
+       n, err = f.Read(buf)
+       c.Check(n, check.Equals, 0)
+       c.Check(err, check.ErrorMatches, `.*O_WRONLY.*`)
+       f, err = fs.OpenFile("wronly", os.O_RDONLY, 0)
+       c.Assert(err, check.IsNil)
+       n, _ = f.Read(buf)
+       c.Check(buf[:n], check.DeepEquals, []byte{3, 2, 1})
+
+       f, err = fs.OpenFile("unsupported", os.O_CREATE|os.O_SYNC, 0)
+       c.Check(f, check.IsNil)
+       c.Check(err, check.NotNil)
+
+       f, err = fs.OpenFile("append", os.O_RDWR|os.O_WRONLY, 0)
+       c.Check(f, check.IsNil)
+       c.Check(err, check.ErrorMatches, `invalid flag.*`)
+}
+
 func (s *CollectionFSSuite) TestFlushFullBlocks(c *check.C) {
        maxBlockSize = 1024
        defer func() { maxBlockSize = 2 << 26 }()