Repository: ceph/go-ceph
Pull request #527 (2021-07-02 15:13:09)

rbd: export mirror image list functions
This change exports the functions that fetch the list chunks. It's a pretty thin wrapper around the `rbd_mirror_image_*_list` functions. The only difference is that a max value 0 will not return an empty slice, but a slice with _all_ items.

Depends on: #523 

Signed-off-by: Sven Anderson <sven@redhat.com>

## 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: Instead of passing in a "magic" value of -1 for all, I think that having a "get all" variant of the function call would be clearer API wise and the implementation of the function "closer" to C api would remain simpler. What do you think?
ansiwen: > Instead of passing in a "magic" value of -1 for all, I think that having a "get all" variant of the function call would be clearer API wise and the implementation of the function "closer" to C api would remain simpler. What do you think?

We also don't have a "no filter" variant, or a "no start" variant, so it strikes me as inconsistent to have a "no max" variant, apart from the fact, that the number of permutations would explode, if we wanted to have that. Like the zero value of `ImageMirrorModeFilter` (`nil`) represents "no filter" and the zero value of `string`(`""`) represents "no start", a zero value of `int` (`0`) respresents "no max". Where is the magic?

dpulls[bot]: :tada: All dependencies have been resolved !
mergify[bot]: This pull request now has conflicts with the target branch. Could you please resolve conflicts and force push the corrected changes? 🙏
ansiwen: Note to myself: as discussed with @phlogistonjohn I will try to separate the "whole-slice" loop from the chunk-function, if possible without costs.
mergify[bot]: This pull request now has conflicts with the target branch. Could you please resolve conflicts and force push the corrected changes? 🙏
ansiwen: @mergifyio rebase
mergify[bot]: **Command `rebase`: success**
> **Branch already up to date**
> 

ansiwen: > Seems the same as the version I approved. Not sure what, if any, the difference is. :-)

The commit ID.
phlogistonjohn: @mergifyio rebase
mergify[bot]: **Command `rebase`: success**
> **Branch has been successfully rebased**
> 


Patch:
diff --git a/rbd/mirror.go b/rbd/mirror.go
index 163824f..e9dfc56 100644
--- a/rbd/mirror.go
+++ b/rbd/mirror.go
@@ -636,6 +636,45 @@ type GlobalMirrorImageIDAndStatus struct {
 	Status GlobalMirrorImageStatus
 }
 
+// iterBufSize is intentionally not a constant. The unit tests alter
+// this value in order to get more code coverage w/o needing to create
+// very many images.
+var iterBufSize = 64
+
+// MirrorImageGlobalStatusList returns a slice of GlobalMirrorImageIDAndStatus.
+// If the length of the returned slice equals max, the next chunk of the list
+// can be obtained by setting start to the ID of the last item of the returned
+// slice. If max is 0 a slice of all items is returned.
+//
+// Implements:
+// int rbd_mirror_image_status_list(rados_ioctx_t p,
+//     const char *start_id, size_t max, char **image_ids,
+//     rbd_mirror_image_status_t *images, size_t *len)
+func MirrorImageGlobalStatusList(
+	ioctx *rados.IOContext, start string, max int) ([]GlobalMirrorImageIDAndStatus, error) {
+	var (
+		result   []GlobalMirrorImageIDAndStatus
+		fetchAll bool
+	)
+	if max <= 0 {
+		max = iterBufSize
+		fetchAll = true
+	}
+	chunk := make([]GlobalMirrorImageIDAndStatus, max)
+	for {
+		length, err := mirrorImageGlobalStatusList(ioctx, start, chunk)
+		if err != nil {
+			return nil, err
+		}
+		result = append(result, chunk[:length]...)
+		if !fetchAll || length < max {
+			break
+		}
+		start = chunk[length-1].ID
+	}
+	return result, nil
+}
+
 func mirrorImageGlobalStatusList(
 	ioctx *rados.IOContext, start string,
 	results []GlobalMirrorImageIDAndStatus) (int, error) {
@@ -658,7 +697,9 @@ func mirrorImageGlobalStatusList(
 		&ids[0],
 		&images[0],
 		&length)
-
+	if err := getError(ret); err != nil {
+		return 0, err
+	}
 	for i := 0; i < int(length); i++ {
 		results[i].ID = C.GoString(ids[i])
 		results[i].Status = newGlobalMirrorImageStatus(&images[0])
@@ -670,11 +711,6 @@ func mirrorImageGlobalStatusList(
 	return int(length), getError(ret)
 }
 
-// iterBufSize is intentionally not a constant. The unit tests alter
-// this value in order to get more code coverage w/o needing to create
-// very many images.
-var iterBufSize = 64
-
 // MirrorImageGlobalStatusIter provide methods for iterating over all
 // the GlobalMirrorImageIdAndStatus values in a pool.
 type MirrorImageGlobalStatusIter struct {
@@ -739,6 +775,44 @@ type MirrorImageInfoItem struct {
 	Info MirrorImageInfo
 }
 
+// MirrorImageInfoList returns a slice of MirrorImageInfoItem. If the length of
+// the returned slice equals max, the next chunk of the list can be obtained by
+// setting start to the ID of the last item of the returned slice. The returned
+// items are filtered by the mirror mode specified with modeFilter. If max is 0
+// a slice of all items is returned.
+//
+// Implements:
+// int rbd_mirror_image_info_list(
+//     rados_ioctx_t p, rbd_mirror_image_mode_t *mode_filter,
+//     const char *start_id, size_t max, char **image_ids,
+//     rbd_mirror_image_mode_t *mode_entries,
+//     rbd_mirror_image_info_t *info_entries, size_t *num_entries)
+func MirrorImageInfoList(
+	ioctx *rados.IOContext, modeFilter ImageMirrorModeFilter, start string,
+	max int) ([]MirrorImageInfoItem, error) {
+	var (
+		result   []MirrorImageInfoItem
+		fetchAll bool
+	)
+	if max <= 0 {
+		max = iterBufSize
+		fetchAll = true
+	}
+	chunk := make([]MirrorImageInfoItem, max)
+	for {
+		length, err := mirrorImageInfoList(ioctx, start, modeFilter, chunk)
+		if err != nil {
+			return nil, err
+		}
+		result = append(result, chunk[:length]...)
+		if !fetchAll || length < max {
+			break
+		}
+		start = chunk[length-1].ID
+	}
+	return result, nil
+}
+
 func mirrorImageInfoList(ioctx *rados.IOContext, start string,
 	modeFilter ImageMirrorModeFilter, results []MirrorImageInfoItem) (int, error) {
 
@@ -767,7 +841,9 @@ func mirrorImageInfoList(ioctx *rados.IOContext, start string,
 		&infos[0],
 		&length,
 	)
-
+	if err := getError(ret); err != nil {
+		return 0, err
+	}
 	for i := 0; i < int(length); i++ {
 		results[i].ID = C.GoString(ids[i])
 		results[i].Mode = ImageMirrorMode(modes[i])


Test patch:
diff --git a/rbd/mirror_test.go b/rbd/mirror_test.go
index 78358b3..063b88c 100644
--- a/rbd/mirror_test.go
+++ b/rbd/mirror_test.go
@@ -879,6 +879,26 @@ func TestMirrorImageLists(t *testing.T) {
 		}
 	})
 
+	t.Run("getStatusSlice", func(t *testing.T) {
+		lst, err := MirrorImageGlobalStatusList(ioctx, "", 0)
+		assert.NoError(t, err)
+		assert.Len(t, lst, 10)
+		for i := 1; i < len(lst); i++ {
+			assert.NotEqual(t, lst[i-1].ID, lst[i].ID)
+		}
+		for i := 1; i <= iterBufSize; i++ {
+			lst, err := MirrorImageGlobalStatusList(ioctx, "", i)
+			assert.NoError(t, err)
+			assert.Len(t, lst, i)
+		}
+		lst, err = MirrorImageGlobalStatusList(ioctx, "", 3)
+		assert.NoError(t, err)
+		assert.Len(t, lst, 3)
+		lst, err = MirrorImageGlobalStatusList(ioctx, lst[2].ID, 0)
+		assert.NoError(t, err)
+		assert.Len(t, lst, 7)
+	})
+
 	t.Run("getInfo", func(t *testing.T) {
 		get := func(iter *MirrorImageInfoIter) []*MirrorImageInfoItem {
 			lst := []*MirrorImageInfoItem{}
@@ -924,4 +944,30 @@ func TestMirrorImageLists(t *testing.T) {
 		assert.Equal(t, item.Info.State, MirrorImageEnabled)
 		assert.Equal(t, item.Info.Primary, true)
 	})
+
+	t.Run("getInfoSlice", func(t *testing.T) {
+		lst, err := MirrorImageInfoList(ioctx, nil, "", 0)
+		assert.NoError(t, err)
+		assert.Len(t, lst, 10)
+		for i := 1; i < len(lst); i++ {
+			assert.NotEqual(t, lst[i-1].ID, lst[i].ID)
+		}
+		for i := 1; i <= iterBufSize; i++ {
+			lst, err := MirrorImageInfoList(ioctx, nil, "", i)
+			assert.NoError(t, err)
+			assert.Len(t, lst, i)
+		}
+		lst, err = MirrorImageInfoList(ioctx, ImageMirrorModeJournal, "", 0)
+		assert.NoError(t, err)
+		assert.Len(t, lst, 5)
+		lst, err = MirrorImageInfoList(ioctx, ImageMirrorModeSnapshot, "", 0)
+		assert.NoError(t, err)
+		assert.Len(t, lst, 5)
+		lst, err = MirrorImageInfoList(ioctx, nil, "", 3)
+		assert.NoError(t, err)
+		assert.Len(t, lst, 3)
+		lst, err = MirrorImageInfoList(ioctx, nil, lst[2].ID, 0)
+		assert.NoError(t, err)
+		assert.Len(t, lst, 7)
+	})
 }