Repository: ceph/go-ceph
Pull request #231 (2020-04-27 19:49:43)

cephfs: add path based Statx function implmenting ceph_statx
Add a Statx wrapper for ceph_statx. Add a type wrapping the statx status
info that exposes the various fields from the C-struct. Note that the
ceph struct is not the same as the linux statx struct.

### Updates 2020-05-06:

A wrapper around x/sys's unix.Timspec has been added and used for the time fields.

The fields in the Go struct are now public. Gone are the accessor functions.  The idea is that future ceph functionality will make use of a "writable" CephStatx structure. C structs are converted/copied into the Golang struct.

We will not be wrapping ceph_{f,l,}stat functions. We may choose to make higher level interfaces on top of these, probably hewing closer to the Go os module higher-level abstractions. This is all outside the scope of the PR.

## 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: So as far as the suite of ceph_{stat,lstat,fstat} functions versus ceph_{statx,fstatx} goes it appears we have a couple of options. We can try to make use of the more-traditional stat calls when available, which I think is octopus only for the ceph versions we are currently supporting (I checked luminus and nautilus and neither have them). We'd need conditional compilation and some sort of wrappers on those that lack the traditional stats.

Alternatively, we just always call the statx variants in go-ceph and always provide the traditional stat wrappers ourselves.

Personally, I see the latter as a lot less work and cleaner anyway. I only hesitate because the traditional stat calls are not based on statx in the C++ code AFAICT. I'm going to ask @jtlayton for an opinion as well due to some of the statements made in https://github.com/ceph/ceph/pull/25650
jtlayton: My opinion: I mostly consider the stat/lstat/fstat functions to be "legacy" at this point. New code that needs that sort functionality ought to be coded to use statx-style interfaces.

If you need to add legacy wrappers to allow existing callers to continue working, then implementing trivial wrappers around the new-style statx interfaces is fine. Otherwise, I'd probably just add the statx-style interfaces and not bother with wrappers at all.
phlogistonjohn: @jtlayton thank you for chiming in. That really helps me with this PR. I like that approach and I appreciate your time.

Patch:
diff --git a/cephfs/errors.go b/cephfs/errors.go
index fdbecb3..766cb60 100644
--- a/cephfs/errors.go
+++ b/cephfs/errors.go
@@ -45,7 +45,7 @@ const (
 // Private errors:
 
 const (
+	errInvalid     = CephFSError(-C.EINVAL)
 	errNameTooLong = CephFSError(-C.ENAMETOOLONG)
-
-	errInvalid = CephFSError(-C.EINVAL)
+	errNoEntry     = CephFSError(-C.ENOENT)
 )
diff --git a/cephfs/path.go b/cephfs/path.go
index e4bd5b8..e58221c 100644
--- a/cephfs/path.go
+++ b/cephfs/path.go
@@ -130,3 +130,29 @@ func (mount *MountInfo) Readlink(path string) (string, error) {
 
 	return string(buf[:ret]), nil
 }
+
+// Statx returns information about a file/directory.
+//
+// Implements:
+//  int ceph_statx(struct ceph_mount_info *cmount, const char *path, struct ceph_statx *stx,
+//                 unsigned int want, unsigned int flags);
+func (mount *MountInfo) Statx(path string, want StatxMask, flags AtFlags) (*CephStatx, error) {
+	if err := mount.validate(); err != nil {
+		return nil, err
+	}
+	cPath := C.CString(path)
+	defer C.free(unsafe.Pointer(cPath))
+
+	var stx C.struct_ceph_statx
+	ret := C.ceph_statx(
+		mount.mount,
+		cPath,
+		&stx,
+		C.uint(want),
+		C.uint(flags),
+	)
+	if err := getError(ret); err != nil {
+		return nil, err
+	}
+	return cStructToCephStatx(stx), nil
+}
diff --git a/cephfs/statx.go b/cephfs/statx.go
new file mode 100644
index 0000000..f822a8d
--- /dev/null
+++ b/cephfs/statx.go
@@ -0,0 +1,140 @@
+package cephfs
+
+/*
+#cgo LDFLAGS: -lcephfs
+#cgo CPPFLAGS: -D_FILE_OFFSET_BITS=64
+#include <cephfs/libcephfs.h>
+*/
+import "C"
+
+// StatxMask values contain bit-flags indicating what data should be
+// populated by a statx-type call.
+type StatxMask uint32
+
+const (
+	// StatxMode requests the mode value be filled in.
+	StatxMode = StatxMask(C.CEPH_STATX_MODE)
+	// StatxNlink requests the nlink value be filled in.
+	StatxNlink = StatxMask(C.CEPH_STATX_NLINK)
+	// StatxUid requests the uid value be filled in.
+	StatxUid = StatxMask(C.CEPH_STATX_UID)
+	// StatxRdev requests the rdev value be filled in.
+	StatxRdev = StatxMask(C.CEPH_STATX_RDEV)
+	// StatxAtime requests the access-time value be filled in.
+	StatxAtime = StatxMask(C.CEPH_STATX_ATIME)
+	// StatxMtime requests the modified-time value be filled in.
+	StatxMtime = StatxMask(C.CEPH_STATX_MTIME)
+	// StatxIno requests the inode be filled in.
+	StatxIno = StatxMask(C.CEPH_STATX_INO)
+	// StatxSize requests the size value be filled in.
+	StatxSize = StatxMask(C.CEPH_STATX_SIZE)
+	// StatxBlocks requests the blocks value be filled in.
+	StatxBlocks = StatxMask(C.CEPH_STATX_BLOCKS)
+	// StatxBasicStats requests all the fields that are part of a
+	// traditional stat call.
+	StatxBasicStats = StatxMask(C.CEPH_STATX_BASIC_STATS)
+	// StatxBtime requests the birth-time value be filled in.
+	StatxBtime = StatxMask(C.CEPH_STATX_BTIME)
+	// StatxVersion requests the version value be filled in.
+	StatxVersion = StatxMask(C.CEPH_STATX_VERSION)
+	// StatxAllStats requests all known stat values be filled in.
+	StatxAllStats = StatxMask(C.CEPH_STATX_ALL_STATS)
+)
+
+// AtFlags represent flags to be passed to calls that control how files
+// are used or referenced. For example, not following symlinks.
+type AtFlags uint
+
+const (
+	// AtNoAttrSync requests that the stat call only fetch locally-cached
+	// values if possible, avoiding round trips to a back-end server.
+	AtNoAttrSync = AtFlags(C.AT_NO_ATTR_SYNC)
+	// AtSymlinkNofollow indicates the call should not follow symlinks
+	// but operate on the symlink itself.
+	AtSymlinkNofollow = AtFlags(C.AT_SYMLINK_NOFOLLOW)
+)
+
+// NOTE: CephStatx fields are meant to be settable by the callers.
+// This is the primary reason we use public fields and not accessors
+// for the CephStatx type.
+
+// CephStatx instances are returned by extended stat (statx) calls.
+// Note that CephStatx results are similar to but not identical
+// to (Linux) system statx results.
+type CephStatx struct {
+	// Mask is a bitmask indicating what fields have been set.
+	Mask StatxMask
+	// Blksize represents the file system's block size.
+	Blksize uint32
+	// Nlink is the number of links for the file.
+	Nlink uint32
+	// Uid (user id) value for the file.
+	Uid uint32
+	// Gid (group id) value for the file.
+	Gid uint32
+	// Mode is the file's type and mode value.
+	Mode uint16
+	// Inode value for the file.
+	Inode Inode
+	// Size of the file in bytes.
+	Size uint64
+	// Blocks indicates the number of blocks allocated to the file.
+	Blocks uint64
+	// Dev describes the device containing this file system.
+	Dev uint64
+	// Rdev describes the device of this file, if the file is a device.
+	Rdev uint64
+	// Atime is the access time of this file.
+	Atime Timespec
+	// Ctime is the status change time of this file.
+	Ctime Timespec
+	// Mtime is the modification time of this file.
+	Mtime Timespec
+	// Btime is the creation (birth) time of this file.
+	Btime Timespec
+	// Version value for the file.
+	Version uint64
+}
+
+func cStructToCephStatx(s C.struct_ceph_statx) *CephStatx {
+	return &CephStatx{
+		Mask:    StatxMask(s.stx_mask),
+		Blksize: uint32(s.stx_blksize),
+		Nlink:   uint32(s.stx_nlink),
+		Uid:     uint32(s.stx_uid),
+		Gid:     uint32(s.stx_gid),
+		Mode:    uint16(s.stx_mode),
+		Inode:   Inode(s.stx_ino),
+		Size:    uint64(s.stx_size),
+		Blocks:  uint64(s.stx_blocks),
+		Dev:     uint64(s.stx_dev),
+		Rdev:    uint64(s.stx_rdev),
+		Atime:   cStructToTimespec(s.stx_atime),
+		Ctime:   cStructToTimespec(s.stx_ctime),
+		Mtime:   cStructToTimespec(s.stx_mtime),
+		Btime:   cStructToTimespec(s.stx_btime),
+		Version: uint64(s.stx_version),
+	}
+}
+
+/* TODO:
+   - enable later when we can test round -trips
+   - add time fields
+
+func (c *CephStatx) toCStruct() C.struct_ceph_statx {
+	var s C.struct_ceph_statx
+	s.stx_mask = C.uint32_t(c.Mask)
+	s.stx_blksize = C.uint32_t(c.Blksize)
+	s.stx_nlink = C.uint32_t(c.Nlink)
+	s.stx_uid = C.uint32_t(c.Uid)
+	s.stx_gid = C.uint32_t(c.Gid)
+	s.stx_mode = C.uint16_t(c.Mode)
+	s.stx_ino = C.uint64_t(c.Inode)
+	s.stx_size = C.uint64_t(c.Size)
+	s.stx_blocks = C.uint64_t(c.Blocks)
+	s.stx_dev = C.uint64_t(c.Dev)
+	s.stx_rdev = C.uint64_t(c.Rdev)
+	s.stx_version = C.uint64_t(c.Version)
+	return s
+}
+*/
diff --git a/cephfs/timespec.go b/cephfs/timespec.go
new file mode 100644
index 0000000..b10b36b
--- /dev/null
+++ b/cephfs/timespec.go
@@ -0,0 +1,22 @@
+package cephfs
+
+/*
+#include <time.h>
+*/
+import "C"
+
+import (
+	"golang.org/x/sys/unix"
+)
+
+// Timespec behaves similarly to C's struct timespec.
+// Timespec is used to retain fidelity to the C based file systems
+// apis that could be lossy with the use of Go time types.
+type Timespec unix.Timespec
+
+func cStructToTimespec(t C.struct_timespec) Timespec {
+	return Timespec{
+		Sec:  int64(t.tv_sec),
+		Nsec: int64(t.tv_nsec),
+	}
+}
diff --git a/go.mod b/go.mod
index ee2d33a..f7564b8 100644
--- a/go.mod
+++ b/go.mod
@@ -5,4 +5,5 @@ go 1.12
 require (
 	github.com/gofrs/uuid v3.2.0+incompatible
 	github.com/stretchr/testify v1.4.0
+	golang.org/x/sys v0.0.0-20200501145240-bc7a7d42d5c3
 )
diff --git a/go.sum b/go.sum
index 1445a01..c090e5f 100644
--- a/go.sum
+++ b/go.sum
@@ -7,6 +7,8 @@ github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZN
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
 github.com/stretchr/testify v1.4.0 h1:2E4SXV/wtOkTonXsotYi4li6zVWxYlZuYNCXe9XRJyk=
 github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
+golang.org/x/sys v0.0.0-20200501145240-bc7a7d42d5c3 h1:5B6i6EAiSYyejWfvc5Rc9BbI3rzIsrrXfAQBWnYfn+w=
+golang.org/x/sys v0.0.0-20200501145240-bc7a7d42d5c3/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
 gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw=


Test patch:
diff --git a/cephfs/path_test.go b/cephfs/path_test.go
index bb8bd92..9065df2 100644
--- a/cephfs/path_test.go
+++ b/cephfs/path_test.go
@@ -357,3 +357,23 @@ func TestReadlink(t *testing.T) {
 		assert.Equal(t, buf, "")
 	})
 }
+
+func TestStatx(t *testing.T) {
+	mount := fsConnect(t)
+	defer fsDisconnect(t, mount)
+
+	dirname := "statme"
+	assert.NoError(t, mount.MakeDir(dirname, 0755))
+
+	st, err := mount.Statx(dirname, StatxBasicStats, 0)
+	assert.NoError(t, err)
+	assert.NotNil(t, st)
+	assert.Equal(t, uint16(0755), st.Mode&0777)
+
+	assert.NoError(t, mount.RemoveDir(dirname))
+
+	st, err = mount.Statx(dirname, StatxBasicStats, 0)
+	assert.Error(t, err)
+	assert.Nil(t, st)
+	assert.Equal(t, errNoEntry, err)
+}
diff --git a/cephfs/statx_test.go b/cephfs/statx_test.go
new file mode 100644
index 0000000..0592eb7
--- /dev/null
+++ b/cephfs/statx_test.go
@@ -0,0 +1,30 @@
+package cephfs
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+// TestStatxFieldsRootDir does not assert much about every field
+// as these can vary between runs. We exercise the getters but
+// can only make "lightweight" assertions here.
+func TestStatxFieldsRootDir(t *testing.T) {
+	mount := fsConnect(t)
+	defer fsDisconnect(t, mount)
+
+	st, err := mount.Statx("/", StatxBasicStats, 0)
+	assert.NoError(t, err)
+	assert.NotNil(t, st)
+
+	assert.Equal(t, StatxBasicStats, st.Mask&StatxBasicStats)
+	assert.Equal(t, uint32(2), st.Nlink)
+	assert.Equal(t, uint32(0), st.Uid)
+	assert.Equal(t, uint32(0), st.Gid)
+	assert.NotEqual(t, uint16(0), st.Mode)
+	assert.Equal(t, uint16(0040000), st.Mode&0040000) // is dir?
+	assert.NotEqual(t, Inode(0), st.Inode)
+	assert.NotEqual(t, uint64(0), st.Dev)
+	assert.Equal(t, uint64(0), st.Rdev)
+	assert.Greater(t, st.Ctime.Sec, int64(1588711788))
+}