Repository: ceph/go-ceph
Pull request #311 (2020-06-18 19:44:34)

cephfs: add file xattr related functions
Add functions wrapping ceph_fsetxattr, ceph_fgetxattr,  ceph_flistxattr, and ceph_fremovexattr. Tests for all are included. See final commit for an interesting wrinkle in the behavior in ceph.

Fixes: #247 

## 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: I put the DNM prefix on to give the ceph team a little time to reply to the issue I filed. If we get quick turnaround on it I can update the code/tests as needed. If not, we can merge this and then just update the docs/tests later.
phlogistonjohn: I've removed the DNM label, as I think the documentation approach is good enough. Please take a look at said doc comment and let me know if you think it needs expanding or if it is sufficient in the current form.
phlogistonjohn: @nixpanic can you please (re-)review this and give an OK (or not)? 

Patch:
diff --git a/cephfs/errors.go b/cephfs/errors.go
index 52d5aa7..1462fc4 100644
--- a/cephfs/errors.go
+++ b/cephfs/errors.go
@@ -35,6 +35,16 @@ func getError(e C.int) error {
 	return cephFSError(e)
 }
 
+// getErrorIfNegative converts a ceph return code to error if negative.
+// This is useful for functions that return a usable positive value on
+// success but a negative error number on error.
+func getErrorIfNegative(ret C.int) error {
+	if ret >= 0 {
+		return nil
+	}
+	return getError(ret)
+}
+
 // Public go errors:
 
 var (
@@ -57,4 +67,5 @@ const (
 	errInvalid     = cephFSError(-C.EINVAL)
 	errNameTooLong = cephFSError(-C.ENAMETOOLONG)
 	errNoEntry     = cephFSError(-C.ENOENT)
+	errRange       = cephFSError(-C.ERANGE)
 )
diff --git a/cephfs/file_xattr.go b/cephfs/file_xattr.go
new file mode 100644
index 0000000..5509e58
--- /dev/null
+++ b/cephfs/file_xattr.go
@@ -0,0 +1,158 @@
+package cephfs
+
+/*
+#cgo LDFLAGS: -lcephfs
+#cgo CPPFLAGS: -D_FILE_OFFSET_BITS=64
+#define _GNU_SOURCE
+#include <stdlib.h>
+#include <sys/types.h>
+#include <sys/xattr.h>
+#include <cephfs/libcephfs.h>
+*/
+import "C"
+
+import (
+	"unsafe"
+
+	"github.com/ceph/go-ceph/internal/cutil"
+	"github.com/ceph/go-ceph/internal/retry"
+)
+
+// XattrFlags are used to control the behavior of set-xattr calls.
+type XattrFlags int
+
+const (
+	// XattrDefault specifies that set-xattr calls use the default behavior of
+	// creating or updating an xattr.
+	XattrDefault = XattrFlags(0)
+	// XattrCreate specifies that set-xattr calls only set new xattrs.
+	XattrCreate = XattrFlags(C.XATTR_CREATE)
+	// XattrReplace specifies that set-xattr calls only replace existing xattr
+	// values.
+	XattrReplace = XattrFlags(C.XATTR_REPLACE)
+)
+
+// SetXattr sets an extended attribute on the open file.
+//
+// NOTE: Attempting to set an xattr value with an empty value may cause
+// the xattr to be unset. Please refer to https://tracker.ceph.com/issues/46084
+//
+// Implements:
+//  int ceph_fsetxattr(struct ceph_mount_info *cmount, int fd, const char *name,
+//                     const void *value, size_t size, int flags);
+func (f *File) SetXattr(name string, value []byte, flags XattrFlags) error {
+	if err := f.validate(); err != nil {
+		return err
+	}
+	if name == "" {
+		return errInvalid
+	}
+	var vptr unsafe.Pointer
+	if len(value) > 0 {
+		vptr = unsafe.Pointer(&value[0])
+	}
+	cName := C.CString(name)
+	defer C.free(unsafe.Pointer(cName))
+
+	ret := C.ceph_fsetxattr(
+		f.mount.mount,
+		f.fd,
+		cName,
+		vptr,
+		C.size_t(len(value)),
+		C.int(flags))
+	return getError(ret)
+}
+
+// GetXattr gets an extended attribute from the open file.
+//
+// Implements:
+//  int ceph_fgetxattr(struct ceph_mount_info *cmount, int fd, const char *name,
+//                     void *value, size_t size);
+func (f *File) GetXattr(name string) ([]byte, error) {
+	if err := f.validate(); err != nil {
+		return nil, err
+	}
+	if name == "" {
+		return nil, errInvalid
+	}
+	cName := C.CString(name)
+	defer C.free(unsafe.Pointer(cName))
+
+	var (
+		ret C.int
+		err error
+		buf []byte
+	)
+	// range from 1k to 64KiB
+	retry.WithSizes(1024, 1<<16, func(size int) retry.Hint {
+		buf = make([]byte, size)
+		ret = C.ceph_fgetxattr(
+			f.mount.mount,
+			f.fd,
+			cName,
+			unsafe.Pointer(&buf[0]),
+			C.size_t(size))
+		err = getErrorIfNegative(ret)
+		return retry.DoubleSize.If(err == errRange)
+	})
+	if err != nil {
+		return nil, err
+	}
+	return buf[:ret], nil
+}
+
+// ListXattr returns a slice containing strings for the name of each xattr set
+// on the fie.
+//
+// Implements:
+//  int ceph_flistxattr(struct ceph_mount_info *cmount, int fd, char *list, size_t size);
+func (f *File) ListXattr() ([]string, error) {
+	if err := f.validate(); err != nil {
+		return nil, err
+	}
+
+	var (
+		ret C.int
+		err error
+		buf []byte
+	)
+	// range from 1k to 64KiB
+	retry.WithSizes(1024, 1<<16, func(size int) retry.Hint {
+		buf = make([]byte, size)
+		ret = C.ceph_flistxattr(
+			f.mount.mount,
+			f.fd,
+			(*C.char)(unsafe.Pointer(&buf[0])),
+			C.size_t(size))
+		err = getErrorIfNegative(ret)
+		return retry.DoubleSize.If(err == errRange)
+	})
+	if err != nil {
+		return nil, err
+	}
+
+	names := cutil.SplitSparseBuffer(buf[:ret])
+	return names, nil
+}
+
+// RemoveXattr removes the named xattr from the open file.
+//
+// Implements:
+//  int ceph_fremovexattr(struct ceph_mount_info *cmount, int fd, const char *name);
+func (f *File) RemoveXattr(name string) error {
+	if err := f.validate(); err != nil {
+		return err
+	}
+	if name == "" {
+		return errInvalid
+	}
+	cName := C.CString(name)
+	defer C.free(unsafe.Pointer(cName))
+
+	ret := C.ceph_fremovexattr(
+		f.mount.mount,
+		f.fd,
+		cName)
+	return getError(ret)
+}


Test patch:
diff --git a/cephfs/file_xattr_test.go b/cephfs/file_xattr_test.go
new file mode 100644
index 0000000..6939604
--- /dev/null
+++ b/cephfs/file_xattr_test.go
@@ -0,0 +1,162 @@
+package cephfs
+
+import (
+	"os"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+var samples = []struct {
+	name  string
+	value []byte
+}{
+	{
+		name:  "user.xPhrase",
+		value: []byte("june and july"),
+	},
+	{
+		name:  "user.xHasNulls",
+		value: []byte("\x00got\x00null?\x00"),
+	},
+	{
+		name:  "user.x2kZeros",
+		value: make([]byte, 2048),
+	},
+	// Ceph's behavior when an empty value is supplied may be considered
+	// to have a bug in some versions. Using an empty value may cause
+	// the xattr to be unset. Please refer to:
+	// https://tracker.ceph.com/issues/46084
+	// So we avoid testing for that case explicitly here.
+	//{
+	//	name:  "user.xEmpty",
+	//	value: []byte(""),
+	//},
+}
+
+func TestGetSetXattr(t *testing.T) {
+	mount := fsConnect(t)
+	defer fsDisconnect(t, mount)
+	fname := "TestGetSetXattr.txt"
+
+	f, err := mount.Open(fname, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0666)
+	require.NoError(t, err)
+	defer func() {
+		assert.NoError(t, f.Close())
+		assert.NoError(t, mount.Unlink(fname))
+	}()
+
+	for _, s := range samples {
+		t.Run("roundTrip-"+s.name, func(t *testing.T) {
+			err := f.SetXattr(s.name, s.value, XattrDefault)
+			assert.NoError(t, err)
+			b, err := f.GetXattr(s.name)
+			assert.NoError(t, err)
+			assert.EqualValues(t, s.value, b)
+		})
+	}
+
+	t.Run("missingXattrOnGet", func(t *testing.T) {
+		_, err := f.GetXattr("user.never-set")
+		assert.Error(t, err)
+	})
+
+	t.Run("emptyNameGet", func(t *testing.T) {
+		_, err := f.GetXattr("")
+		assert.Error(t, err)
+	})
+
+	t.Run("emptyNameSet", func(t *testing.T) {
+		err := f.SetXattr("", []byte("foo"), XattrDefault)
+		assert.Error(t, err)
+	})
+
+	t.Run("invalidFile", func(t *testing.T) {
+		f1 := &File{}
+		err := f1.SetXattr(samples[0].name, samples[0].value, XattrDefault)
+		assert.Error(t, err)
+		_, err = f1.GetXattr(samples[0].name)
+		assert.Error(t, err)
+	})
+}
+
+func TestListXattr(t *testing.T) {
+	mount := fsConnect(t)
+	defer fsDisconnect(t, mount)
+	fname := "TestListXattr.txt"
+
+	f, err := mount.Open(fname, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0666)
+	require.NoError(t, err)
+	defer func() {
+		assert.NoError(t, f.Close())
+		assert.NoError(t, mount.Unlink(fname))
+	}()
+
+	t.Run("listXattrs1", func(t *testing.T) {
+		for _, s := range samples[:1] {
+			err := f.SetXattr(s.name, s.value, XattrDefault)
+			assert.NoError(t, err)
+		}
+		xl, err := f.ListXattr()
+		assert.NoError(t, err)
+		assert.Len(t, xl, 1)
+		assert.Contains(t, xl, samples[0].name)
+	})
+
+	t.Run("listXattrs2", func(t *testing.T) {
+		for _, s := range samples {
+			err := f.SetXattr(s.name, s.value, XattrDefault)
+			assert.NoError(t, err)
+		}
+		xl, err := f.ListXattr()
+		assert.NoError(t, err)
+		assert.Len(t, xl, 3)
+		assert.Contains(t, xl, samples[0].name)
+		assert.Contains(t, xl, samples[1].name)
+		assert.Contains(t, xl, samples[2].name)
+	})
+
+	t.Run("invalidFile", func(t *testing.T) {
+		f1 := &File{}
+		_, err := f1.ListXattr()
+		assert.Error(t, err)
+	})
+}
+
+func TestRemoveXattr(t *testing.T) {
+	mount := fsConnect(t)
+	defer fsDisconnect(t, mount)
+	fname := "TestRemoveXattr.txt"
+
+	f, err := mount.Open(fname, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0666)
+	require.NoError(t, err)
+	defer func() {
+		assert.NoError(t, f.Close())
+		assert.NoError(t, mount.Unlink(fname))
+	}()
+
+	t.Run("removeXattr", func(t *testing.T) {
+		s := samples[0]
+		err := f.SetXattr(s.name, s.value, XattrDefault)
+		err = f.RemoveXattr(s.name)
+		assert.NoError(t, err)
+	})
+
+	t.Run("removeMissingXattr", func(t *testing.T) {
+		s := samples[1]
+		err := f.RemoveXattr(s.name)
+		assert.Error(t, err)
+	})
+
+	t.Run("emptyName", func(t *testing.T) {
+		err := f.RemoveXattr("")
+		assert.Error(t, err)
+	})
+
+	t.Run("invalidFile", func(t *testing.T) {
+		f1 := &File{}
+		err := f1.RemoveXattr(samples[0].name)
+		assert.Error(t, err)
+	})
+}