From e8da7617696a8409fd1a5fc336bf643141d2c079 Mon Sep 17 00:00:00 2001 From: Sven Anderson Date: Fri, 10 Apr 2020 19:35:49 +0200 Subject: [PATCH] update various parts of the code to use the retry lib. Signed-off-by: John Mulligan Signed-off-by: Sven Anderson --- cephfs/cephfs.go | 28 ++++++++----- cephfs/errors.go | 6 +++ rados/conn.go | 33 ++++++++++------ rados/errors.go | 18 +++++++++ rados/ioctx.go | 31 +++++++++------ rbd/errors.go | 16 ++++++++ rbd/rbd.go | 95 ++++++++++++++++++++++++--------------------- rbd/rbd_nautilus.go | 36 +++++++++-------- 8 files changed, 169 insertions(+), 94 deletions(-) diff --git a/cephfs/cephfs.go b/cephfs/cephfs.go index 98e7ac3..03ce64d 100644 --- a/cephfs/cephfs.go +++ b/cephfs/cephfs.go @@ -11,6 +11,7 @@ import "C" import ( "unsafe" + "github.com/ceph/go-ceph/internal/retry" "github.com/ceph/go-ceph/rados" ) @@ -82,15 +83,24 @@ func (mount *MountInfo) SetConfigOption(option, value string) error { func (mount *MountInfo) GetConfigOption(option string) (string, error) { cOption := C.CString(option) defer C.free(unsafe.Pointer(cOption)) - buf := make([]byte, 4096) - // TODO: handle ENAMETOOLONG cases. problem also exists in rados - ret := C.ceph_conf_get( - mount.mount, - cOption, - (*C.char)(unsafe.Pointer(&buf[0])), - C.size_t(len(buf))) - if ret < 0 { - return "", getError(ret) + + var ( + err error + buf []byte + ) + // range from 4k to 256KiB + retry.WithSizes(4096, 1<<18, func(size int) retry.Hint { + buf = make([]byte, size) + ret := C.ceph_conf_get( + mount.mount, + cOption, + (*C.char)(unsafe.Pointer(&buf[0])), + C.size_t(len(buf))) + err = getError(ret) + return retry.DoubleSize.If(err == errNameTooLong) + }) + if err != nil { + return "", err } value := C.GoString((*C.char)(unsafe.Pointer(&buf[0]))) return value, nil diff --git a/cephfs/errors.go b/cephfs/errors.go index a28573e..fa42c3e 100644 --- a/cephfs/errors.go +++ b/cephfs/errors.go @@ -33,3 +33,9 @@ func getError(e C.int) error { } return CephFSError(e) } + +// Private errors: + +const ( + errNameTooLong = CephFSError(-C.ENAMETOOLONG) +) diff --git a/rados/conn.go b/rados/conn.go index cd69a18..16ca738 100644 --- a/rados/conn.go +++ b/rados/conn.go @@ -8,6 +8,8 @@ import "C" import ( "bytes" "unsafe" + + "github.com/ceph/go-ceph/internal/retry" ) // ClusterStat represents Ceph cluster statistics. @@ -140,19 +142,26 @@ func (c *Conn) SetConfigOption(option, value string) error { // GetConfigOption returns the value of the Ceph configuration option // identified by the given name. func (c *Conn) GetConfigOption(name string) (value string, err error) { - buf := make([]byte, 4096) - c_name := C.CString(name) - defer C.free(unsafe.Pointer(c_name)) - ret := int(C.rados_conf_get(c.cluster, c_name, - (*C.char)(unsafe.Pointer(&buf[0])), C.size_t(len(buf)))) - // FIXME: ret may be -ENAMETOOLONG if the buffer is not large enough. We - // can handle this case, but we need a reliable way to test for - // -ENAMETOOLONG constant. Will the syscall/Errno stuff in Go help? - if ret == 0 { - value = C.GoString((*C.char)(unsafe.Pointer(&buf[0]))) - return value, nil + cOption := C.CString(name) + defer C.free(unsafe.Pointer(cOption)) + + var buf []byte + // range from 4k to 256KiB + retry.WithSizes(4096, 1<<18, func(size int) retry.Hint { + buf = make([]byte, size) + ret := C.rados_conf_get( + c.cluster, + cOption, + (*C.char)(unsafe.Pointer(&buf[0])), + C.size_t(len(buf))) + err = getError(ret) + return retry.DoubleSize.If(err == errNameTooLong) + }) + if err != nil { + return "", err } - return "", RadosError(ret) + value = C.GoString((*C.char)(unsafe.Pointer(&buf[0]))) + return value, nil } // WaitForLatestOSDMap blocks the caller until the latest OSD map has been diff --git a/rados/errors.go b/rados/errors.go index d01a425..3cd5f00 100644 --- a/rados/errors.go +++ b/rados/errors.go @@ -35,6 +35,16 @@ func getError(e C.int) error { 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 ( @@ -61,3 +71,11 @@ const ( // Deprecated: use ErrPermissionDenied instead RadosErrorPermissionDenied = ErrPermissionDenied ) + +// Private errors: + +const ( + errNameTooLong = RadosError(-C.ENAMETOOLONG) + + errRange = RadosError(-C.ERANGE) +) diff --git a/rados/ioctx.go b/rados/ioctx.go index 45069a9..b0663cc 100644 --- a/rados/ioctx.go +++ b/rados/ioctx.go @@ -28,6 +28,8 @@ import ( "syscall" "time" "unsafe" + + "github.com/ceph/go-ceph/internal/retry" ) // CreateOption is passed to IOContext.Create() and should be one of @@ -249,19 +251,24 @@ func (ioctx *IOContext) GetPoolStats() (stat PoolStat, err error) { // GetPoolName returns the name of the pool associated with the I/O context. func (ioctx *IOContext) GetPoolName() (name string, err error) { - buf := make([]byte, 128) - for { - ret := C.rados_ioctx_get_pool_name(ioctx.ioctx, - (*C.char)(unsafe.Pointer(&buf[0])), C.unsigned(len(buf))) - if ret == -C.ERANGE { - buf = make([]byte, len(buf)*2) - continue - } else if ret < 0 { - return "", getError(ret) - } - name = C.GoStringN((*C.char)(unsafe.Pointer(&buf[0])), ret) - return name, nil + var ( + buf []byte + ret C.int + ) + retry.WithSizes(128, 8192, func(size int) retry.Hint { + buf = make([]byte, size) + ret = C.rados_ioctx_get_pool_name( + ioctx.ioctx, + (*C.char)(unsafe.Pointer(&buf[0])), + C.unsigned(len(buf))) + err = getErrorIfNegative(ret) + return retry.DoubleSize.If(err == errRange) + }) + if err != nil { + return "", err } + name = C.GoStringN((*C.char)(unsafe.Pointer(&buf[0])), ret) + return name, nil } // ObjectListFunc is the type of the function called for each object visited diff --git a/rbd/errors.go b/rbd/errors.go index 0ad766e..8b7a07f 100644 --- a/rbd/errors.go +++ b/rbd/errors.go @@ -37,6 +37,16 @@ func getError(err C.int) error { return nil } +// 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 ( @@ -60,3 +70,9 @@ var ( RbdErrorNotFound = ErrNotFound // revive:enable:exported ) + +// Private errors: + +const ( + errRange = RBDError(-C.ERANGE) +) diff --git a/rbd/rbd.go b/rbd/rbd.go index df5da19..fc0b9df 100644 --- a/rbd/rbd.go +++ b/rbd/rbd.go @@ -17,6 +17,7 @@ import ( "time" "unsafe" + "github.com/ceph/go-ceph/internal/retry" "github.com/ceph/go-ceph/rados" ) @@ -873,22 +874,24 @@ func (image *Image) GetMetadata(key string) (string, error) { c_key := C.CString(key) defer C.free(unsafe.Pointer(c_key)) - var c_vallen C.size_t - ret := C.rbd_metadata_get(image.image, c_key, nil, (*C.size_t)(&c_vallen)) - // get size of value - // ret -34 because we pass nil as value pointer - if ret != 0 && ret != -C.ERANGE { - return "", RBDError(ret) + var ( + buf []byte + err error + ) + retry.WithSizes(4096, 262144, func(size int) retry.Hint { + csize := C.size_t(size) + buf = make([]byte, csize) + // rbd_metadata_get is a bit quirky and *does not* update the size + // value if the size passed in >= the needed size. + ret := C.rbd_metadata_get( + image.image, c_key, (*C.char)(unsafe.Pointer(&buf[0])), &csize) + err = getError(ret) + return retry.Size(int(csize)).If(err == errRange) + }) + if err != nil { + return "", err } - - // make a bytes array with a good size - value := make([]byte, c_vallen-1) - ret = C.rbd_metadata_get(image.image, c_key, (*C.char)(unsafe.Pointer(&value[0])), (*C.size_t)(&c_vallen)) - if ret < 0 { - return "", RBDError(ret) - } - - return string(value), nil + return C.GoString((*C.char)(unsafe.Pointer(&buf[0]))), nil } // SetMetadata updates the metadata string associated with the given key. @@ -941,42 +944,50 @@ func (image *Image) GetId() (string, error) { if err := image.validate(imageIsOpen); err != nil { return "", err } - size := C.size_t(1024) - buf := make([]byte, size) - for { + var ( + err error + buf []byte + ) + retry.WithSizes(1, 8192, func(size int) retry.Hint { + buf = make([]byte, size) ret := C.rbd_get_id( image.image, (*C.char)(unsafe.Pointer(&buf[0])), - size) - if ret == -C.ERANGE && size <= 8192 { - size *= 2 - buf = make([]byte, size) - } else if ret < 0 { - return "", getError(ret) - } - id := C.GoString((*C.char)(unsafe.Pointer(&buf[0]))) - return id, nil + C.size_t(size)) + err = getErrorIfNegative(ret) + return retry.DoubleSize.If(err == errRange) + }) + if err != nil { + return "", err } + id := C.GoString((*C.char)(unsafe.Pointer(&buf[0]))) + return id, nil + } // GetTrashList returns a slice of TrashInfo structs, containing information about all RBD images // currently residing in the trash. func GetTrashList(ioctx *rados.IOContext) ([]TrashInfo, error) { - var num_entries C.size_t - - // Call rbd_trash_list with nil pointer to get number of trash entries. - if C.rbd_trash_list(cephIoctx(ioctx), nil, &num_entries); num_entries == 0 { - return nil, nil + var ( + err error + count C.size_t + entries []C.rbd_trash_image_info_t + ) + retry.WithSizes(32, 1024, func(size int) retry.Hint { + count = C.size_t(size) + entries = make([]C.rbd_trash_image_info_t, count) + ret := C.rbd_trash_list(cephIoctx(ioctx), &entries[0], &count) + err = getErrorIfNegative(ret) + return retry.Size(int(count)).If(err == errRange) + }) + if err != nil { + return nil, err } + // Free rbd_trash_image_info_t pointers + defer C.rbd_trash_list_cleanup(&entries[0], count) - c_entries := make([]C.rbd_trash_image_info_t, num_entries) - trashList := make([]TrashInfo, num_entries) - - if ret := C.rbd_trash_list(cephIoctx(ioctx), &c_entries[0], &num_entries); ret < 0 { - return nil, RBDError(ret) - } - - for i, ti := range c_entries { + trashList := make([]TrashInfo, count) + for i, ti := range entries[:count] { trashList[i] = TrashInfo{ Id: C.GoString(ti.id), Name: C.GoString(ti.name), @@ -984,10 +995,6 @@ func GetTrashList(ioctx *rados.IOContext) ([]TrashInfo, error) { DefermentEndTime: time.Unix(int64(ti.deferment_end_time), 0), } } - - // Free rbd_trash_image_info_t pointers - C.rbd_trash_list_cleanup(&c_entries[0], num_entries) - return trashList, nil } diff --git a/rbd/rbd_nautilus.go b/rbd/rbd_nautilus.go index 95c07e2..cf479a0 100644 --- a/rbd/rbd_nautilus.go +++ b/rbd/rbd_nautilus.go @@ -11,34 +11,36 @@ package rbd import "C" import ( - "fmt" "unsafe" + "github.com/ceph/go-ceph/internal/retry" "github.com/ceph/go-ceph/rados" ) // GetImageNames returns the list of current RBD images. func GetImageNames(ioctx *rados.IOContext) ([]string, error) { - size := C.size_t(0) - ret := C.rbd_list2(cephIoctx(ioctx), nil, &size) - if ret < 0 && ret != -C.ERANGE { - return nil, RBDError(ret) - } else if ret > 0 { - return nil, fmt.Errorf("rbd_list2() returned %d names, expected 0", ret) - } else if ret == 0 && size == 0 { - return nil, nil - } - - // expected: ret == -ERANGE, size contains number of image names - images := make([]C.rbd_image_spec_t, size) - ret = C.rbd_list2(cephIoctx(ioctx), (*C.rbd_image_spec_t)(unsafe.Pointer(&images[0])), &size) - if ret < 0 { - return nil, RBDError(ret) + var ( + err error + images []C.rbd_image_spec_t + size C.size_t + ) + retry.WithSizes(32, 4096, func(s int) retry.Hint { + size = C.size_t(s) + images = make([]C.rbd_image_spec_t, size) + ret := C.rbd_list2( + cephIoctx(ioctx), + (*C.rbd_image_spec_t)(unsafe.Pointer(&images[0])), + &size) + err = getErrorIfNegative(ret) + return retry.Size(int(size)).If(err == errRange) + }) + if err != nil { + return nil, err } defer C.rbd_image_spec_list_cleanup((*C.rbd_image_spec_t)(unsafe.Pointer(&images[0])), size) names := make([]string, size) - for i, image := range images { + for i, image := range images[:size] { names[i] = C.GoString(image.name) } return names, nil