From 74d77ff49bd242beaac26e3c1857b71801181902 Mon Sep 17 00:00:00 2001 From: Livio Soares Date: Thu, 24 Mar 2016 22:37:52 -0400 Subject: [PATCH 1/5] Fix rados.GetRadosError() interface to allow package-external use. In Go, the "C" package is a special type of package that creates local symbols names (non exportable) to the importing package. The result, in the case of rados.GetRadosError() is that trying to use results in: cannot use cerr (type C.int) as type rados.C.int in argument to rados.GetRadosError It seems that "C.int" within 'rados' namespace produces a symbol of the type `_Ctype_int`, which given it's first character `_` is not exported. As such, we have the type being defined as `rados._Ctype_int`, which is not accessible outside of the rados package. This commit changes the interface to use the native Go `int` type. --- rados/ioctx.go | 32 ++++++++++++++++---------------- rados/rados.go | 2 +- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/rados/ioctx.go b/rados/ioctx.go index 27b687d..b604120 100644 --- a/rados/ioctx.go +++ b/rados/ioctx.go @@ -62,7 +62,7 @@ func (ioctx *IOContext) Write(oid string, data []byte, offset uint64) error { (C.size_t)(len(data)), (C.uint64_t)(offset)) - return GetRadosError(ret) + return GetRadosError(int(ret)) } // WriteFull writes len(data) bytes to the object with key oid. @@ -75,7 +75,7 @@ func (ioctx *IOContext) WriteFull(oid string, data []byte) error { ret := C.rados_write_full(ioctx.ioctx, c_oid, (*C.char)(unsafe.Pointer(&data[0])), (C.size_t)(len(data))) - return GetRadosError(ret) + return GetRadosError(int(ret)) } // Append appends len(data) bytes to the object with key oid. @@ -88,7 +88,7 @@ func (ioctx *IOContext) Append(oid string, data []byte) error { ret := C.rados_append(ioctx.ioctx, c_oid, (*C.char)(unsafe.Pointer(&data[0])), (C.size_t)(len(data))) - return GetRadosError(ret) + return GetRadosError(int(ret)) } // Read reads up to len(data) bytes from the object with key oid starting at byte @@ -111,7 +111,7 @@ func (ioctx *IOContext) Read(oid string, data []byte, offset uint64) (int, error if ret >= 0 { return int(ret), nil } else { - return 0, GetRadosError(ret) + return 0, GetRadosError(int(ret)) } } @@ -120,7 +120,7 @@ func (ioctx *IOContext) Delete(oid string) error { c_oid := C.CString(oid) defer C.free(unsafe.Pointer(c_oid)) - return GetRadosError(C.rados_remove(ioctx.ioctx, c_oid)) + return GetRadosError(int(C.rados_remove(ioctx.ioctx, c_oid))) } // Truncate resizes the object with key oid to size size. If the operation @@ -131,7 +131,7 @@ func (ioctx *IOContext) Truncate(oid string, size uint64) error { c_oid := C.CString(oid) defer C.free(unsafe.Pointer(c_oid)) - return GetRadosError(C.rados_trunc(ioctx.ioctx, c_oid, (C.uint64_t)(size))) + return GetRadosError(int(C.rados_trunc(ioctx.ioctx, c_oid, (C.uint64_t)(size)))) } // Destroy informs librados that the I/O context is no longer in use. @@ -253,7 +253,7 @@ func (ioctx *IOContext) GetXattr(object string, name string, data []byte) (int, if ret >= 0 { return int(ret), nil } else { - return 0, GetRadosError(ret) + return 0, GetRadosError(int(ret)) } } @@ -271,7 +271,7 @@ func (ioctx *IOContext) SetXattr(object string, name string, data []byte) error (*C.char)(unsafe.Pointer(&data[0])), (C.size_t)(len(data))) - return GetRadosError(ret) + return GetRadosError(int(ret)) } // function that lists all the xattrs for an object, since xattrs are @@ -285,7 +285,7 @@ func (ioctx *IOContext) ListXattrs(oid string) (map[string][]byte, error) { ret := C.rados_getxattrs(ioctx.ioctx, c_oid, &it) if ret < 0 { - return nil, GetRadosError(ret) + return nil, GetRadosError(int(ret)) } defer func() { C.rados_getxattrs_end(it) }() m := make(map[string][]byte) @@ -297,7 +297,7 @@ func (ioctx *IOContext) ListXattrs(oid string) (map[string][]byte, error) { ret := C.rados_getxattrs_next(it, &c_name, &c_val, &c_len) if ret < 0 { - return nil, GetRadosError(ret) + return nil, GetRadosError(int(ret)) } // rados api returns a null name,val & 0-length upon // end of iteration @@ -320,7 +320,7 @@ func (ioctx *IOContext) RmXattr(oid string, name string) error { c_oid, c_name) - return GetRadosError(ret) + return GetRadosError(int(ret)) } // Append the map `pairs` to the omap `oid` @@ -376,7 +376,7 @@ func (ioctx *IOContext) SetOmap(oid string, pairs map[string][]byte) error { ret := C.rados_write_op_operate(op, ioctx.ioctx, c_oid, nil, 0) C.rados_release_write_op(op) - return GetRadosError(ret) + return GetRadosError(int(ret)) } // OmapListFunc is the type of the function called for each omap key @@ -416,7 +416,7 @@ func (ioctx *IOContext) ListOmapValues(oid string, startAfter string, filterPref if int(c_prval) != 0 { return RadosError(int(c_prval)) } else if int(ret) != 0 { - return GetRadosError(ret) + return GetRadosError(int(ret)) } for { @@ -427,7 +427,7 @@ func (ioctx *IOContext) ListOmapValues(oid string, startAfter string, filterPref ret = C.rados_omap_get_next(c_iter, &c_key, &c_val, &c_len) if int(ret) != 0 { - return GetRadosError(ret) + return GetRadosError(int(ret)) } if c_key == nil { @@ -520,7 +520,7 @@ func (ioctx *IOContext) RmOmapKeys(oid string, keys []string) error { ret := C.rados_write_op_operate(op, ioctx.ioctx, c_oid, nil, 0) C.rados_release_write_op(op) - return GetRadosError(ret) + return GetRadosError(int(ret)) } // Clear the omap `oid` @@ -534,5 +534,5 @@ func (ioctx *IOContext) CleanOmap(oid string) error { ret := C.rados_write_op_operate(op, ioctx.ioctx, c_oid, nil, 0) C.rados_release_write_op(op) - return GetRadosError(ret) + return GetRadosError(int(ret)) } diff --git a/rados/rados.go b/rados/rados.go index f1340c3..1e6b722 100644 --- a/rados/rados.go +++ b/rados/rados.go @@ -20,7 +20,7 @@ func (e RadosError) Error() string { var RadosErrorNotFound = errors.New("Rados error not found") -func GetRadosError(err C.int) error { +func GetRadosError(err int) error { if err != 0 { if err == -C.ENOENT { return RadosErrorNotFound From d5b49dd4eaf7bc4df50b8534b0593a469d776d11 Mon Sep 17 00:00:00 2001 From: Livio Soares Date: Fri, 25 Mar 2016 00:57:23 -0400 Subject: [PATCH 2/5] rados_test.go: Fix import header to allow `gofmt` to work. --- rados/rados_test.go | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/rados/rados_test.go b/rados/rados_test.go index b2b7c70..57f5e9d 100644 --- a/rados/rados_test.go +++ b/rados/rados_test.go @@ -1,19 +1,20 @@ package rados_test -import "testing" +import ( + "encoding/json" + "fmt" + "io" + "io/ioutil" + "net" + "os" + "os/exec" + "sort" + "testing" + "time" -//import "bytes" -import "github.com/ceph/go-ceph/rados" -import "github.com/stretchr/testify/assert" -import "os" -import "os/exec" -import "io" -import "io/ioutil" -import "time" -import "net" -import "fmt" -import "sort" -import "encoding/json" + "github.com/ceph/go-ceph/rados" + "github.com/stretchr/testify/assert" +) func GetUUID() string { out, _ := exec.Command("uuidgen").Output() From 57195e64fe8ae0c8c7c9e600c95931d4ecbf2cf3 Mon Sep 17 00:00:00 2001 From: Livio Soares Date: Fri, 25 Mar 2016 00:58:11 -0400 Subject: [PATCH 3/5] rados_test.go: Improve Stat() test for non existant object name. The test currently fails since 'RadosErrorNotFound' is not properly returned. --- rados/rados_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/rados/rados_test.go b/rados/rados_test.go index 57f5e9d..81fe58d 100644 --- a/rados/rados_test.go +++ b/rados/rados_test.go @@ -399,6 +399,9 @@ func TestObjectStat(t *testing.T) { assert.Equal(t, uint64(len(bytes_in)), stat.Size) assert.NotNil(t, stat.ModTime) + _, err = pool.Stat("notfound") + assert.Equal(t, err, rados.RadosErrorNotFound) + pool.Destroy() conn.Shutdown() } From f74dc6c3c65f8bb6f35e66f8f76a3240b7914657 Mon Sep 17 00:00:00 2001 From: Livio Soares Date: Fri, 25 Mar 2016 00:59:18 -0400 Subject: [PATCH 4/5] rados: Use GetRadosError() in lieu of RadosError() so as to guarantee the translation of error codes into rados errors where applicable. This commit fixes the TestObjectStat() case where a non existing object name is tested for existance. --- rados/ioctx.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/rados/ioctx.go b/rados/ioctx.go index b604120..cd0d981 100644 --- a/rados/ioctx.go +++ b/rados/ioctx.go @@ -147,7 +147,7 @@ func (ioctx *IOContext) GetPoolStats() (stat PoolStat, err error) { c_stat := C.struct_rados_pool_stat_t{} ret := C.rados_ioctx_pool_stat(ioctx.ioctx, &c_stat) if ret < 0 { - return PoolStat{}, RadosError(int(ret)) + return PoolStat{}, GetRadosError(int(ret)) } else { return PoolStat{ Num_bytes: uint64(c_stat.num_bytes), @@ -176,7 +176,7 @@ func (ioctx *IOContext) GetPoolName() (name string, err error) { buf = make([]byte, len(buf)*2) continue } else if ret < 0 { - return "", RadosError(ret) + return "", GetRadosError(int(ret)) } name = C.GoStringN((*C.char)(unsafe.Pointer(&buf[0])), ret) return name, nil @@ -194,7 +194,7 @@ func (ioctx *IOContext) ListObjects(listFn ObjectListFunc) error { var ctx C.rados_list_ctx_t ret := C.rados_objects_list_open(ioctx.ioctx, &ctx) if ret < 0 { - return RadosError(ret) + return GetRadosError(int(ret)) } defer func() { C.rados_objects_list_close(ctx) }() @@ -204,7 +204,7 @@ func (ioctx *IOContext) ListObjects(listFn ObjectListFunc) error { if ret == -2 { // FIXME return nil } else if ret < 0 { - return RadosError(ret) + return GetRadosError(int(ret)) } listFn(C.GoString(c_entry)) } @@ -226,7 +226,7 @@ func (ioctx *IOContext) Stat(object string) (stat ObjectStat, err error) { &c_pmtime) if ret < 0 { - return ObjectStat{}, RadosError(int(ret)) + return ObjectStat{}, GetRadosError(int(ret)) } else { return ObjectStat{ Size: uint64(c_psize), From 3d88c03e6e4fe147e570ee7e33965310790d0961 Mon Sep 17 00:00:00 2001 From: Livio Soares Date: Sat, 26 Mar 2016 11:50:03 -0400 Subject: [PATCH 5/5] rados: Change RadoError Error() interface to provide meaningful string, even for non-default cases GetRadosError(). I got a "rados: ret=-22" message as part of call I made. I'm a bit rusty with my C stderror codes, which made the error a bit unhelpful. This commit makes this particular error come out as "rados: invalid argument", even if we don't need to explicitly create a Rados error variable for invalid argument case. It does so by using C.strerror(). A further change is that with this, we do not need to explicitly code RadosErrorNotFound and RadosErrorPermissionsDenied using a different error type thank RadosError. --- rados/rados.go | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/rados/rados.go b/rados/rados.go index 1e6b722..944e642 100644 --- a/rados/rados.go +++ b/rados/rados.go @@ -7,7 +7,6 @@ package rados import "C" import ( - "errors" "fmt" "unsafe" ) @@ -15,20 +14,17 @@ import ( type RadosError int func (e RadosError) Error() string { - return fmt.Sprintf("rados: ret=%d", e) + return fmt.Sprintf("rados: %s", C.GoString(C.strerror(C.int(-e)))) } -var RadosErrorNotFound = errors.New("Rados error not found") +var RadosErrorNotFound = RadosError(-C.ENOENT) +var RadosErrorPermissionDenied = RadosError(-C.EPERM) func GetRadosError(err int) error { - if err != 0 { - if err == -C.ENOENT { - return RadosErrorNotFound - } - return RadosError(err) - } else { + if err == 0 { return nil } + return RadosError(err) } // Version returns the major, minor, and patch components of the version of