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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list