On 01/22/2016 08:14 AM, Wido den Hollander wrote:
On 21-01-16 21:55, John Ferlan wrote:
On 12/23/2015 04:29 AM, Wido den Hollander wrote:
RBD supports cloning by creating a snapshot, protecting it and create a child image based on that snapshot afterwards.
The RBD storage driver will try to find a snapshot with zero deltas between the current state of the original volume and the snapshot.
If such a snapshot is found a clone/child image will be created using the rbd_clone2() function from librbd.
It will use the same features, strip size and stripe count as the parent image.
This implementation will only create a single snapshot on the parent image if this never changes. That should improve performance when removing the parent image at some point.
During build the decision will be made to user rbd_diff_iterate() or rbd_diff_iterate2(). The latter is faster, but only available on Ceph versions after 0.94 (Hammer).
Cloning is only supported if RBD format 2 is used. All images created by libvirt are already format 2.
If a RBD format 1 image is used as the original volume libvirt will return VIR_ERR_OPERATION_UNSUPPORTED
Long line... and the API should return 0 or -1 (based on other backends)... Caller doesn't expect that VIR_ERR* on return...
I assume you realize that 'virsh vol-create-from' and 'vol-clone' end up using the same API...
FWIW: I reviewed bottom up - I may also have been distracted more than once looking for specific code. Hopefully this is coherent ;-)
Thanks for all the feedback, really appreciate it!
Working on a revised version of all the patches right now. Rebasing a lot, editing commits, etc, etc.
The coding style changes are new to me though. I run make syntax-check and that works. Never knew that this was needed:
static int virStorageRBDXXXXXXX
All the exisiting code does:
static int virStorageRBDXXXXXX
Tough to make a syntax-check rule since there'd be a lot of cleanup before it could be 'enforced'. This has been more of a code review type effort as new functions are created or function arguments are changed. Similarly two blank lines between functions. It's stylistic - the more code that can follow, the better/easier to read other code. BTW: Use of 'ret' instead of the 'r' from each rbd_* call (IOW: -errno values) is definitely something I've seen crop up in a few recent bz's so it's fresher on my mind.
Anyway, I'll get back with new patches. Ignore anything which is on the list right now.
Btw, a Pull Request mechanism like Github would be so much easier than sending patches per e-mail ;)
And there's a segment of folks that aren't fans of github... John