cephfs: add additional error handling and state checks to file funcs

Now, the close function is idempotent wrt being called multiple times
on a valid file. Additionally, check the state of the file in various
functions to produce sensible errors, matching those cephfs itself
returns, if the object was not constructed properly.

Signed-off-by: John Mulligan <jmulligan@redhat.com>

cephfs/errors.go

@@ -34,6 +34,14 @@ func getError(e C.int) error {
 	return CephFSError(e)
 }
 
+// Public go errors:
+
+const (
+	// ErrNotConnected may be returned when client is not connected
+	// to a cluster.
+	ErrNotConnected = CephFSError(-C.ENOTCONN)
+)
+
 // Private errors:
 
 const (

cephfs/file.go

@@ -34,6 +34,9 @@ type File struct {
 // Implements:
 //  int ceph_open(struct ceph_mount_info *cmount, const char *path, int flags, mode_t mode);
 func (mount *MountInfo) Open(path string, flags int, mode uint32) (*File, error) {
+	if mount.mount == nil {
+		return nil, ErrNotConnected
+	}
 	cPath := C.CString(path)
 	defer C.free(unsafe.Pointer(cPath))
 	ret := C.ceph_open(mount.mount, cPath, C.int(flags), C.mode_t(mode))
@@ -43,12 +46,30 @@ func (mount *MountInfo) Open(path string, flags int, mode uint32) (*File, error)
 	return &File{mount: mount, fd: ret}, nil
 }
 
+func (f *File) validate() error {
+	if f.mount == nil {
+		return ErrNotConnected
+	}
+	return nil
+}
+
 // Close the file.
 //
 // Implements:
 //  int ceph_close(struct ceph_mount_info *cmount, int fd);
 func (f *File) Close() error {
-	return getError(C.ceph_close(f.mount.mount, f.fd))
+	if f.fd == -1 {
+		// already closed
+		return nil
+	}
+	if err := f.validate(); err != nil {
+		return err
+	}
+	if err := getError(C.ceph_close(f.mount.mount, f.fd)); err != nil {
+		return err
+	}
+	f.fd = -1
+	return nil
 }
 
 // read directly wraps the ceph_read call. Because read is such a common
@@ -58,6 +79,9 @@ func (f *File) Close() error {
 // Implements:
 //  int ceph_read(struct ceph_mount_info *cmount, int fd, char *buf, int64_t size, int64_t offset);
 func (f *File) read(buf []byte, offset int64) (int, error) {
+	if err := f.validate(); err != nil {
+		return 0, err
+	}
 	bufptr := (*C.char)(unsafe.Pointer(&buf[0]))
 	ret := C.ceph_read(
 		f.mount.mount, f.fd, bufptr, C.int64_t(len(buf)), C.int64_t(offset))
@@ -90,6 +114,9 @@ func (f *File) ReadAt(buf []byte, offset int64) (int, error) {
 //  int ceph_write(struct ceph_mount_info *cmount, int fd, const char *buf,
 //                 int64_t size, int64_t offset);
 func (f *File) write(buf []byte, offset int64) (int, error) {
+	if err := f.validate(); err != nil {
+		return 0, err
+	}
 	bufptr := (*C.char)(unsafe.Pointer(&buf[0]))
 	ret := C.ceph_write(
 		f.mount.mount, f.fd, bufptr, C.int64_t(len(buf)), C.int64_t(offset))
@@ -116,6 +143,9 @@ func (f *File) WriteAt(buf []byte, offset int64) (int, error) {
 // Implements:
 //  int64_t ceph_lseek(struct ceph_mount_info *cmount, int fd, int64_t offset, int whence);
 func (f *File) Seek(offset int64, whence int) (int64, error) {
+	if err := f.validate(); err != nil {
+		return 0, err
+	}
 	// validate the seek whence value in case the caller skews
 	// from the seek values we technically support from C as documented.
 	// TODO: need to support seek-(hole|data) in mimic and later.

cephfs/file_test.go

@@ -45,6 +45,34 @@ func TestFileOpen(t *testing.T) {
 		assert.NoError(t, err)
 		assert.EqualValues(t, 0, s.Size())
 	})
+
+	t.Run("idempotentClose", func(t *testing.T) {
+		f1, err := mount.Open(fname, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0666)
+		assert.NoError(t, err)
+		assert.NotNil(t, f1)
+		assert.NoError(t, f1.Close())
+		assert.NoError(t, f1.Close()) // call close again. it should not fail
+		defer func() { assert.NoError(t, mount.Unlink(fname)) }()
+	})
+
+	t.Run("uninitializedFileClose", func(t *testing.T) {
+		f := &File{}
+		err := f.Close()
+		assert.Error(t, err)
+		assert.Equal(t, ErrNotConnected, err)
+	})
+
+	t.Run("invalidFdClose", func(t *testing.T) {
+		f := &File{mount, 1980}
+		err := f.Close()
+		assert.Error(t, err)
+	})
+
+	t.Run("openInvalidMount", func(t *testing.T) {
+		m := &MountInfo{}
+		_, err := m.Open(fname, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0666)
+		assert.Error(t, err)
+	})
 }
 
 func TestFileReadWrite(t *testing.T) {
@@ -91,6 +119,17 @@ func TestFileReadWrite(t *testing.T) {
 		_, err = f1.Write([]byte("yo"))
 		assert.Error(t, err)
 	})
+
+	t.Run("uninitializedFile", func(t *testing.T) {
+		f := &File{}
+		b := []byte("testme")
+		_, err := f.Write(b)
+		assert.Error(t, err)
+		assert.Equal(t, ErrNotConnected, err)
+		_, err = f.Read(b)
+		assert.Error(t, err)
+		assert.Equal(t, ErrNotConnected, err)
+	})
 }
 
 func TestFileReadWriteAt(t *testing.T) {
@@ -145,6 +184,17 @@ func TestFileReadWriteAt(t *testing.T) {
 		_, err = f1.WriteAt([]byte("yo"), 0)
 		assert.Error(t, err)
 	})
+
+	t.Run("uninitializedFile", func(t *testing.T) {
+		f := &File{}
+		b := []byte("testme")
+		_, err := f.WriteAt(b, 0)
+		assert.Error(t, err)
+		assert.Equal(t, ErrNotConnected, err)
+		_, err = f.ReadAt(b, 0)
+		assert.Error(t, err)
+		assert.Equal(t, ErrNotConnected, err)
+	})
 }
 
 func TestFileInterfaces(t *testing.T) {
@@ -221,4 +271,11 @@ func TestFileSeek(t *testing.T) {
 		assert.Error(t, err)
 		assert.EqualValues(t, 0, o)
 	})
+
+	t.Run("uninitializedFile", func(t *testing.T) {
+		f := &File{}
+		_, err := f.Seek(0, SeekSet)
+		assert.Error(t, err)
+		assert.Equal(t, ErrNotConnected, err)
+	})
 }