Repository: ceph/go-ceph
Pull request #177 (2020-02-05 17:31:21)

rbd: create DiffIterate wrapper function
Add a DiffIterate wrapper for rbd_diff_iterate2.

First patch adds a helper type that deals with some of the tricky aspects of callbacks between C and Go. See the commit description and code comments for references about why this is needed and the implementation.

The 2nd patch actually implements the callback wrappers needed to make the call and the wrapper around the librbd function itself. We only wrap rbd_diff_iterate2 as, unless I'm wrong, it fully supersedes rbd_diff_iterate when the parameters are defaulted.

I opted to put all of the arguments to rbd_diff_iterate2 in the DiffIterateConfig as I find more then 4-5 arguments to a function call in Go start to get confusing. Because of this choice I had the ability to pass the whole configuration value to the callback. I did so, but this is not strictly required and we can discuss if this is a good idea or not. I like it because avoids having to make the caller do the work, via closures or what not, at the cost of only having to de-reference config.Data to get at the extra data value if you don't care about the rest of the config.

As far as I can tell we had no other callback based methods wrapped so this will set the pattern for future callback methods in the future. Please consider that when reviewing.

I'm happy to say the new files have 100% coverage in the tests.

Closes: #53 

## 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: New revision:
* moves the callbacks type to its own "internal/callbacks" package
  * using internal as to not expose this as a public api beyond our rados, rbd, cephfs, etc packages
* callback uses the data parameter, more similar to the C api, not the entire config type
nixpanic: Just waiting a few more days in case there is feedback through #53. If there is none by the end of this week, this can be merged.

Patch:
diff --git a/entrypoint.sh b/entrypoint.sh
index 6d18e90..b8ff44b 100755
--- a/entrypoint.sh
+++ b/entrypoint.sh
@@ -148,6 +148,7 @@ test_go_ceph() {
     pkgs=(\
         "cephfs" \
         "errutil" \
+        "internal/callbacks" \
         "rados" \
         "rbd" \
         )
diff --git a/internal/callbacks/callbacks.go b/internal/callbacks/callbacks.go
new file mode 100644
index 0000000..2bce42a
--- /dev/null
+++ b/internal/callbacks/callbacks.go
@@ -0,0 +1,64 @@
+package callbacks
+
+import (
+	"sync"
+)
+
+// The logic of this file is largely adapted from:
+// https://github.com/golang/go/wiki/cgo#function-variables
+//
+// Also helpful:
+// https://eli.thegreenplace.net/2019/passing-callbacks-and-pointers-to-cgo/
+
+// Callbacks provides a tracker for data that is to be passed between Go
+// and C callback functions. The Go callback/object may not be passed
+// by a pointer to C code and so instead integer indexes into an internal
+// map are used.
+// Typically the item being added will either be a callback function or
+// a data structure containing a callback function. It is up to the caller
+// to control and validate what "callbacks" get used.
+type Callbacks struct {
+	mutex sync.RWMutex
+	cmap  map[int]interface{}
+}
+
+// New returns a new callbacks tracker.
+func New() *Callbacks {
+	return &Callbacks{cmap: make(map[int]interface{})}
+}
+
+// Add a callback/object to the tracker and return a new index
+// for the object.
+func (cb *Callbacks) Add(v interface{}) int {
+	cb.mutex.Lock()
+	defer cb.mutex.Unlock()
+	// this approach assumes that there are typically very few callbacks
+	// in play at once and can just use the length of the map as our
+	// index. But in case of collisions we fall back to simply incrementing
+	// until we find a free key like in the cgo wiki page.
+	// If this code ever becomes a hot path there's surely plenty of room
+	// for optimization in the future :-)
+	index := len(cb.cmap) + 1
+	for {
+		if _, found := cb.cmap[index]; !found {
+			break
+		}
+		index++
+	}
+	cb.cmap[index] = v
+	return index
+}
+
+// Remove a callback/object given it's index.
+func (cb *Callbacks) Remove(index int) {
+	cb.mutex.Lock()
+	defer cb.mutex.Unlock()
+	delete(cb.cmap, index)
+}
+
+// Lookup returns a mapped callback/object given an index.
+func (cb *Callbacks) Lookup(index int) interface{} {
+	cb.mutex.RLock()
+	defer cb.mutex.RUnlock()
+	return cb.cmap[index]
+}
diff --git a/rbd/callback_shims.go b/rbd/callback_shims.go
new file mode 100644
index 0000000..ab040e7
--- /dev/null
+++ b/rbd/callback_shims.go
@@ -0,0 +1,13 @@
+package rbd
+
+/*
+
+#include <rbd/librbd.h>
+
+extern int diffIterateCallback(uint64_t ofs, size_t len, int exists, int index);
+
+int callDiffIterateCallback(uint64_t ofs, size_t len, int exists, int index) {
+	return diffIterateCallback(ofs, len, exists, index);
+}
+*/
+import "C"
diff --git a/rbd/diff_iterate.go b/rbd/diff_iterate.go
new file mode 100644
index 0000000..5e53317
--- /dev/null
+++ b/rbd/diff_iterate.go
@@ -0,0 +1,136 @@
+package rbd
+
+/*
+#cgo LDFLAGS: -lrbd
+#undef _GNU_SOURCE
+#include <errno.h>
+#include <stdlib.h>
+#include <rbd/librbd.h>
+
+extern int callDiffIterateCallback(uint64_t ofs, size_t len, int exists, int index);
+
+// cgo is having trouble converting the callback from the librbd header
+// to a unsafe.Pointer. This shim exists solely to help it along.
+static inline int wrap_rbd_diff_iterate2(
+			rbd_image_t image,
+			const char *fromsnapname,
+			uint64_t ofs, uint64_t len,
+			uint8_t include_parent, uint8_t whole_object,
+			void *cb,
+			void *arg) {
+	return rbd_diff_iterate2(image, fromsnapname, ofs, len, include_parent, whole_object, cb, arg);
+}
+*/
+import "C"
+
+import (
+	"unsafe"
+
+	"github.com/ceph/go-ceph/internal/callbacks"
+)
+
+var diffIterateCallbacks = callbacks.New()
+
+// DiffIncludeParent values control if the difference should include the parent
+// image.
+type DiffIncludeParent uint8
+
+// DiffWholeObject values control if the diff extents should cover the whole
+// object.
+type DiffWholeObject uint8
+
+// DiffIterateCallback defines the function signature needed for the
+// DiffIterate callback.
+//
+// The function will be called with the arguments: offset, length, exists, and
+// data. The offset and length correspond to the changed region of the image.
+// The exists value is set to zero if the region is known to be zeros,
+// otherwise it is set to 1. The data value is the extra data parameter that
+// was set on the DiffIterateConfig and is meant to be used for passing
+// arbitrary user-defined items to the callback function.
+//
+// The callback can trigger the iteration to terminate early by returning
+// a non-zero error code.
+type DiffIterateCallback func(uint64, uint64, int, interface{}) int
+
+// DiffIterateConfig is used to define the parameters of a DiffIterate call.
+// Callback, Offset, and Length should always be specified when passed to
+// DiffIterate. The other values are optional.
+type DiffIterateConfig struct {
+	SnapName      string
+	Offset        uint64
+	Length        uint64
+	IncludeParent DiffIncludeParent
+	WholeObject   DiffWholeObject
+	Callback      DiffIterateCallback
+	Data          interface{}
+}
+
+const (
+	// ExcludeParent will exclude the parent from the diff.
+	ExcludeParent = DiffIncludeParent(0)
+	// IncludeParent will include the parent in the diff.
+	IncludeParent = DiffIncludeParent(1)
+
+	// DisableWholeObject will not use the whole object in the diff.
+	DisableWholeObject = DiffWholeObject(0)
+	// EnableWholeObject will use the whole object in the diff.
+	EnableWholeObject = DiffWholeObject(1)
+)
+
+// DiffIterate calls a callback on changed extents of an image.
+//
+// Calling DiffIterate will cause the callback specified in the
+// DiffIterateConfig to be called as many times as there are changed
+// regions in the image (controlled by the parameters as passed to librbd).
+//
+// See the documentation of DiffIterateCallback for a description of the
+// arguments to the callback and the return behavior.
+//
+// Implements:
+//  int rbd_diff_iterate2(rbd_image_t image,
+//                        const char *fromsnapname,
+//                        uint64_t ofs, uint64_t len,
+//                        uint8_t include_parent, uint8_t whole_object,
+//                        int (*cb)(uint64_t, size_t, int, void *),
+//                        void *arg);
+func (image *Image) DiffIterate(config DiffIterateConfig) error {
+
+	if err := image.validate(imageIsOpen); err != nil {
+		return err
+	}
+	if config.Callback == nil {
+		return RBDError(C.EINVAL)
+	}
+
+	var cSnapName *C.char
+	if config.SnapName != NoSnapshot {
+		cSnapName = C.CString(config.SnapName)
+		defer C.free(unsafe.Pointer(cSnapName))
+	}
+
+	cbIndex := diffIterateCallbacks.Add(config)
+	defer diffIterateCallbacks.Remove(cbIndex)
+
+	ret := C.wrap_rbd_diff_iterate2(
+		image.image,
+		cSnapName,
+		C.uint64_t(config.Offset),
+		C.uint64_t(config.Length),
+		C.uint8_t(config.IncludeParent),
+		C.uint8_t(config.WholeObject),
+		C.callDiffIterateCallback,
+		unsafe.Pointer(uintptr(cbIndex)))
+
+	return getError(ret)
+}
+
+//export diffIterateCallback
+func diffIterateCallback(
+	offset C.uint64_t, length C.size_t, exists, index C.int) C.int {
+
+	v := diffIterateCallbacks.Lookup(int(index))
+	config := v.(DiffIterateConfig)
+	return C.int(config.Callback(
+		uint64(offset), uint64(length), int(exists), config.Data))
+}


Test patch:
diff --git a/internal/callbacks/callbacks_test.go b/internal/callbacks/callbacks_test.go
new file mode 100644
index 0000000..1adffa8
--- /dev/null
+++ b/internal/callbacks/callbacks_test.go
@@ -0,0 +1,110 @@
+package callbacks
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestCallbacks(t *testing.T) {
+	cbks := New()
+	assert.Len(t, cbks.cmap, 0)
+
+	i1 := cbks.Add("foo")
+	i2 := cbks.Add("bar")
+	i3 := cbks.Add("baz")
+	assert.Len(t, cbks.cmap, 3)
+
+	var x interface{}
+	x = cbks.Lookup(i1)
+	assert.NotNil(t, x)
+	if s, ok := x.(string); ok {
+		assert.EqualValues(t, s, "foo")
+	}
+
+	x = cbks.Lookup(5555)
+	assert.Nil(t, x)
+
+	x = cbks.Lookup(i3)
+	assert.NotNil(t, x)
+	if s, ok := x.(string); ok {
+		assert.EqualValues(t, s, "baz")
+	}
+	cbks.Remove(i3)
+	x = cbks.Lookup(i3)
+	assert.Nil(t, x)
+
+	cbks.Remove(i2)
+	x = cbks.Lookup(i2)
+	assert.Nil(t, x)
+
+	cbks.Remove(i1)
+	assert.Len(t, cbks.cmap, 0)
+}
+
+func TestCallbacksIndexing(t *testing.T) {
+	cbks := New()
+	assert.Len(t, cbks.cmap, 0)
+
+	i1 := cbks.Add("foo")
+	i2 := cbks.Add("bar")
+	_ = cbks.Add("baz")
+	_ = cbks.Add("wibble")
+	_ = cbks.Add("wabble")
+	assert.Len(t, cbks.cmap, 5)
+
+	// generally we assume that the callback data will be mostly LIFO
+	// but can't guarantee it. Thus we check that when we remove the
+	// first items inserted into the map there are no subsequent issues
+	cbks.Remove(i1)
+	cbks.Remove(i2)
+	_ = cbks.Add("flim")
+	ilast := cbks.Add("flam")
+	assert.Len(t, cbks.cmap, 5)
+
+	x := cbks.Lookup(ilast)
+	assert.NotNil(t, x)
+	if s, ok := x.(string); ok {
+		assert.EqualValues(t, s, "flam")
+	}
+}
+
+func TestCallbacksData(t *testing.T) {
+	cbks := New()
+	assert.Len(t, cbks.cmap, 0)
+
+	// insert a plain function
+	i1 := cbks.Add(func(v int) int { return v + 1 })
+
+	// insert a type "containing" a function, note that it doesn't
+	// actually have a callable function. Users of the type must
+	// check that themselves
+	type flup struct {
+		Stuff int
+		Junk  func(int, int) error
+	}
+	i2 := cbks.Add(flup{
+		Stuff: 55,
+	})
+
+	// did we get a function back
+	x1 := cbks.Lookup(i1)
+	if assert.NotNil(t, x1) {
+		if f, ok := x1.(func(v int) int); ok {
+			assert.Equal(t, 2, f(1))
+		} else {
+			t.Fatalf("conversion failed")
+		}
+	}
+
+	// did we get our data structure back
+	x2 := cbks.Lookup(i2)
+	if assert.NotNil(t, x2) {
+		if d, ok := x2.(flup); ok {
+			assert.Equal(t, 55, d.Stuff)
+			assert.Nil(t, d.Junk)
+		} else {
+			t.Fatalf("conversion failed")
+		}
+	}
+}
diff --git a/rbd/diff_iterate_test.go b/rbd/diff_iterate_test.go
new file mode 100644
index 0000000..ff1d1b1
--- /dev/null
+++ b/rbd/diff_iterate_test.go
@@ -0,0 +1,522 @@
+package rbd
+
+import (
+	"sync"
+	"testing"
+	"time"
+
+	"github.com/ceph/go-ceph/rados"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestDiffIterate(t *testing.T) {
+	conn := radosConnect(t)
+	defer conn.Shutdown()
+
+	poolname := GetUUID()
+	err := conn.MakePool(poolname)
+	assert.NoError(t, err)
+	defer conn.DeletePool(poolname)
+
+	ioctx, err := conn.OpenIOContext(poolname)
+	require.NoError(t, err)
+	defer ioctx.Destroy()
+
+	t.Run("basic", func(t *testing.T) {
+		testDiffIterateBasic(t, ioctx)
+	})
+	t.Run("twoAtOnce", func(t *testing.T) {
+		testDiffIterateTwoAtOnce(t, ioctx)
+	})
+	t.Run("earlyExit", func(t *testing.T) {
+		testDiffIterateEarlyExit(t, ioctx)
+	})
+	t.Run("snapshot", func(t *testing.T) {
+		testDiffIterateSnapshot(t, ioctx)
+	})
+	t.Run("callbackData", func(t *testing.T) {
+		testDiffIterateCallbackData(t, ioctx)
+	})
+	t.Run("badImage", func(t *testing.T) {
+		var gotCalled int
+		img := GetImage(ioctx, "bob")
+		err := img.DiffIterate(
+			DiffIterateConfig{
+				Offset: 0,
+				Length: uint64(1 << 22),
+				Callback: func(o, l uint64, e int, x interface{}) int {
+					gotCalled++
+					return 0
+				},
+			})
+		assert.Error(t, err)
+		assert.EqualValues(t, 0, gotCalled)
+	})
+	t.Run("missingCallback", func(t *testing.T) {
+		name := GetUUID()
+		isize := uint64(1 << 23) // 8MiB
+		iorder := 20             // 1MiB
+		options := NewRbdImageOptions()
+		defer options.Destroy()
+		assert.NoError(t,
+			options.SetUint64(RbdImageOptionOrder, uint64(iorder)))
+		err := CreateImage(ioctx, name, isize, options)
+		assert.NoError(t, err)
+
+		img, err := OpenImage(ioctx, name, NoSnapshot)
+		assert.NoError(t, err)
+		defer func() {
+			assert.NoError(t, img.Close())
+			assert.NoError(t, img.Remove())
+		}()
+
+		var gotCalled int
+		err = img.DiffIterate(
+			DiffIterateConfig{
+				Offset: 0,
+				Length: uint64(1 << 22),
+			})
+		assert.Error(t, err)
+		assert.EqualValues(t, 0, gotCalled)
+	})
+}
+
+func testDiffIterateBasic(t *testing.T, ioctx *rados.IOContext) {
+	name := GetUUID()
+	isize := uint64(1 << 23) // 8MiB
+	iorder := 20             // 1MiB
+	options := NewRbdImageOptions()
+	defer options.Destroy()
+	assert.NoError(t,
+		options.SetUint64(RbdImageOptionOrder, uint64(iorder)))
+	err := CreateImage(ioctx, name, isize, options)
+	assert.NoError(t, err)
+
+	img, err := OpenImage(ioctx, name, NoSnapshot)
+	assert.NoError(t, err)
+	defer func() {
+		assert.NoError(t, img.Close())
+		assert.NoError(t, img.Remove())
+	}()
+
+	type diResult struct {
+		offset uint64
+		length uint64
+	}
+	calls := []diResult{}
+
+	err = img.DiffIterate(
+		DiffIterateConfig{
+			Offset: 0,
+			Length: isize,
+			Callback: func(o, l uint64, e int, x interface{}) int {
+				calls = append(calls, diResult{offset: o, length: l})
+				return 0
+			},
+		})
+	assert.NoError(t, err)
+	// Image is new, empty. Callback will not be called
+	assert.Len(t, calls, 0)
+
+	_, err = img.WriteAt([]byte("sometimes you feel like a nut"), 0)
+	assert.NoError(t, err)
+
+	err = img.DiffIterate(
+		DiffIterateConfig{
+			Offset: 0,
+			Length: isize,
+			Callback: func(o, l uint64, e int, x interface{}) int {
+				calls = append(calls, diResult{offset: o, length: l})
+				return 0
+			},
+		})
+	assert.NoError(t, err)
+	if assert.Len(t, calls, 1) {
+		assert.EqualValues(t, 0, calls[0].offset)
+		assert.EqualValues(t, 29, calls[0].length)
+	}
+
+	_, err = img.WriteAt([]byte("sometimes you don't"), 32)
+	assert.NoError(t, err)
+
+	calls = []diResult{}
+	err = img.DiffIterate(
+		DiffIterateConfig{
+			Offset: 0,
+			Length: isize,
+			Callback: func(o, l uint64, e int, x interface{}) int {
+				calls = append(calls, diResult{offset: o, length: l})
+				return 0
+			},
+		})
+	if assert.NoError(t, err) {
+		assert.Len(t, calls, 1)
+		assert.EqualValues(t, 0, calls[0].offset)
+		assert.EqualValues(t, 51, calls[0].length)
+	}
+
+	// dirty a 2nd chunk
+	newOffset := 3145728 // 3MiB
+	_, err = img.WriteAt([]byte("alright, alright, alright"), int64(newOffset))
+	assert.NoError(t, err)
+
+	calls = []diResult{}
+	err = img.DiffIterate(
+		DiffIterateConfig{
+			Offset: 0,
+			Length: isize,
+			Callback: func(o, l uint64, e int, x interface{}) int {
+				calls = append(calls, diResult{offset: o, length: l})
+				return 0
+			},
+		})
+	assert.NoError(t, err)
+	if assert.Len(t, calls, 2) {
+		assert.EqualValues(t, 0, calls[0].offset)
+		assert.EqualValues(t, 51, calls[0].length)
+		assert.EqualValues(t, newOffset, calls[1].offset)
+		assert.EqualValues(t, 25, calls[1].length)
+	}
+
+	// dirty a 3rd chunk
+	newOffset2 := 5242880 + 1024 // 5MiB + 1KiB
+	_, err = img.WriteAt([]byte("zowie!"), int64(newOffset2))
+	assert.NoError(t, err)
+
+	calls = []diResult{}
+	err = img.DiffIterate(
+		DiffIterateConfig{
+			Offset: 0,
+			Length: isize,
+			Callback: func(o, l uint64, e int, x interface{}) int {
+				calls = append(calls, diResult{offset: o, length: l})
+				return 0
+			},
+		})
+	assert.NoError(t, err)
+	if assert.Len(t, calls, 3) {
+		assert.EqualValues(t, 0, calls[0].offset)
+		assert.EqualValues(t, 51, calls[0].length)
+		assert.EqualValues(t, newOffset, calls[1].offset)
+		assert.EqualValues(t, 25, calls[1].length)
+		assert.EqualValues(t, newOffset2-1024, calls[2].offset)
+		assert.EqualValues(t, 6+1024, calls[2].length)
+	}
+}
+
+// testDiffIterateTwoAtOnce aims to verify that multiple DiffIterate
+// callbacks can be executed at the same time without error.
+func testDiffIterateTwoAtOnce(t *testing.T, ioctx *rados.IOContext) {
+	isize := uint64(1 << 23) // 8MiB
+	iorder := 20             // 1MiB
+	options := NewRbdImageOptions()
+	defer options.Destroy()
+	assert.NoError(t,
+		options.SetUint64(RbdImageOptionOrder, uint64(iorder)))
+
+	name1 := GetUUID()
+	err := CreateImage(ioctx, name1, isize, options)
+	assert.NoError(t, err)
+
+	img1, err := OpenImage(ioctx, name1, NoSnapshot)
+	assert.NoError(t, err)
+	defer func() {
+		assert.NoError(t, img1.Close())
+		assert.NoError(t, img1.Remove())
+	}()
+
+	name2 := GetUUID()
+	err = CreateImage(ioctx, name2, isize, options)
+	assert.NoError(t, err)
+
+	img2, err := OpenImage(ioctx, name2, NoSnapshot)
+	assert.NoError(t, err)
+	defer func() {
+		assert.NoError(t, img2.Close())
+		assert.NoError(t, img2.Remove())
+	}()
+
+	type diResult struct {
+		offset uint64
+		length uint64
+	}
+
+	diffTest := func(wg *sync.WaitGroup, inbuf []byte, img *Image) {
+		_, err = img.WriteAt(inbuf[0:3], 0)
+		assert.NoError(t, err)
+		_, err = img.WriteAt(inbuf[3:6], 3145728)
+		assert.NoError(t, err)
+		_, err = img.WriteAt(inbuf[6:9], 5242880)
+		assert.NoError(t, err)
+
+		calls := []diResult{}
+		err = img.DiffIterate(
+			DiffIterateConfig{
+				Offset: 0,
+				Length: isize,
+				Callback: func(o, l uint64, e int, x interface{}) int {
+					time.Sleep(8 * time.Millisecond)
+					calls = append(calls, diResult{offset: o, length: l})
+					return 0
+				},
+			})
+		assert.NoError(t, err)
+		if assert.Len(t, calls, 3) {
+			assert.EqualValues(t, 0, calls[0].offset)
+			assert.EqualValues(t, 3, calls[0].length)
+			assert.EqualValues(t, 3145728, calls[1].offset)
+			assert.EqualValues(t, 3, calls[1].length)
+			assert.EqualValues(t, 5242880, calls[2].offset)
+			assert.EqualValues(t, 3, calls[2].length)
+		}
+
+		wg.Done()
+	}
+
+	wg := &sync.WaitGroup{}
+	wg.Add(1)
+	go diffTest(wg, []byte("foobarbaz"), img1)
+	wg.Add(1)
+	go diffTest(wg, []byte("abcdefghi"), img2)
+	wg.Wait()
+}
+
+// testDiffIterateEarlyExit checks that returning an error from the callback
+// function triggers the DiffIterate call to stop.
+func testDiffIterateEarlyExit(t *testing.T, ioctx *rados.IOContext) {
+	isize := uint64(1 << 23) // 8MiB
+	iorder := 20             // 1MiB
+	options := NewRbdImageOptions()
+	defer options.Destroy()
+	assert.NoError(t,
+		options.SetUint64(RbdImageOptionOrder, uint64(iorder)))
+
+	name := GetUUID()
+	err := CreateImage(ioctx, name, isize, options)
+	assert.NoError(t, err)
+
+	img, err := OpenImage(ioctx, name, NoSnapshot)
+	assert.NoError(t, err)
+	defer func() {
+		assert.NoError(t, img.Close())
+		assert.NoError(t, img.Remove())
+	}()
+
+	type diResult struct {
+		offset uint64
+		length uint64
+	}
+
+	// "damage" the image
+	inbuf := []byte("xxxyyyzzz")
+	_, err = img.WriteAt(inbuf[0:3], 0)
+	assert.NoError(t, err)
+	_, err = img.WriteAt(inbuf[3:6], 3145728)
+	assert.NoError(t, err)
+	_, err = img.WriteAt(inbuf[6:9], 5242880)
+	assert.NoError(t, err)
+
+	// if the offset is less than zero the callback will return an "error" and
+	// that will abort the DiffIterate call early and it will return the error
+	// code our callback used.
+	calls := []diResult{}
+	err = img.DiffIterate(
+		DiffIterateConfig{
+			Offset: 0,
+			Length: isize,
+			Callback: func(o, l uint64, e int, x interface{}) int {
+				if o > 1 {
+					return -5
+				}
+				calls = append(calls, diResult{offset: o, length: l})
+				return 0
+			},
+		})
+	assert.Error(t, err)
+	if rbderr, ok := err.(RBDError); assert.True(t, ok) {
+		assert.EqualValues(t, -5, int(rbderr))
+	}
+	if assert.Len(t, calls, 1) {
+		assert.EqualValues(t, 0, calls[0].offset)
+		assert.EqualValues(t, 3, calls[0].length)
+	}
+}
+
+func testDiffIterateSnapshot(t *testing.T, ioctx *rados.IOContext) {
+	name := GetUUID()
+	isize := uint64(1 << 23) // 8MiB
+	iorder := 20             // 1MiB
+	options := NewRbdImageOptions()
+	defer options.Destroy()
+	assert.NoError(t,
+		options.SetUint64(RbdImageOptionOrder, uint64(iorder)))
+	err := CreateImage(ioctx, name, isize, options)
+	assert.NoError(t, err)
+
+	img, err := OpenImage(ioctx, name, NoSnapshot)
+	assert.NoError(t, err)
+	defer func() {
+		assert.NoError(t, img.Close())
+		assert.NoError(t, img.Remove())
+	}()
+
+	type diResult struct {
+		offset uint64
+		length uint64
+	}
+	calls := []diResult{}
+
+	err = img.DiffIterate(
+		DiffIterateConfig{
+			Offset: 0,
+			Length: isize,
+			Callback: func(o, l uint64, e int, x interface{}) int {
+				calls = append(calls, diResult{offset: o, length: l})
+				return 0
+			},
+		})
+	assert.NoError(t, err)
+	// Image is new, empty. Callback will not be called
+	assert.Len(t, calls, 0)
+
+	_, err = img.WriteAt([]byte("sometimes you feel like a nut"), 0)
+	assert.NoError(t, err)
+
+	calls = []diResult{}
+	err = img.DiffIterate(
+		DiffIterateConfig{
+			Offset: 0,
+			Length: isize,
+			Callback: func(o, l uint64, e int, x interface{}) int {
+				calls = append(calls, diResult{offset: o, length: l})
+				return 0
+			},
+		})
+	assert.NoError(t, err)
+	if assert.Len(t, calls, 1) {
+		assert.EqualValues(t, 0, calls[0].offset)
+		assert.EqualValues(t, 29, calls[0].length)
+	}
+
+	ss1, err := img.CreateSnapshot("ss1")
+	assert.NoError(t, err)
+	defer func() { assert.NoError(t, ss1.Remove()) }()
+
+	// there should be no differences between "now" and "ss1"
+	calls = []diResult{}
+	err = img.DiffIterate(
+		DiffIterateConfig{
+			SnapName: "ss1",
+			Offset:   0,
+			Length:   isize,
+			Callback: func(o, l uint64, e int, x interface{}) int {
+				calls = append(calls, diResult{offset: o, length: l})
+				return 0
+			},
+		})
+	assert.NoError(t, err)
+	assert.Len(t, calls, 0)
+
+	// this next check was shamelessly cribbed from the pybind
+	// tests for diff_iterate out of the ceph tree
+	// it discards the current image, makes a 2nd snap, and compares
+	// the diff between snapshots 1 & 2.
+	_, err = img.Discard(0, isize)
+	assert.NoError(t, err)
+
+	ss2, err := img.CreateSnapshot("ss2")
+	assert.NoError(t, err)
+	defer func() { assert.NoError(t, ss2.Remove()) }()
+	err = ss2.Set() // caution: this side-effects img!
+	assert.NoError(t, err)
+
+	calls = []diResult{}
+	err = img.DiffIterate(
+		DiffIterateConfig{
+			SnapName: "ss1",
+			Offset:   0,
+			Length:   isize,
+			Callback: func(o, l uint64, e int, x interface{}) int {
+				calls = append(calls, diResult{offset: o, length: l})
+				return 0
+			},
+		})
+	assert.NoError(t, err)
+	if assert.Len(t, calls, 1) {
+		assert.EqualValues(t, 0, calls[0].offset)
+		assert.EqualValues(t, 29, calls[0].length)
+	}
+}
+
+func testDiffIterateCallbackData(t *testing.T, ioctx *rados.IOContext) {
+	name := GetUUID()
+	isize := uint64(1 << 23) // 8MiB
+	iorder := 20             // 1MiB
+	options := NewRbdImageOptions()
+	defer options.Destroy()
+	assert.NoError(t,
+		options.SetUint64(RbdImageOptionOrder, uint64(iorder)))
+	err := CreateImage(ioctx, name, isize, options)
+	assert.NoError(t, err)
+
+	img, err := OpenImage(ioctx, name, NoSnapshot)
+	assert.NoError(t, err)
+	defer func() {
+		assert.NoError(t, img.Close())
+		assert.NoError(t, img.Remove())
+	}()
+
+	type diResult struct {
+		offset uint64
+		length uint64
+	}
+	calls := []diResult{}
+
+	_, err = img.WriteAt([]byte("sometimes you feel like a nut"), 0)
+	assert.NoError(t, err)
+
+	err = img.DiffIterate(
+		DiffIterateConfig{
+			Offset: 0,
+			Length: isize,
+			Callback: func(o, l uint64, e int, x interface{}) int {
+				if v, ok := x.(int); ok {
+					assert.EqualValues(t, 77, v)
+				} else {
+					t.Fatalf("incorrect type")
+				}
+				calls = append(calls, diResult{offset: o, length: l})
+				return 0
+			},
+			Data: 77,
+		})
+	assert.NoError(t, err)
+	if assert.Len(t, calls, 1) {
+		assert.EqualValues(t, 0, calls[0].offset)
+		assert.EqualValues(t, 29, calls[0].length)
+	}
+
+	calls = []diResult{}
+	err = img.DiffIterate(
+		DiffIterateConfig{
+			Offset: 0,
+			Length: isize,
+			Callback: func(o, l uint64, e int, x interface{}) int {
+				if v, ok := x.(string); ok {
+					assert.EqualValues(t, "bob", v)
+				} else {
+					t.Fatalf("incorrect type")
+				}
+				calls = append(calls, diResult{offset: o, length: l})
+				return 0
+			},
+			Data: "bob",
+		})
+	assert.NoError(t, err)
+	if assert.Len(t, calls, 1) {
+		assert.EqualValues(t, 0, calls[0].offset)
+		assert.EqualValues(t, 29, calls[0].length)
+	}
+}