Repository: ceph/go-ceph
Pull request #220 (2020-04-10 17:42:38)

Retry support for enlarging buffers
Alternative approach to #191 


phlogistonjohn: I've looked over the code one but want to meditate on it some more. One thing that I noticed it that a lot of the stuff done to make my for-loop based approach and the callback based approach, outside of the loop or the callback are actually quite similar. I think this is good. I think one main difference is I was intent on handling the "signaling"  on one line - thus the structure of Update/UpdateWants in my pr. However, looking at this shows that just assigning err in the callback and then doing the retry signaling on the next line doesn't look bad at all.

My current biggest complaint is the return type of HintIf / the callback and some of the naming. My gut says that using just an int for the signaling is wee bit concerning.
I do like the nature of the "hint function" as sugar... but there's something about the flow that doesn't entirely sit right with me.

Perhaps, using a custom type on int, let's call it Hint [1], can make it clearer about how this is used for control as well as data. Maybe even have something like `const Stop = Hint(-1)`, `const Continue = Hint(0)` thus the `Continue` hint value means "I got no proper hint value but keep going" and all other hint int >=1 are valid hints (ex. `Hint(42)`). 

One additional small (possible) nit: I don't think this really solves the error-comparison-can-panic issue you pointed out on the other pr does it? It's not a big deal either here or there to me I just want to see if I'm missing something.

Regardless I want a little while to mull over this and I think I'd like to hear @nixpanic thoughts as well.

1- pun intended
nixpanic: I do not have a strong preference between this approach or the one in #191.

However, I do agree with the hints as an integer, that should be more user friendly. Maybe use `err`, `nil` and a special 'error' like [`SkipDir` in `Walk()`/`WalkFunc`](https://golang.org/pkg/path/filepath/#Walk)?
ansiwen: > My current biggest complaint is the return type of HintIf / the callback and some of the naming. My gut says that using just an int for the signaling is wee bit concerning.
> I do like the nature of the "hint function" as sugar... but there's something about the flow that doesn't entirely sit right with me.

As you know from our conversation about the empty string as a "signalling" and my objections about it in an earlier PR, I'm all in to separate data and control. In this case I hesitated to make it more complex, because I actually though (from our earlier conversation) that you will find it unnecessary. Also note, that there is no "special handling" of certain int values, it all naturally comes out of the code logic and semantically makes sense: hint=0 means: "I need at least 0 more, so you can stop", hint=1 means "I need at least 1 more (but don't know how much), so please retry", hint=x means "I need at least x more, so please retry". So it felt smooth for me. But as I said, I'm generally a fan of typing arguments, if you prefer that.

> Perhaps, using a custom type on int, let's call it Hint [1], can make it clearer about how this is used for control as well as data. Maybe even have something like `const Stop = Hint(-1)`, `const Continue = Hint(0)` thus the `Continue` hint value means "I got no proper hint value but keep going" and all other hint int >=1 are valid hints (ex. `Hint(42)`).

Sure, can do that.

> One additional small (possible) nit: I don't think this really solves the error-comparison-can-panic issue you pointed out on the other pr does it? It's not a big deal either here or there to me I just want to see if I'm missing something.

No, as I said in my comment in #191, this doesn't solve it at all, but it makes it more explicit, because usually the error is also defined in the same context were the == operator is used, therefore it feels less risky (and more flexible) to me.

> Regardless I want a little while to mull over this and I think I'd like to hear @nixpanic thoughts as well.

Sure, let me know when it settled a bit, then I will adapt the PR accordingly.

ansiwen: > I do not have a strong preference between this approach or the one in #191.
> 
> However, I do agree with the hints as an integer, that should be more user friendly. Maybe use `err`, `nil` and a special 'error' like [`SkipDir` in `Walk()`/`WalkFunc`](https://golang.org/pkg/path/filepath/#Walk)?

Thanks for the pointer. I now made a version where `SizeFunc` returns an objects of the `Hint` interface if a retry is required, or `nil` if no retry is required. PTAL
phlogistonjohn: @nixpanic do you have anything to add here? If not I'd like to merge this by my EOD today.

Patch:
diff --git a/Makefile b/Makefile
index 528cfae..5e8cf5f 100644
--- a/Makefile
+++ b/Makefile
@@ -42,7 +42,7 @@ check:
 
 # Do a quick compile only check of the tests and impliclity the
 # library code as well.
-test-binaries: cephfs.test internal/errutil.test rados.test rbd.test
+test-binaries: cephfs.test rados.test rbd.test internal/callbacks.test internal/errutil.test internal/retry.test
 test-bins: test-binaries
 
 %.test: % force_go_build
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/entrypoint.sh b/entrypoint.sh
index d019901..7e92045 100755
--- a/entrypoint.sh
+++ b/entrypoint.sh
@@ -176,6 +176,7 @@ test_go_ceph() {
         "cephfs" \
         "internal/callbacks" \
         "internal/errutil" \
+        "internal/retry" \
         "rados" \
         "rbd" \
         )
diff --git a/internal/retry/sizer.go b/internal/retry/sizer.go
new file mode 100644
index 0000000..5b27e51
--- /dev/null
+++ b/internal/retry/sizer.go
@@ -0,0 +1,64 @@
+package retry
+
+// Hint is a type for retry hints
+type Hint interface {
+	If(bool) Hint
+	size() int
+}
+
+type hintInt int
+
+func (hint hintInt) size() int {
+	return int(hint)
+}
+
+// If is a convenience function, that returns a given hint only if a certain
+// condition is met (for example a test for a "buffer too small" error).
+// Otherwise it returns a nil which stops the retries.
+func (hint hintInt) If(cond bool) Hint {
+	if cond {
+		return hint
+	}
+	return nil
+}
+
+// DoubleSize is a hint to retry with double the size
+const DoubleSize = hintInt(0)
+
+// Size returns a hint for a specific size
+func Size(s int) Hint {
+	return hintInt(s)
+}
+
+// SizeFunc is used to implement 'resize loops' that hides the complexity of the
+// sizing away from most of the application. It's a function that takes a size
+// argument and returns nil, if no retry is necessary, or a hint indicating the
+// size for the next retry. If errors or other results are required from the
+// function, the function can write them to function closures of the surrounding
+// scope. See tests for examples.
+type SizeFunc func(size int) (hint Hint)
+
+// WithSizes repeatingly calls a SizeFunc with increasing sizes until either it
+// returns nil, or the max size has been reached. If the returned hint is
+// DoubleSize or indicating a size not greater than the current size, the size
+// is doubled. If the hint or next size is greater than the max size, the max
+// size is used for a last retry.
+func WithSizes(size int, max int, f SizeFunc) {
+	if size > max {
+		return
+	}
+	for {
+		hint := f(size)
+		if hint == nil || size == max {
+			break
+		}
+		if hint.size() > size {
+			size = hint.size()
+		} else {
+			size *= 2
+		}
+		if size > max {
+			size = max
+		}
+	}
+}
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)
-	}
-
-	// 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)
+	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
 	}
-
-	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
-	}
-
-	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)
+	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)
 
-	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_mimic.go b/rbd/rbd_mimic.go
index 523e806..d25ac74 100644
--- a/rbd/rbd_mimic.go
+++ b/rbd/rbd_mimic.go
@@ -15,29 +15,34 @@ import (
 	"bytes"
 	"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) (names []string, err error) {
-	buf := make([]byte, 4096)
-	for {
-		size := C.size_t(len(buf))
+	var (
+		buf   []byte
+		csize C.size_t
+	)
+	// from 4KiB to 32KiB
+	retry.WithSizes(4096, 1<<15, func(size int) retry.Hint {
+		csize = C.size_t(size)
+		buf = make([]byte, csize)
 		ret := C.rbd_list(cephIoctx(ioctx),
-			(*C.char)(unsafe.Pointer(&buf[0])), &size)
-		if ret == -C.ERANGE {
-			buf = make([]byte, size)
-			continue
-		} else if ret < 0 {
-			return nil, RBDError(ret)
-		}
-		tmp := bytes.Split(buf[:size-1], []byte{0})
-		for _, s := range tmp {
-			if len(s) > 0 {
-				name := C.GoString((*C.char)(unsafe.Pointer(&s[0])))
-				names = append(names, name)
-			}
+			(*C.char)(unsafe.Pointer(&buf[0])), &csize)
+		err = getErrorIfNegative(ret)
+		return retry.Size(int(csize)).If(err == errRange)
+	})
+	if err != nil {
+		return nil, err
+	}
+	tmp := bytes.Split(buf[:csize-1], []byte{0})
+	for _, s := range tmp {
+		if len(s) > 0 {
+			name := C.GoString((*C.char)(unsafe.Pointer(&s[0])))
+			names = append(names, name)
 		}
-		return names, nil
 	}
+	return names, 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
diff --git a/rbd/snapshot_nautilus.go b/rbd/snapshot_nautilus.go
index e46390d..6018463 100644
--- a/rbd/snapshot_nautilus.go
+++ b/rbd/snapshot_nautilus.go
@@ -11,8 +11,9 @@ package rbd
 import "C"
 
 import (
-	"fmt"
 	"unsafe"
+
+	"github.com/ceph/go-ceph/internal/retry"
 )
 
 // GetParentInfo looks for the parent of the image and stores the pool, name
@@ -76,27 +77,28 @@ func (image *Image) ListChildren() (pools []string, images []string, err error)
 		return nil, nil, err
 	}
 
-	size := C.size_t(0)
-	ret := C.rbd_list_children3(image.image, nil, &size)
-	if ret < 0 && ret != -C.ERANGE {
-		return nil, nil, RBDError(ret)
-	} else if ret > 0 {
-		return nil, nil, fmt.Errorf("rbd_list_children3() returned %d, expected 0", ret)
-	} else if ret == 0 && size == 0 {
-		return nil, nil, nil
-	}
-
-	// expected: ret == -ERANGE, size contains number of image names
-	children := make([]C.rbd_linked_image_spec_t, size)
-	ret = C.rbd_list_children3(image.image, (*C.rbd_linked_image_spec_t)(unsafe.Pointer(&children[0])), &size)
-	if ret < 0 {
-		return nil, nil, RBDError(ret)
+	var (
+		csize    C.size_t
+		children []C.rbd_linked_image_spec_t
+	)
+	retry.WithSizes(16, 4096, func(size int) retry.Hint {
+		csize = C.size_t(size)
+		children = make([]C.rbd_linked_image_spec_t, csize)
+		ret := C.rbd_list_children3(
+			image.image,
+			(*C.rbd_linked_image_spec_t)(unsafe.Pointer(&children[0])),
+			&csize)
+		err = getErrorIfNegative(ret)
+		return retry.Size(int(csize)).If(err == errRange)
+	})
+	if err != nil {
+		return nil, nil, err
 	}
-	defer C.rbd_linked_image_spec_list_cleanup((*C.rbd_linked_image_spec_t)(unsafe.Pointer(&children[0])), size)
+	defer C.rbd_linked_image_spec_list_cleanup((*C.rbd_linked_image_spec_t)(unsafe.Pointer(&children[0])), csize)
 
-	pools = make([]string, size)
-	images = make([]string, size)
-	for i, child := range children {
+	pools = make([]string, csize)
+	images = make([]string, csize)
+	for i, child := range children[:csize] {
 		pools[i] = C.GoString(child.pool_name)
 		images[i] = C.GoString(child.image_name)
 	}
diff --git a/rbd/watchers_mimic.go b/rbd/watchers_mimic.go
index 6f6bfbd..198292a 100644
--- a/rbd/watchers_mimic.go
+++ b/rbd/watchers_mimic.go
@@ -9,6 +9,10 @@ package rbd
 // #include <rbd/librbd.h>
 import "C"
 
+import (
+	"github.com/ceph/go-ceph/internal/retry"
+)
+
 // ImageWatcher is a representation of the rbd_image_watcher_t from librbd.h
 type ImageWatcher struct {
 	Addr   string
@@ -30,28 +34,28 @@ func (image *Image) ListWatchers() ([]ImageWatcher, error) {
 		return nil, err
 	}
 
-	count := C.ulong(0)
-	ret := C.rbd_watchers_list(image.image, nil, &count)
-	if ret != 0 && ret != -C.ERANGE {
-		return nil, getError(ret)
-	}
-	if ret == 0 && count == 0 {
-		return nil, nil
-	}
-
-	watchers := make([]C.rbd_image_watcher_t, count)
-	ret = C.rbd_watchers_list(image.image, &watchers[0], &count)
-	if ret != 0 && ret != -C.ERANGE {
-		return nil, getError(ret)
+	var (
+		err      error
+		count    C.size_t
+		watchers []C.rbd_image_watcher_t
+	)
+	retry.WithSizes(16, 4096, func(size int) retry.Hint {
+		count = C.size_t(size)
+		watchers = make([]C.rbd_image_watcher_t, count)
+		ret := C.rbd_watchers_list(image.image, &watchers[0], &count)
+		err = getErrorIfNegative(ret)
+		return retry.Size(int(count)).If(err == errRange)
+	})
+	if err != nil {
+		return nil, err
 	}
 	defer C.rbd_watchers_list_cleanup(&watchers[0], count)
 
-	imageWatchers := make([]ImageWatcher, len(watchers))
-	for i, watcher := range watchers {
+	imageWatchers := make([]ImageWatcher, count)
+	for i, watcher := range watchers[:count] {
 		imageWatchers[i].Addr = C.GoString(watcher.addr)
 		imageWatchers[i].Id = int64(watcher.id)
 		imageWatchers[i].Cookie = uint64(watcher.cookie)
 	}
-
 	return imageWatchers, nil
 }


Test patch:
diff --git a/internal/retry/example_sizer_test.go b/internal/retry/example_sizer_test.go
new file mode 100644
index 0000000..6322a66
--- /dev/null
+++ b/internal/retry/example_sizer_test.go
@@ -0,0 +1,56 @@
+package retry
+
+import (
+	"fmt"
+)
+
+var errTooSmall = fmt.Errorf("too small")
+
+func fakeComplexOp(v []string) error {
+	if len(v) < 30 {
+		fmt.Println("too small:", len(v))
+		return errTooSmall
+	}
+	fmt.Println("good size:", len(v))
+	return nil
+}
+
+func ExampleWithSizes() {
+	var err error
+	WithSizes(1, 128, func(size int) Hint {
+		buf := make([]string, size)
+		// do something complex with buf
+		err = fakeComplexOp(buf)
+		return DoubleSize.If(err == errTooSmall)
+	})
+	// Output:
+	// too small: 1
+	// too small: 2
+	// too small: 4
+	// too small: 8
+	// too small: 16
+	// good size: 32
+}
+
+func fakeComplexOp2(v []string, s *int) error {
+	if len(v) < 30 {
+		fmt.Println("too small:", len(v))
+		*s = 30
+		return errTooSmall
+	}
+	fmt.Println("good size:", len(v))
+	return nil
+}
+
+func ExampleWithSizes_hint() {
+	var err error
+	WithSizes(1, 128, func(size int) Hint {
+		buf := make([]string, size)
+		// do something complex with buf
+		err = fakeComplexOp2(buf, &size)
+		return Size(size).If(err == errTooSmall)
+	})
+	// Output:
+	// too small: 1
+	// good size: 30
+}
diff --git a/internal/retry/sizer_test.go b/internal/retry/sizer_test.go
new file mode 100644
index 0000000..ec16fc4
--- /dev/null
+++ b/internal/retry/sizer_test.go
@@ -0,0 +1,105 @@
+package retry
+
+import (
+	"errors"
+	"fmt"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestSizer(t *testing.T) {
+	tooLong := errors.New("too long")
+
+	src := [][]byte{
+		[]byte("foobarbaz"),
+		[]byte("gondwandaland"),
+		[]byte("longer and longer still, not quite done"),
+		[]byte("Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua."),
+	}
+
+	// mimic a complex-ish data copy call
+	bcopy := func(src []byte, size int) ([]byte, error) {
+		if size < len(src) {
+			return nil, tooLong
+		}
+		dst := make([]byte, size)
+		copy(dst, src)
+		return dst, nil
+	}
+
+	for i, b := range src {
+		t.Run(fmt.Sprintf("bcopy_%d", i), func(t *testing.T) {
+			var out []byte
+			var err error
+			WithSizes(1, 4096, func(size int) Hint {
+				out, err = bcopy(b, size)
+				return DoubleSize.If(err == tooLong)
+			})
+			assert.Nil(t, err)
+			assert.Equal(t, b, out[:len(b)])
+		})
+	}
+
+	for i, b := range src {
+		t.Run(fmt.Sprintf("bcopy_hint_%d", i), func(t *testing.T) {
+			var tries int
+			var err error
+			var out []byte
+			WithSizes(1, 4096, func(size int) Hint {
+				tries++
+				out, err = bcopy(b, size)
+				return Size(len(b)).If(err == tooLong)
+			})
+			assert.Nil(t, err)
+			assert.Equal(t, b, out)
+			assert.Equal(t, 2, tries)
+		})
+	}
+
+	t.Run("exceedsMax", func(t *testing.T) {
+		var tries int
+		var err error
+		WithSizes(1, 1024, func(size int) Hint {
+			tries++
+			err = errors.New("foo")
+			return DoubleSize
+		})
+		assert.Error(t, err)
+		assert.Equal(t, 11, tries)
+	})
+
+	t.Run("hintExceedsMax", func(t *testing.T) {
+		var tries int
+		var lastSize int
+		WithSizes(1, 1024, func(size int) Hint {
+			tries++
+			lastSize = size
+			return Size(1025)
+		})
+		assert.Equal(t, 1024, lastSize)
+		assert.Equal(t, 2, tries)
+	})
+
+	t.Run("weirdSizeAndMax", func(t *testing.T) {
+		var tries int
+		var lastSize int
+		WithSizes(3, 1022, func(size int) Hint {
+			tries++
+			lastSize = size
+			return DoubleSize
+		})
+		assert.Equal(t, 10, tries)
+		assert.Equal(t, 1022, lastSize)
+	})
+
+	t.Run("sizeExceedsMax", func(t *testing.T) {
+		var lastSize int
+		WithSizes(1023, 1022, func(size int) Hint {
+			lastSize = size
+			return DoubleSize
+		})
+		assert.Equal(t, 0, lastSize)
+	})
+
+}