[Gluster-devel] [PATCH v7] vfs_glusterfs: Samba VFS module for glusterfs

[Andrew Bartlett]

On Tue, 2013-05-28 at 17:36 -0400, Anand Avati wrote:
> Implement a Samba VFS plugin for glusterfs based on gluster's gfapi.
> This is a "bottom" vfs plugin (not something to be stacked on top of
> another module), and translates (most) calls into closest actions
> on gfapi.

Thank you for your patience here.

> Signed-off-by: Anand Avati <avati at redhat.com>
> ---
> 
> Hi,
> 
> I hope the waf changes are fine. I could not test them because builds were failing generally on samba.git master HEAD.

You need to resolve whatever is causing your problems here, as it is not
a general issue. Our autobuild system ensures that git master and
v4-0-test always compile and test in at least Ubuntu 10.04, and it is
incredibly rare that it not work on at least Linux without it being a
specifically local issue.  

Also (sorry for the dup mail), autoconf just went away yesterday, so you
need to drop that part from your patch.

+
+bld.SAMBA3_MODULE('vfs_glusterfs',
+                  subsystem='vfs',
+                  source=VFS_GLUSTERFS_SRC,
+                  deps='samba-util',
+                  init_function='',
+                  internal_module=bld.SAMBA3_IS_STATIC_MODULE('vfs_glusterfs'),
+                  enabled=bld.SAMBA3_IS_ENABLED_MODULE('vfs_glusterfs'))

You should ensure your module depends on glusterfs, by changing that to
adding deps='glusterfs samba-util' (and making that match what the
conf.check_cfg finds and stores). 

> diff --git a/source3/wscript b/source3/wscript
> index dba6cdc..0d07692 100644
> --- a/source3/wscript
> +++ b/source3/wscript
> @@ -59,6 +59,11 @@ def set_options(opt):
>                     help=("Directory under which libcephfs is installed"),
>                     action="store", dest='libcephfs_dir', default=None)
>  
> +    opt.add_option('--enable-glusterfs',
> +                   help=("Enable building vfs_glusterfs module"),
> +                   action="store_true", dest='enable_glusterfs', default=True)
> +    opt.add_option('--disable-glusterfs', help=SUPPRESS_HELP,
> +                   action="store_false", dest='enable_glusterfs')

In general we would prefer not to have options for every possible
module.  Instead, please just make it automatic based on finding
glusterfs. 

>  def configure(conf):
> @@ -1709,6 +1714,13 @@ main() {
>      if conf.CHECK_HEADERS('cephfs/libcephfs.h', False, False, 'cephfs') and conf.CHECK_LIB('cephfs'):
>          conf.DEFINE('HAVE_CEPH', '1')
>  
> +    conf.env.enable_glusterfs = Options.options.enable_glusterfs
> +
> +    if Options.options.enable_glusterfs:
> +        conf.check_cfg(package='glusterfs-api', args='"glusterfs-api >= 4" --cflags --libs',
> +                       uselib_store='GLUSTERFS', msg='Checking for glusterfs-api >= 4',
> +                       mandatory=False)
> +
>      conf.env.build_regedit = False
>      if not Options.options.with_regedit == False:
>          conf.PROCESS_SEPARATE_RULE('system_ncurses')
> @@ -1797,6 +1809,9 @@ main() {
>      if conf.CONFIG_SET("HAVE_CEPH"):
>          default_shared_modules.extend(TO_LIST('vfs_ceph'))
>  
> +    if conf.CONFIG_SET('HAVE_GLUSTERFS'):
> +        default_shared_modules.extend(TO_LIST('vfs_glusterfs'))
> +
>      explicit_shared_modules = TO_LIST(Options.options.shared_modules, delimiter=',')
>      explicit_static_modules = TO_LIST(Options.options.static_modules, delimiter=',')

This much looks good, assuming it is tested and works.   You might need
to make uselib_store='glusterfs' (not sure). 

Thanks,

Andrew Bartlett

-- 
Andrew Bartlett                                http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org