[PATCH] Ceph RADOS cluster mutex helper for Samba CTDB

[Amitay Isaacs]

On Fri, Dec 9, 2016 at 5:39 AM, David Disseldorp <ddiss at suse.de> wrote:

> Hi Amitay,
>
> On Wed, 7 Dec 2016 13:32:34 +1100, Amitay Isaacs wrote:
>
> > Hi David,
> >
> > On Tue, Dec 6, 2016 at 11:18 PM, David Disseldorp <ddiss at suse.de> wrote:
> >
> > > This time with the patch-set attached...
> > >
> > > >  ctdb/doc/Makefile                             |   3 +-
> > > >  ctdb/doc/ctdb_mutex_ceph_rados_helper.7.xml   |  90 +++++
> > > >  .../utils/ceph/ctdb_mutex_ceph_rados_helper.c | 334
> ++++++++++++++++++
> > > >  ctdb/utils/ceph/test_ceph_rados_reclock.sh    | 151 ++++++++
> > > >  ctdb/wscript                                  |  19 +
> > > >  5 files changed, 596 insertions(+), 1 deletion(-)
> > >
> >
> > In patch 1, why do you need to include any of the CTDB files
> > (protocol/protocol.h and common/system.h) and have dependency on
> > ctdb-system?  I don't see you are using any of the functions defined in
> > common/system.h.
> >
> > Please include the manpage in SAMBA_BINARY() definition. Also include it
> in
> > manpages[] list.  It might be better to merge patch 1 and patch 2.
>
> Thanks for the feedback. Please find a new version attached (atop the
> etcd changes), attempting to address your points above:
> - drop unnecessary includes and ctdb-system dependency
>   + add separate talloc and tevent deps
>   + use tevent_timeval_current_ofs() instead of timeval_current_ofs()
> - conditionally generate the man page
>
> Cheers, David


Looks good.  Pushed to autobuild with following minor fixups.

- In manpage, replace ctdb_mutex_ceph_rados_helper_lock with
ctdb_mutex_ceph_rados_helper
- In wscript, add missing manpages_ceph in the dist target

Amitay.