[PATCH v4] fix ctdb_mutex_ceph_rados_helper deadlock

[David Disseldorp]

Hi Amitay,

On Mon, 6 Aug 2018 16:58:46 +1000, Amitay Isaacs wrote:

> On Fri, Aug 3, 2018 at 12:05 AM, David Disseldorp via samba-technical
> <samba-technical at lists.samba.org> wrote:
> > On Thu, 2 Aug 2018 16:34:43 +1000, Amitay Isaacs via samba-technical wrote:
> > ...  
> >> > Additional team review / push appreciated.  
> >>
> >> The first patch breaks the standalone CTDB build.  Here is a fixup
> >> patch that you can squash with your first patch.
> >>
> >> I have tested with distro based (fedora 28) ceph/rados packages.  I
> >> haven't built ceph from source.
> >>
> >> With this fixup, reviewed by me.  
> >
> > Thanks a lot for the review. Looks like there's one last change that's
> > needed to handle standalone build with --with-libcephfs=<path>, as we
> > can't rely on the source3/wscript env:
> >
> > --- a/ctdb/wscript
> > +++ b/ctdb/wscript
> > @@ -243,8 +243,12 @@ def configure(conf):
> >      if Options.options.ctdb_ceph_reclock:
> >          # Use custom libcephfs library path if provided
> >          if Options.options.libcephfs_dir:
> > -            conf.env['CPPPATH_RADOS'] = conf.env['CPPPATH_CEPHFS']
> > -            conf.env['LIBPATH_RADOS'] = conf.env['LIBPATH_CEPHFS']
> > +            conf.env['CPPPATH_RADOS'] = Options.options.libcephfs_dir + '/include'
> > +            conf.env['LIBPATH_RADOS'] = Options.options.libcephfs_dir + '/lib'
> > +            conf.env['LIBPATH_CEPH-COMMON'] = conf.env['LIBPATH_RADOS'] + '/ceph'
> > +        else:
> > +            conf.env['LIBPATH_CEPH-COMMON'] = Options.options.LIBDIR + '/ceph'  
> 
> This does not seem right. Are you sure it should be Options.options.LIBDIR?
> 
> After configuring with install ceph packages I get
> 
>    conf.env['LIBPATH_CEPH-COMMON] = '/ceph'

This mirrors the source3/wscript logic added via b2764959200 ("build:
fix libceph-common detection").
Options.options.LIBDIR only appears to be set if configure --libdir=X is
explicitly specified. Using conf.env.libdir instead would likely make
more sense, as waf sets it to a sensible default which can then be
overridden by an explicit --libdir=X parameter.

> If libcephfs_dir is not specified, then why do you want to guess the
> LIBPATH_CEPH-COMMON?

IIUC (@Günther please correct me if I'm wrong) the lack of a ceph
pkgconfig and libceph-common.so nested install path makes this
necessary.

> If you are guessing the path, then shouldn't subsequent check for
> ceph-common library use that variable?
> 
>    conf.CHECK_LIB('ceph-common', shlib=True)
> 
> The above check does not take into account LIBPATH_CEPH-COMMON
> variable at at all.

It does in my environment, waf picks it up automatically:
(from bin/config.log)
Checking for library ceph-common
['/usr/lib64/ccache/gcc', 'default/test_1.o', '-o', '/home/ddiss/isms/samba/bin/.conf_check_0/testbuild/default/libtestprog.so', '-lpthread', '-Wl,-no-undefined', '-Wl,--export-dynamic', '-shared', '-L/usr/local/lib', '-L<LIBPATH_CEPH-COMMON>', '-Wl,-Bdynamic', '-lceph-common']

Cheers, David