ceph
Audit ERANGE error handling and buffer resizing
There are several places where libraries can return ERANGE and a size argument. It would be good to have a common approach for checking this state and resizing buffers accordingly.
Some (if not all) places may be racy, as the return of ERANGE is done before a 2nd call to the function (with a correctly sized buffer).
After a lot of pondering I've come up with a 2nd approach that is very clear to read and avoids some of the issues I have with writing generic code (that could potentially need templating/generics): Preview here: https://github.com/phlogistonjohn/go-ceph/commit/a15c05ddb179d58078401f91d6aa6abbcf6fa6f0 I suggest looking at the examples first then looking at the implementation. This can cover the following cases that the approach in the current pr does not:
- Types other than
[]btye - Functions that return/update a size value with a desired size
- Functions that want to mess with those sizes/raw buffers outside of the (loop|callback)
This iterator-with-error-checking approach also has the advantage of potentially increasing the (by line) code coverage. This is good for those tests where it's really difficult to get ceph to spit out the error condition we might want to check for.
I like the callback approach for simplicity but it won't fit in a bunch of other use cases. Note that the callback can actually be re-implemented in terms of this loop.
Thoughts or questions? @nixpanic @ansiwen ?
@nixpanic do you think this is sufficiently covered by the previous retry work? I did a quick audit of the code and the only "weird" use of ERANGE at this is GetParentInfo in rbd/snapshot_nautilus.go. And its only that way because the code is trying to be compatible with the older function signatures that were added for pre-nautilus apis. The awkwardness of the function is due to the clumsy api that was added for those older ceph versions. Perhaps we should file a new issue to expose functions that more closely map to rbd_get_parent and rbd_list_children3? I am inclined in that direction seeing as how the rbd_linked_image_spec_t has a bunch of potentially useful fields that are just being thrown away by these calls.