Repository: ceph/go-ceph
Pull request #285 (2020-05-13 20:40:15)

cephfs: add ReadDirPlus function and associated types
Add the ReadDirPlus function call, wrapping ceph_readdirplus_r. Add
DirEntryPlus which is a DirEntry plus a getter for the statx field.
Add a test similar to the ReadDir test and (since it is possible to
induce an error with ceph_readdirplus_r) a test for the error handling
path to improve code coverage.

Fixes: #260 

## Checklist
- [x] Added tests for features and functional changes
- [x] Public functions and types are documented
- [x] Standard formatting is applied to Go code


phlogistonjohn: @agarwal-mudit  you may also be interested in looking at this.

@nixpanic I noticed nothing in directory.go has a validate()-type check function like we added for mount and file. I will be adding that in a follow up PR so no need say so. :-) 

Patch:
diff --git a/cephfs/directory.go b/cephfs/directory.go
index 28bd2c7..23b0ae6 100644
--- a/cephfs/directory.go
+++ b/cephfs/directory.go
@@ -99,6 +99,20 @@ func (d *DirEntry) DType() DType {
 	return d.dtype
 }
 
+// DirEntryPlus is a DirEntry plus additional data (stat) for an entry
+// within a directory.
+type DirEntryPlus struct {
+	DirEntry
+	// statx: the converted statx returned by ceph_readdirplus_r
+	statx *CephStatx
+}
+
+// Statx returns cached stat metadata for the directory entry.
+// This call does not incur an actual file system stat.
+func (d *DirEntryPlus) Statx() *CephStatx {
+	return d.statx
+}
+
 // toDirEntry converts a c struct dirent to our go wrapper.
 func toDirEntry(de *C.struct_dirent) *DirEntry {
 	return &DirEntry{
@@ -108,6 +122,15 @@ func toDirEntry(de *C.struct_dirent) *DirEntry {
 	}
 }
 
+// toDirEntryPlus converts c structs set by ceph_readdirplus_r to our go
+// wrapper.
+func toDirEntryPlus(de *C.struct_dirent, s C.struct_ceph_statx) *DirEntryPlus {
+	return &DirEntryPlus{
+		DirEntry: *toDirEntry(de),
+		statx:    cStructToCephStatx(s),
+	}
+}
+
 // ReadDir reads a single directory entry from the open Directory.
 // A nil DirEntry pointer will be returned when the Directory stream has been
 // exhausted.
@@ -126,6 +149,40 @@ func (dir *Directory) ReadDir() (*DirEntry, error) {
 	return toDirEntry(&de), nil
 }
 
+// ReadDirPlus reads a single directory entry and stat information from the
+// open Directory.
+// A nil DirEntryPlus pointer will be returned when the Directory stream has
+// been exhausted.
+// See Statx for a description of the wants and flags parameters.
+//
+// Implements:
+//  int ceph_readdirplus_r(struct ceph_mount_info *cmount, struct ceph_dir_result *dirp, struct dirent *de,
+//                         struct ceph_statx *stx, unsigned want, unsigned flags, struct Inode **out);
+func (dir *Directory) ReadDirPlus(
+	want StatxMask, flags AtFlags) (*DirEntryPlus, error) {
+
+	var (
+		de C.struct_dirent
+		s  C.struct_ceph_statx
+	)
+	ret := C.ceph_readdirplus_r(
+		dir.mount.mount,
+		dir.dir,
+		&de,
+		&s,
+		C.uint(want),
+		C.uint(flags),
+		nil, // unused, internal Inode type not needed for high level api
+	)
+	if ret < 0 {
+		return nil, getError(ret)
+	}
+	if ret == 0 {
+		return nil, nil // End-of-stream
+	}
+	return toDirEntryPlus(&de, s), nil
+}
+
 // RewindDir sets the directory stream to the beginning of the directory.
 //
 // Implements:


Test patch:
diff --git a/cephfs/directory_test.go b/cephfs/directory_test.go
index 76d0e7c..04b6531 100644
--- a/cephfs/directory_test.go
+++ b/cephfs/directory_test.go
@@ -163,3 +163,86 @@ func TestDirectoryList(t *testing.T) {
 		assert.Subset(t, found, subdirs)
 	})
 }
+
+func TestReadDirPlus(t *testing.T) {
+	mount := fsConnect(t)
+	defer fsDisconnect(t, mount)
+
+	dir1 := "/base"
+	err := mount.MakeDir(dir1, 0755)
+	assert.NoError(t, err)
+	defer func() { assert.NoError(t, mount.RemoveDir(dir1)) }()
+
+	subdirs := []string{"a", "bb", "ccc", "dddd"}
+	for _, s := range subdirs {
+		spath := dir1 + "/" + s
+		err = mount.MakeDir(spath, 0755)
+		assert.NoError(t, err)
+		defer func(d string) {
+			assert.NoError(t, mount.RemoveDir(d))
+		}(spath)
+	}
+
+	t.Run("root", func(t *testing.T) {
+		dir, err := mount.OpenDir("/")
+		assert.NoError(t, err)
+		assert.NotNil(t, dir)
+		defer func() { assert.NoError(t, dir.Close()) }()
+
+		found := []string{}
+		for {
+			entry, err := dir.ReadDirPlus(StatxBasicStats, AtSymlinkNofollow)
+			assert.NoError(t, err)
+			if entry == nil {
+				break
+			}
+			assert.NotEqual(t, Inode(0), entry.Inode())
+			assert.NotEqual(t, "", entry.Name())
+			found = append(found, entry.Name())
+		}
+		assert.Contains(t, found, "base")
+	})
+	t.Run("dir1", func(t *testing.T) {
+		dir, err := mount.OpenDir(dir1)
+		assert.NoError(t, err)
+		assert.NotNil(t, dir)
+		defer func() { assert.NoError(t, dir.Close()) }()
+
+		found := []string{}
+		for {
+			entry, err := dir.ReadDirPlus(StatxBasicStats, AtSymlinkNofollow)
+			assert.NoError(t, err)
+			if entry == nil {
+				break
+			}
+			assert.NotEqual(t, Inode(0), entry.Inode())
+			assert.NotEqual(t, "", entry.Name())
+			// we have created all the contents of this dir and they are all
+			// empty dirs.
+			assert.Equal(t, DTypeDir, entry.DType())
+			// get statx data from the entry, and check it
+			st := entry.Statx()
+			assert.Equal(t, StatxBasicStats, st.Mask&StatxBasicStats)
+			assert.Equal(t, uint16(0755), st.Mode&0777)
+			found = append(found, entry.Name())
+		}
+		assert.Subset(t, found, subdirs)
+	})
+}
+
+func TestReadDirPlusInvalid(t *testing.T) {
+	mount := fsConnect(t)
+	defer fsDisconnect(t, mount)
+
+	dir, err := mount.OpenDir("/")
+	assert.NoError(t, err)
+	assert.NotNil(t, dir)
+	defer func() { assert.NoError(t, dir.Close()) }()
+
+	// Feed it an invalid flag to trigger in EINVAL in libcephfs. This could
+	// break in the future if it ever becomes a valid flag but it works well
+	// enough for now, and the error suddenly changing to no error will be
+	// kinda obvious.
+	_, err = dir.ReadDirPlus(StatxBasicStats, AtFlags(1<<13))
+	assert.Error(t, err)
+}