Repository: ceph/go-ceph
Pull request #1032 (2024-10-02 14:47:33)

Introduce common cephError type to compare errors between packages
A new `internal/errutil.cephError` can be used as a common type to replace all error types that are specific to the different packages (`cephfsError`, `radosError`, `rbdError`). By using the new `cephError` type everywhere, it becomes possible to match `rados.ErrNotFound` and `rbd.ErrNotFound` with the standard `errors.Is()` function.

Some go-ceph functions from the cephfs and rbd packages can return errors generated by the lower-level rados package. In such a case, a caller may need to check for e.g. `rados.ErrNotFound` as well as `rbd.ErrNotFound`, depending on the function that was called. This is prone to errors, as users of go-ceph may not inspect what the go-ceph function does and which error type it could return.

Closes: #1031

## Checklist
- [x] Added tests for features and functional changes
- [x] Public functions and types are documented
- [x] Standard formatting is applied to Go code
- [ ] Is this a new API? Added a new file that begins with `//go:build ceph_preview`
- [ ] Ran `make api-update` to record new APIs

New or infrequent contributors may want to review the go-ceph [Developer's Guide](https://github.com/ceph/go-ceph/blob/master/docs/development.md) including the section on how we track [API Status](https://github.com/ceph/go-ceph/blob/master/docs/development.md#api-status) and the [API Stability Plan](https://github.com/ceph/go-ceph/blob/master/docs/api-stability.md).

The go-ceph project uses mergify. View the [mergify command guide](https://docs.mergify.com/commands/#commands) for information on how to interact with mergify. Add a comment with `@Mergifyio` `rebase` to rebase your PR when github indicates that the PR is out of date with the base branch.


nixpanic: @anoopcs9 the file naming an location is actually intentional, because of two reasons:

1. the tests do not really test the _errutil_ package but users of the package,
2. importing the _rados_ package in the _errutil_ package that gets imported by _rados_ causes recursive imports (although that may be allowed for tests?)

If you insist on renaming/moving, I'll do that. Please state your preference in that case. Thanks!
anoopcs9: > @anoopcs9 the file naming an location is actually intentional, because of two reasons:
> 
> 1. the tests do not really test the _errutil_ package but users of the package,
> 2. importing the _rados_ package in the _errutil_ package that gets imported by _rados_ causes recursive imports (although that may be allowed for tests?)

In that case how about we rewrite _errors_test.go_ in the direction of _strerror_test.go_ and put in under _internal/errutil_?

> > Shouldn't we consider updating the API docs about those older implementations getting removed?
> 
> The implementation is an internal-only change. I do not expect any visible changes on the outside, all should be backwards compatible.

I agree but those APIs would still remain listed in _docs/api-status.json_.
nixpanic: > I agree but those APIs would still remain listed in _docs/api-status.json_.

oh, right, I see `rbdError` and so on there :thinking:
nixpanic: Hey @anoopcs9,

>     * rename and move _internal/errors_test.go_ to _internal/errutil/error_test.go_

Just to confirm, also tests may not do the 2-way imports:
```
# github.com/ceph/go-ceph/internal/errutil
package github.com/ceph/go-ceph/internal/errutil
	imports github.com/ceph/go-ceph/cephfs
	imports github.com/ceph/go-ceph/internal/errutil: import cycle not allowed in test
*** ERROR: returned 1
*** ERROR: internal/errutil tests failed
```

> In that case how about we rewrite errors_test.go in the direction of strerror_test.go and put in under internal/errutil?

The main thing it tests is comparing errors between the `rados`, `rbd` and `cephfs` packages. Such a test needs to live outside of the `internal/errutil` package to prevent the _import cycle_.

Do you have an other idea where to place `errors_test.go`?

> I agree but those APIs would still remain listed in docs/api-status.json.

The references to internal error types have been removed now.
anoopcs9: > Hey @anoopcs9,
> 
> > ```
> > * rename and move _internal/errors_test.go_ to _internal/errutil/error_test.go_
> > ```
> 
> Just to confirm, also tests may not do the 2-way imports:
> 
> ```
> # github.com/ceph/go-ceph/internal/errutil
> package github.com/ceph/go-ceph/internal/errutil
> 	imports github.com/ceph/go-ceph/cephfs
> 	imports github.com/ceph/go-ceph/internal/errutil: import cycle not allowed in test
> *** ERROR: returned 1
> *** ERROR: internal/errutil tests failed
> ```
> 
> > In that case how about we rewrite errors_test.go in the direction of strerror_test.go and put in under internal/errutil?
> 
> The main thing it tests is comparing errors between the `rados`, `rbd` and `cephfs` packages. Such a test needs to live outside of the `internal/errutil` package to prevent the _import cycle_.
> 
> Do you have an other idea where to place `errors_test.go`?

May be we choose option 1 to rename and move _internal/errors_test.go_ to _internal/errutil/error_test.go_ as follows:

```
package errutil

import (
	"errors"
	"fmt"
	"testing"

	"github.com/stretchr/testify/assert"
)

func TestCephError(t *testing.T) {
	errstr := cephErrno(107).Error()
	assert.Equal(t, "Transport endpoint is not connected", errstr)

	cephFSErr := GetError("cephfs", 2)
	assert.Equal(t, "cephfs: ret=2, No such file or directory",
		cephFSErr.Error())

	rbdErr := fmt.Errorf("RBD image not found: %w",
			GetError("rbd", 2))
	assert.True(t, errors.Is(cephFSErr, errors.Unwrap(rbdErr)))
}
```

> > I agree but those APIs would still remain listed in docs/api-status.json.
> 
> The references to internal error types have been removed now.

Are you sure you pushed those changes? I don't see any update here.
phlogistonjohn: Gentle reminder that the next release is on 2024-10-15. @anoopcs9 and @ansiwen please provide any feedback & @nixpanic check existing feedback please. If it misses the train, it misses the train (there will be another coming along later).
anoopcs9: @Mergifyio rebase
mergify[bot]: > rebase

#### ✅ Branch has been successfully rebased



<!---
DO NOT EDIT
-*- Mergify Payload -*-
{"command": "rebase", "conclusion": "success", "action_is_running": true}
-*- Mergify Payload End -*-
-->
anoopcs9: > > > I agree but those APIs would still remain listed in docs/api-status.json.
> > 
> > The references to internal error types have been removed now.
> 
> Are you sure you pushed those changes? I don't see any update here.

Oh..I totally forgot this afterwards. Do you want to take this final step?
anoopcs9: @Mergifyio rebase
mergify[bot]: > rebase

#### ✅ Branch has been successfully rebased



<!---
DO NOT EDIT
-*- Mergify Payload -*-
{"command": "rebase", "conclusion": "success", "action_is_running": true}
-*- Mergify Payload End -*-
-->

Patch:
diff --git a/cephfs/errors.go b/cephfs/errors.go
index b139229..ab28f09 100644
--- a/cephfs/errors.go
+++ b/cephfs/errors.go
@@ -11,23 +11,8 @@ import (
 	"github.com/ceph/go-ceph/internal/errutil"
 )
 
-// cephFSError represents an error condition returned from the CephFS APIs.
-type cephFSError int
-
-// Error returns the error string for the cephFSError type.
-func (e cephFSError) Error() string {
-	return errutil.FormatErrorCode("cephfs", int(e))
-}
-
-func (e cephFSError) ErrorCode() int {
-	return int(e)
-}
-
 func getError(e C.int) error {
-	if e == 0 {
-		return nil
-	}
-	return cephFSError(e)
+	return errutil.GetError("cephfs", int(e))
 }
 
 // getErrorIfNegative converts a ceph return code to error if negative.
@@ -46,22 +31,16 @@ var (
 	// ErrEmptyArgument may be returned if a function argument is passed
 	// a zero-length slice or map.
 	ErrEmptyArgument = errors.New("Argument must contain at least one item")
-)
 
-// Public CephFSErrors:
-
-const (
 	// ErrNotConnected may be returned when client is not connected
 	// to a cluster.
-	ErrNotConnected = cephFSError(-C.ENOTCONN)
+	ErrNotConnected = getError(-C.ENOTCONN)
 	// ErrNotExist indicates a non-specific missing resource.
-	ErrNotExist = cephFSError(-C.ENOENT)
-)
+	ErrNotExist = getError(-C.ENOENT)
 
-// Private errors:
+	// Private errors:
 
-const (
-	errInvalid     = cephFSError(-C.EINVAL)
-	errNameTooLong = cephFSError(-C.ENAMETOOLONG)
-	errRange       = cephFSError(-C.ERANGE)
+	errInvalid     = getError(-C.EINVAL)
+	errNameTooLong = getError(-C.ENAMETOOLONG)
+	errRange       = getError(-C.ERANGE)
 )
diff --git a/docs/api-status.json b/docs/api-status.json
index 8713bbb..4cccad4 100644
--- a/docs/api-status.json
+++ b/docs/api-status.json
@@ -117,13 +117,6 @@
         "name": "Directory.RewindDir",
         "comment": "RewindDir sets the directory stream to the beginning of the directory.\n\nImplements:\n void ceph_rewinddir(struct ceph_mount_info *cmount, struct ceph_dir_result *dirp);\n"
       },
-      {
-        "name": "cephFSError.Error",
-        "comment": "Error returns the error string for the cephFSError type.\n"
-      },
-      {
-        "name": "cephFSError.ErrorCode"
-      },
       {
         "name": "MountInfo.Open",
         "comment": "Open a file at the given path. The flags are the same os flags as\na local open call. Mode is the same mode bits as a local open call.\n\nImplements:\n int ceph_open(struct ceph_mount_info *cmount, const char *path, int flags, mode_t mode);\n"
@@ -748,13 +741,6 @@
         "name": "Conn.GetPoolByID",
         "comment": "GetPoolByID returns the name of a pool by a given ID.\n"
       },
-      {
-        "name": "radosError.Error",
-        "comment": "Error returns the error string for the radosError type.\n"
-      },
-      {
-        "name": "radosError.ErrorCode"
-      },
       {
         "name": "IOContext.Pointer",
         "comment": "Pointer returns a pointer reference to an internal structure.\nThis function should NOT be used outside of go-ceph itself.\n"
@@ -1210,12 +1196,6 @@
         "name": "Image.EncryptionLoad",
         "comment": "EncryptionLoad enables IO on an open encrypted image\n\nImplements:\n int rbd_encryption_load(rbd_image_t image,\n                         rbd_encryption_format_t format,\n                         rbd_encryption_options_t opts,\n                         size_t opts_size);\n"
       },
-      {
-        "name": "rbdError.Error"
-      },
-      {
-        "name": "rbdError.ErrorCode"
-      },
       {
         "name": "FeatureSetFromNames",
         "comment": "FeatureSetFromNames returns a FeatureSet built from flag bits corresponding\nto the provided feature names.\n"
@@ -2376,4 +2356,4 @@
       }
     ]
   }
-}
\ No newline at end of file
+}
diff --git a/internal/errutil/error.go b/internal/errutil/error.go
new file mode 100644
index 0000000..de9a7b9
--- /dev/null
+++ b/internal/errutil/error.go
@@ -0,0 +1,57 @@
+package errutil
+
+type cephErrno int
+
+// Error returns the error string for the errno.
+func (e cephErrno) Error() string {
+	_, strerror := FormatErrno(int(e))
+	return strerror
+}
+
+// cephError combines the source/component that generated the error and its
+// related errno.
+type cephError struct {
+	source string
+	errno  cephErrno
+}
+
+// Error returns the error string with the source and errno.
+func (e cephError) Error() string {
+	return FormatErrorCode(e.source, int(e.errno))
+}
+
+// Unwrap returns an error without the source.
+func (e cephError) Unwrap() error {
+	if e.errno == 0 {
+		return nil
+	}
+
+	return e.errno
+}
+
+// Is checks if both errors have the same errno.
+func (e cephError) Is(err error) bool {
+	ce, ok := err.(cephError)
+	if !ok {
+		return false
+	}
+
+	return e.errno == ce.errno
+}
+
+// ErrorCode returns the errno of the error.
+func (e cephError) ErrorCode() int {
+	return int(e.errno)
+}
+
+// GetError returns a new error that can be compared with errors.Is(),
+// independently of the source/component of the error.
+func GetError(source string, e int) error {
+	if e == 0 {
+		return nil
+	}
+	return cephError{
+		source: source,
+		errno:  cephErrno(int(e)),
+	}
+}
diff --git a/rados/conn.go b/rados/conn.go
index b866a79..8e58017 100644
--- a/rados/conn.go
+++ b/rados/conn.go
@@ -291,11 +291,11 @@ func (c *Conn) GetPoolByName(name string) (int64, error) {
 	}
 	cName := C.CString(name)
 	defer C.free(unsafe.Pointer(cName))
-	ret := int64(C.rados_pool_lookup(c.cluster, cName))
+	ret := C.rados_pool_lookup(c.cluster, cName)
 	if ret < 0 {
-		return 0, radosError(ret)
+		return 0, getError(C.int(ret))
 	}
-	return ret, nil
+	return int64(ret), nil
 }
 
 // GetPoolByID returns the name of a pool by a given ID.
@@ -305,9 +305,9 @@ func (c *Conn) GetPoolByID(id int64) (string, error) {
 		return "", err
 	}
 	cid := C.int64_t(id)
-	ret := int(C.rados_pool_reverse_lookup(c.cluster, cid, (*C.char)(unsafe.Pointer(&buf[0])), C.size_t(len(buf))))
+	ret := C.rados_pool_reverse_lookup(c.cluster, cid, (*C.char)(unsafe.Pointer(&buf[0])), C.size_t(len(buf)))
 	if ret < 0 {
-		return "", radosError(ret)
+		return "", getError(ret)
 	}
 	return C.GoString((*C.char)(unsafe.Pointer(&buf[0]))), nil
 }
diff --git a/rados/errors.go b/rados/errors.go
index d0de0c8..659eaab 100644
--- a/rados/errors.go
+++ b/rados/errors.go
@@ -11,41 +11,12 @@ import (
 	"github.com/ceph/go-ceph/internal/errutil"
 )
 
-// radosError represents an error condition returned from the Ceph RADOS APIs.
-type radosError int
-
-// Error returns the error string for the radosError type.
-func (e radosError) Error() string {
-	return errutil.FormatErrorCode("rados", int(e))
-}
-
-func (e radosError) ErrorCode() int {
-	return int(e)
-}
-
-func getError(e C.int) error {
-	if e == 0 {
-		return nil
-	}
-	return radosError(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 (
 	// ErrNotConnected is returned when functions are called
 	// without a RADOS connection.
-	ErrNotConnected = errors.New("RADOS not connected")
+	ErrNotConnected = getError(-C.ENOTCONN)
 	// ErrEmptyArgument may be returned if a function argument is passed
 	// a zero-length slice or map.
 	ErrEmptyArgument = errors.New("Argument must contain at least one item")
@@ -55,17 +26,13 @@ var (
 	// ErrOperationIncomplete is returned from write op or read op steps for
 	// which the operation has not been performed yet.
 	ErrOperationIncomplete = errors.New("Operation has not been performed yet")
-)
-
-// Public radosErrors:
 
-const (
 	// ErrNotFound indicates a missing resource.
-	ErrNotFound = radosError(-C.ENOENT)
+	ErrNotFound = getError(-C.ENOENT)
 	// ErrPermissionDenied indicates a permissions issue.
-	ErrPermissionDenied = radosError(-C.EPERM)
+	ErrPermissionDenied = getError(-C.EPERM)
 	// ErrObjectExists indicates that an exclusive object creation failed.
-	ErrObjectExists = radosError(-C.EEXIST)
+	ErrObjectExists = getError(-C.EEXIST)
 
 	// RadosErrorNotFound indicates a missing resource.
 	//
@@ -75,12 +42,23 @@ const (
 	//
 	// Deprecated: use ErrPermissionDenied instead
 	RadosErrorPermissionDenied = ErrPermissionDenied
-)
-
-// Private errors:
 
-const (
-	errNameTooLong = radosError(-C.ENAMETOOLONG)
+	// Private errors:
 
-	errRange = radosError(-C.ERANGE)
+	errNameTooLong = getError(-C.ENAMETOOLONG)
+	errRange       = getError(-C.ERANGE)
 )
+
+func getError(errno C.int) error {
+	return errutil.GetError("rados", int(errno))
+}
+
+// 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)
+}
diff --git a/rados/ioctx.go b/rados/ioctx.go
index e6e35bf..910372d 100644
--- a/rados/ioctx.go
+++ b/rados/ioctx.go
@@ -640,7 +640,7 @@ func (ioctx *IOContext) ListLockers(oid, name string) (*LockInfo, error) {
 	}
 
 	if ret < 0 {
-		return nil, radosError(ret)
+		return nil, getError(C.int(ret))
 	}
 	return &LockInfo{int(ret), cExclusive == 1, C.GoString(cTag), splitCString(cClients, cClientsLen), splitCString(cCookies, cCookiesLen), splitCString(cAddrs, cAddrsLen)}, nil
 }
diff --git a/rbd/clone_image_by_id.go b/rbd/clone_image_by_id.go
index 63cf552..f1adf48 100644
--- a/rbd/clone_image_by_id.go
+++ b/rbd/clone_image_by_id.go
@@ -52,7 +52,7 @@ var (
 func CloneImageByID(ioctx *rados.IOContext, parentName string, snapID uint64,
 	destctx *rados.IOContext, name string, rio *ImageOptions) error {
 	if rio == nil {
-		return rbdError(C.EINVAL)
+		return getError(C.EINVAL)
 	}
 
 	rbdClone4Once.Do(func() {
diff --git a/rbd/diff_iterate.go b/rbd/diff_iterate.go
index bb20388..60a29eb 100644
--- a/rbd/diff_iterate.go
+++ b/rbd/diff_iterate.go
@@ -96,7 +96,7 @@ func (image *Image) DiffIterate(config DiffIterateConfig) error {
 		return err
 	}
 	if config.Callback == nil {
-		return rbdError(C.EINVAL)
+		return getError(C.EINVAL)
 	}
 
 	var cSnapName *C.char
diff --git a/rbd/errors.go b/rbd/errors.go
index e26e9d1..e2a3b7f 100644
--- a/rbd/errors.go
+++ b/rbd/errors.go
@@ -7,27 +7,17 @@ import "C"
 
 import (
 	"errors"
+	"fmt"
 
 	"github.com/ceph/go-ceph/internal/errutil"
 )
 
-// rbdError represents an error condition returned from the librbd APIs.
-type rbdError int
-
-func (e rbdError) Error() string {
-	return errutil.FormatErrorCode("rbd", int(e))
-}
-
-func (e rbdError) ErrorCode() int {
-	return int(e)
-}
-
 func getError(err C.int) error {
 	if err != 0 {
 		if err == -C.ENOENT {
 			return ErrNotFound
 		}
-		return rbdError(err)
+		return errutil.GetError("rbd", int(err))
 	}
 	return nil
 }
@@ -60,7 +50,7 @@ var (
 	ErrImageIsOpen = errors.New("RBD image is open")
 	// ErrNotFound may be returned from an api call when the requested item is
 	// missing.
-	ErrNotFound = errors.New("RBD image not found")
+	ErrNotFound = fmt.Errorf("RBD image not found: %w", errutil.GetError("rbd", -C.ENOENT))
 	// ErrNoNamespaceName maye be returned if an api call requires a namespace
 	// name and it is not provided.
 	ErrNoNamespaceName = errors.New("Namespace value is missing")
@@ -69,20 +59,15 @@ var (
 	RbdErrorImageNotOpen = ErrImageNotOpen
 	RbdErrorNotFound     = ErrNotFound
 	// revive:enable:exported
-)
 
-// Public general error
-const (
 	// ErrExist indicates a non-specific already existing resource.
-	ErrExist = rbdError(-C.EEXIST)
+	ErrExist = getError(-C.EEXIST)
 	// ErrNotExist indicates a non-specific missing resource.
-	ErrNotExist = rbdError(-C.ENOENT)
+	ErrNotExist = getError(-C.ENOENT)
 	// ErrNotImplemented indicates a function is not implemented in by librbd.
-	ErrNotImplemented = rbdError(-C.ENOSYS)
-)
+	ErrNotImplemented = getError(-C.ENOSYS)
 
-// Private errors:
+	// Private errors:
 
-const (
-	errRange = rbdError(-C.ERANGE)
+	errRange = getError(-C.ERANGE)
 )
diff --git a/rbd/features.go b/rbd/features.go
index 6bfb10d..ca9a93a 100644
--- a/rbd/features.go
+++ b/rbd/features.go
@@ -171,7 +171,7 @@ func (image *Image) GetFeatures() (features uint64, err error) {
 	}
 
 	if ret := C.rbd_get_features(image.image, (*C.uint64_t)(&features)); ret < 0 {
-		return 0, rbdError(ret)
+		return 0, getError(ret)
 	}
 
 	return features, nil
diff --git a/rbd/group_snap.go b/rbd/group_snap.go
index 6948fa7..a9994b9 100644
--- a/rbd/group_snap.go
+++ b/rbd/group_snap.go
@@ -204,7 +204,7 @@ func GroupSnapRollbackWithProgress(
 	cb GroupSnapRollbackCallback, data interface{}) error {
 	// the provided callback must be a real function
 	if cb == nil {
-		return rbdError(C.EINVAL)
+		return getError(C.EINVAL)
 	}
 
 	cGroupName := C.CString(group)
diff --git a/rbd/metadata.go b/rbd/metadata.go
index 5419dc2..4573205 100644
--- a/rbd/metadata.go
+++ b/rbd/metadata.go
@@ -62,7 +62,7 @@ func (image *Image) SetMetadata(key string, value string) error {
 
 	ret := C.rbd_metadata_set(image.image, cKey, cValue)
 	if ret < 0 {
-		return rbdError(ret)
+		return getError(ret)
 	}
 
 	return nil
@@ -83,7 +83,7 @@ func (image *Image) RemoveMetadata(key string) error {
 
 	ret := C.rbd_metadata_remove(image.image, cKey)
 	if ret < 0 {
-		return rbdError(ret)
+		return getError(ret)
 	}
 
 	return nil
diff --git a/rbd/rbd.go b/rbd/rbd.go
index 60582fa..d90882b 100644
--- a/rbd/rbd.go
+++ b/rbd/rbd.go
@@ -178,7 +178,7 @@ func Create(ioctx *rados.IOContext, name string, size uint64, order int,
 	}
 
 	if ret < 0 {
-		return nil, rbdError(ret)
+		return nil, getError(ret)
 	}
 
 	return &Image{
@@ -205,7 +205,7 @@ func Create2(ioctx *rados.IOContext, name string, size uint64, features uint64,
 	ret = C.rbd_create2(cephIoctx(ioctx), cName,
 		C.uint64_t(size), C.uint64_t(features), &cOrder)
 	if ret < 0 {
-		return nil, rbdError(ret)
+		return nil, getError(ret)
 	}
 
 	return &Image{
@@ -235,7 +235,7 @@ func Create3(ioctx *rados.IOContext, name string, size uint64, features uint64,
 		C.uint64_t(size), C.uint64_t(features), &cOrder,
 		C.uint64_t(stripeUnit), C.uint64_t(stripeCount))
 	if ret < 0 {
-		return nil, rbdError(ret)
+		return nil, getError(ret)
 	}
 
 	return &Image{
@@ -274,7 +274,7 @@ func (image *Image) Clone(snapname string, cIoctx *rados.IOContext, cName string
 		C.uint64_t(features),
 		&cOrder)
 	if ret < 0 {
-		return nil, rbdError(ret)
+		return nil, getError(ret)
 	}
 
 	return &Image{
@@ -325,9 +325,9 @@ func (image *Image) Rename(destname string) error {
 	defer C.free(unsafe.Pointer(cSrcName))
 	defer C.free(unsafe.Pointer(cDestName))
 
-	err := rbdError(C.rbd_rename(cephIoctx(image.ioctx),
+	err := getError(C.rbd_rename(cephIoctx(image.ioctx),
 		cSrcName, cDestName))
-	if err == 0 {
+	if err == nil {
 		image.name = destname
 		return nil
 	}
@@ -385,7 +385,7 @@ func (image *Image) Close() error {
 	}
 
 	if ret := C.rbd_close(image.image); ret != 0 {
-		return rbdError(ret)
+		return getError(ret)
 	}
 
 	image.image = nil
@@ -418,7 +418,7 @@ func (image *Image) Stat() (info *ImageInfo, err error) {
 	var cStat C.rbd_image_info_t
 
 	if ret := C.rbd_stat(image.image, &cStat, C.size_t(unsafe.Sizeof(info))); ret < 0 {
-		return info, rbdError(ret)
+		return info, getError(ret)
 	}
 
 	return &ImageInfo{
@@ -443,7 +443,7 @@ func (image *Image) IsOldFormat() (bool, error) {
 	ret := C.rbd_get_old_format(image.image,
 		&cOldFormat)
 	if ret < 0 {
-		return false, rbdError(ret)
+		return false, getError(ret)
 	}
 
 	return cOldFormat != 0, nil
@@ -460,7 +460,7 @@ func (image *Image) GetSize() (size uint64, err error) {
 	}
 
 	if ret := C.rbd_get_size(image.image, (*C.uint64_t)(&size)); ret < 0 {
-		return 0, rbdError(ret)
+		return 0, getError(ret)
 	}
 
 	return size, nil
@@ -478,7 +478,7 @@ func (image *Image) GetStripeUnit() (uint64, error) {
 
 	var stripeUnit uint64
 	if ret := C.rbd_get_stripe_unit(image.image, (*C.uint64_t)(&stripeUnit)); ret < 0 {
-		return 0, rbdError(ret)
+		return 0, getError(ret)
 	}
 
 	return stripeUnit, nil
@@ -496,7 +496,7 @@ func (image *Image) GetStripeCount() (uint64, error) {
 
 	var stripeCount uint64
 	if ret := C.rbd_get_stripe_count(image.image, (*C.uint64_t)(&stripeCount)); ret < 0 {
-		return 0, rbdError(ret)
+		return 0, getError(ret)
 	}
 
 	return stripeCount, nil
@@ -514,7 +514,7 @@ func (image *Image) GetOverlap() (overlap uint64, err error) {
 	}
 
 	if ret := C.rbd_get_overlap(image.image, (*C.uint64_t)(&overlap)); ret < 0 {
-		return overlap, rbdError(ret)
+		return overlap, getError(ret)
 	}
 
 	return overlap, nil
@@ -573,7 +573,7 @@ func (image *Image) DeepCopy(ioctx *rados.IOContext, destname string, rio *Image
 		return ErrNoName
 	}
 	if rio == nil {
-		return rbdError(C.EINVAL)
+		return getError(C.EINVAL)
 	}
 
 	cDestname := C.CString(destname)
@@ -644,8 +644,8 @@ func (image *Image) ListLockers() (tag string, lockers []Locker, err error) {
 	// and *0* means no locker held on rbd image.
 	// but *0* is unexpected here because first rbd_list_lockers already
 	// dealt with no locker case
-	if int(cLockerCount) <= 0 {
-		return "", nil, rbdError(cLockerCount)
+	if cLockerCount <= 0 {
+		return "", nil, getError(C.int(cLockerCount))
 	}
 
 	clients := cutil.SplitSparseBuffer(clientsBuf)
@@ -754,7 +754,7 @@ func (image *Image) Read(data []byte) (int, error) {
 		(*C.char)(unsafe.Pointer(&data[0]))))
 
 	if ret < 0 {
-		return 0, rbdError(ret)
+		return 0, getError(C.int(ret))
 	}
 
 	image.offset += int64(ret)
@@ -786,7 +786,7 @@ func (image *Image) Write(data []byte) (n int, err error) {
 	}
 
 	if ret != len(data) {
-		err = rbdError(-C.EPERM)
+		err = getError(-C.EPERM)
 	}
 
 	return ret, err
@@ -825,7 +825,7 @@ func (image *Image) Discard(ofs uint64, length uint64) (int, error) {
 
 	ret := C.rbd_discard(image.image, C.uint64_t(ofs), C.uint64_t(length))
 	if ret < 0 {
-		return 0, rbdError(ret)
+		return 0, getError(ret)
 	}
 
 	return int(ret), nil
@@ -848,7 +848,7 @@ func (image *Image) ReadAt(data []byte, off int64) (int, error) {
 		(*C.char)(unsafe.Pointer(&data[0]))))
 
 	if ret < 0 {
-		return 0, rbdError(ret)
+		return 0, getError(C.int(ret))
 	}
 
 	if ret < len(data) {
@@ -872,7 +872,7 @@ func (image *Image) WriteAt(data []byte, off int64) (n int, err error) {
 		C.size_t(len(data)), (*C.char)(unsafe.Pointer(&data[0]))))
 
 	if ret != len(data) {
-		err = rbdError(-C.EPERM)
+		err = getError(-C.EPERM)
 	}
 
 	return ret, err
@@ -938,7 +938,7 @@ func (image *Image) GetSnapshotNames() (snaps []SnapInfo, err error) {
 	ret := C.rbd_snap_list(image.image, nil, &cMaxSnaps)
 	// bugfix index out of range(&cSnaps[0])
 	if cMaxSnaps < 1 {
-		return nil, rbdError(ret)
+		return nil, getError(ret)
 	}
 	cSnaps := make([]C.rbd_snap_info_t, cMaxSnaps)
 	snaps = make([]SnapInfo, cMaxSnaps)
@@ -946,7 +946,7 @@ func (image *Image) GetSnapshotNames() (snaps []SnapInfo, err error) {
 	ret = C.rbd_snap_list(image.image,
 		&cSnaps[0], &cMaxSnaps)
 	if ret < 0 {
-		return nil, rbdError(ret)
+		return nil, getError(ret)
 	}
 
 	for i, s := range cSnaps {
@@ -1253,7 +1253,7 @@ func CreateImage(ioctx *rados.IOContext, name string, size uint64, rio *ImageOpt
 		return ErrNoName
 	}
 	if rio == nil {
-		return rbdError(C.EINVAL)
+		return getError(C.EINVAL)
 	}
 
 	cName := C.CString(name)
@@ -1294,7 +1294,7 @@ func CloneImage(ioctx *rados.IOContext, parentName, snapName string,
 	destctx *rados.IOContext, name string, rio *ImageOptions) error {
 
 	if rio == nil {
-		return rbdError(C.EINVAL)
+		return getError(C.EINVAL)
 	}
 
 	cParentName := C.CString(parentName)
diff --git a/rbd/resize.go b/rbd/resize.go
index 00f1187..86b1cee 100644
--- a/rbd/resize.go
+++ b/rbd/resize.go
@@ -57,7 +57,7 @@ func resize2Callback(
 func (image *Image) Resize2(size uint64, allowShrink bool, cb Resize2ProgressCallback, data interface{}) error {
 	// the provided callback must be a real function
 	if cb == nil {
-		return rbdError(C.EINVAL)
+		return getError(C.EINVAL)
 	}
 
 	if err := image.validate(imageIsOpen); err != nil {
diff --git a/rbd/snapshot.go b/rbd/snapshot.go
index d321b59..86de2b9 100644
--- a/rbd/snapshot.go
+++ b/rbd/snapshot.go
@@ -33,7 +33,7 @@ func (image *Image) CreateSnapshot(snapname string) (*Snapshot, error) {
 
 	ret := C.rbd_snap_create(image.image, cSnapName)
 	if ret < 0 {
-		return nil, rbdError(ret)
+		return nil, getError(ret)
 	}
 
 	return &Snapshot{
@@ -147,7 +147,7 @@ func (snapshot *Snapshot) IsProtected() (bool, error) {
 	ret := C.rbd_snap_is_protected(snapshot.image.image, cSnapName,
 		&cIsProtected)
 	if ret < 0 {
-		return false, rbdError(ret)
+		return false, getError(ret)
 	}
 
 	return cIsProtected != 0, nil
diff --git a/rbd/snapshot_nautilus.go b/rbd/snapshot_nautilus.go
index f81e8d9..ce3e11a 100644
--- a/rbd/snapshot_nautilus.go
+++ b/rbd/snapshot_nautilus.go
@@ -32,7 +32,7 @@ func (image *Image) GetParentInfo(pool, name, snapname []byte) error {
 	parentSnap := C.rbd_snap_spec_t{}
 	ret := C.rbd_get_parent(image.image, &parentImage, &parentSnap)
 	if ret != 0 {
-		return rbdError(ret)
+		return getError(ret)
 	}
 
 	defer C.rbd_linked_image_spec_cleanup(&parentImage)
@@ -40,26 +40,26 @@ func (image *Image) GetParentInfo(pool, name, snapname []byte) error {
 
 	strlen := int(C.strlen(parentImage.pool_name))
 	if len(pool) < strlen {
-		return rbdError(C.ERANGE)
+		return getError(C.ERANGE)
 	}
 	if copy(pool, C.GoString(parentImage.pool_name)) != strlen {
-		return rbdError(C.ERANGE)
+		return getError(C.ERANGE)
 	}
 
 	strlen = int(C.strlen(parentImage.image_name))
 	if len(name) < strlen {
-		return rbdError(C.ERANGE)
+		return getError(C.ERANGE)
 	}
 	if copy(name, C.GoString(parentImage.image_name)) != strlen {
-		return rbdError(C.ERANGE)
+		return getError(C.ERANGE)
 	}
 
 	strlen = int(C.strlen(parentSnap.name))
 	if len(snapname) < strlen {
-		return rbdError(C.ERANGE)
+		return getError(C.ERANGE)
 	}
 	if copy(snapname, C.GoString(parentSnap.name)) != strlen {
-		return rbdError(C.ERANGE)
+		return getError(C.ERANGE)
 	}
 
 	return nil
diff --git a/rbd/sparsify.go b/rbd/sparsify.go
index e46ca0d..7435a15 100644
--- a/rbd/sparsify.go
+++ b/rbd/sparsify.go
@@ -60,7 +60,7 @@ func (image *Image) SparsifyWithProgress(
 ) error {
 	// the provided callback must be a real function
 	if cb == nil {
-		return rbdError(C.EINVAL)
+		return getError(C.EINVAL)
 	}
 
 	if err := image.validate(imageIsOpen); err != nil {


Test patch:
diff --git a/internal/errutil/error_test.go b/internal/errutil/error_test.go
new file mode 100644
index 0000000..5c1784e
--- /dev/null
+++ b/internal/errutil/error_test.go
@@ -0,0 +1,22 @@
+package errutil
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestCephError(t *testing.T) {
+	radosErr := cephErrno(107)
+	assert.Equal(t, "Transport endpoint is not connected", radosErr.Error())
+
+	cephFSErr := GetError("cephfs", 2)
+	assert.Equal(t, "cephfs: ret=2, No such file or directory",
+		cephFSErr.Error())
+	assert.Equal(t, 2, cephFSErr.(cephError).ErrorCode())
+
+	rbdErr := GetError("rbd", 2)
+	assert.True(t, errors.Is(cephFSErr, rbdErr))
+	assert.True(t, errors.Unwrap(cephFSErr) == errors.Unwrap(rbdErr))
+}