Re: [Gluster-devel] [PATCH 2/2] vfs_glusterfs: Samba VFS module for glusterfs

[Volker Lendecke]

On Wed, Apr 24, 2013 at 08:28:18AM +0000, 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.
> 
> Signed-off-by: Anand Avati <address@hidden>
> ---
>  source3/Makefile.in             |    5 +
>  source3/configure.in            |    9 +
>  source3/modules/vfs_glusterfs.c | 1186 
> +++++++++++++++++++++++++++++++++++++++
>  source3/modules/wscript_build   |    9 +
>  source3/wscript                 |    6 +
>  5 files changed, 1215 insertions(+), 0 deletions(-)
>  create mode 100644 source3/modules/vfs_glusterfs.c
> 
> diff --git a/source3/Makefile.in b/source3/Makefile.in
> index a868685..f199300 100644
> --- a/source3/Makefile.in
> +++ b/source3/Makefile.in
> @@ -913,6 +913,7 @@ VFS_LINUX_XFS_SGID_OBJ = modules/vfs_linux_xfs_sgid.o
>  VFS_TIME_AUDIT_OBJ = modules/vfs_time_audit.o
>  VFS_MEDIA_HARMONY_OBJ = modules/vfs_media_harmony.o
>  VFS_BTRFS_OBJ = modules/vfs_btrfs.o
> +VFS_GLUSTERFS_OBJ = modules/vfs_glusterfs.o
>  
>  PAM_ERRORS_OBJ = ../libcli/auth/pam_errors.o
>  PLAINTEXT_AUTH_OBJ = auth/pampass.o auth/pass_check.o $(PAM_ERRORS_OBJ)
> @@ -2965,6 +2966,10 @@ bin/address@hidden@: $(BINARY_PREREQS) $(VFS_BTRFS_OBJ)
>       @echo "Building plugin $@"
>       @$(SHLD_MODULE) $(VFS_BTRFS_OBJ)
>  
> +bin/address@hidden@: $(BINARY_PREREQS) $(VFS_GLUSTERFS_OBJ)
> +     @echo "Building plugin $@"
> +     $(SHLD_MODULE) $(VFS_GLUSTERFS_OBJ)
> +
>  #########################################################
>  ## IdMap NSS plugins
>  
> diff --git a/source3/configure.in b/source3/configure.in
> index 88c71d4..101db89 100644
> --- a/source3/configure.in
> +++ b/source3/configure.in
> @@ -1023,6 +1023,14 @@ if test x"$ac_cv_header_gpfs_gpl_h" = x"yes"; then
>      default_shared_modules="$default_shared_modules vfs_gpfs"
>  fi
>  
> +PKG_CHECK_MODULES([GLFS], [glusterfs-api >= 3], glfs_found=yes, 
> glfs_found=no)
> +if test "x$glfs_found" = "xyes"; then
> +    CFLAGS="$CFLAGS $GLFS_CFLAGS"
> +    LDFLAGS="$LDFLAGS $GLFS_LDFLAGS"
> +    LIBS="$LIBS $GLFS_LIBS"
> +    default_shared_modules="$default_shared_modules vfs_glusterfs"
> +fi
> +
>  # Note that all the libunwind symbols in the API are defined to internal
>  # platform-specific version, so we must include libunwind.h before checking
>  # any of them.
> @@ -6455,6 +6463,7 @@ SMB_MODULE(vfs_linux_xfs_sgid, 
> \$(VFS_LINUX_XFS_SGID_OBJ), "bin/linux_xfs_sgid.$
>  SMB_MODULE(vfs_time_audit, \$(VFS_TIME_AUDIT_OBJ), 
> "bin/time_audit.$SHLIBEXT", VFS)
>  SMB_MODULE(vfs_media_harmony, \$(VFS_MEDIA_HARMONY_OBJ), 
> "bin/media_harmony.$SHLIBEXT", VFS)
>  SMB_MODULE(vfs_btrfs, \$(VFS_BTRFS_OBJ), "bin/btrfs.$SHLIBEXT", VFS)
> +SMB_MODULE(vfs_glusterfs, \$(VFS_GLUSTERFS_OBJ), "bin/glusterfs.$SHLIBEXT", 
> VFS)
>  
>  SMB_SUBSYSTEM(VFS,smbd/vfs.o)
>  
> diff --git a/source3/modules/vfs_glusterfs.c b/source3/modules/vfs_glusterfs.c
> new file mode 100644
> index 0000000..1dc9c87
> --- /dev/null
> +++ b/source3/modules/vfs_glusterfs.c
> @@ -0,0 +1,1186 @@
> +/*
> +   Unix SMB/CIFS implementation.
> +
> +   Wrap GlusterFS GFAPI calls in vfs functions.
> +
> +   Copyright (c) 2013 Anand Avati <address@hidden>
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +*/
> +
> +
> +#include "includes.h"
> +#include "smbd/smbd.h"
> +#include <stdio.h>
> +#include <sys/acl.h>
> +#include "api/glfs.h"
> +#include "modules/vfs_posixacl.h"
> +
> +#define DEFAULT_VOLFILE_SERVER "localhost"
> +
> +/*
> +  TODO
> +  ----
> +  Short term:
> +  - AIO support
> +  - sendfile/recvfile support
> +  - vfs_gluster_sys_acl_get_file dynamic xattr size
> +  - single glfs_t per volume
> +
> +  Long term:
> +  - Case insensitivity
> +*/
> +
> +/* Helpers to provide 'integer' fds */
> +
> +/* This is global. gfapi's FD operations do not
> +   require filesystem context.
> +*/
> +static glfs_fd_t **glfd_fd;
> +static glfd_fd_size;
> +static glfd_fd_used;
> +static int
> +glfd_fd_store (glfs_fd_t *glfd)
> +{
> +     int i;
> +     void *tmp;
> +
> +     if (glfd_fd_size == glfd_fd_used) {
> +             tmp = realloc (glfd_fd, glfd_fd_size + 1024);

Is this correct? Shouldn't that be

tmp = realloc (glfd_fd, (glfd_fd_size + 1024) * sizeof(glfs_fd_t *));

> +static glfs_fd_t *
> +glfd_fd_get (int i)
> +{
> +     return glfd_fd[i];

I'd feel better with a size check here.

> +}
> +
> +static glfs_fd_t *
> +glfd_fd_clear (int i)
> +{
> +     glfs_fd_t *glfd = glfd_fd[i];

Same here for the size check.

> +
> +     glfd_fd[i] = 0;
> +     glfd_fd_used--;
> +     return glfd;
> +}
> +
> +
> +/* Helper to convert stat to stat_ex */
> +
> +static void
> +smb_stat_ex_from_stat (struct stat_ex *dst, const struct stat *src)
> +{
> +     memset (dst, 0, sizeof (*dst));

More Samba-like would be ZERO_STRUCTP(dst).
> +static struct dirent *
> +vfs_gluster_readdir (struct vfs_handle_struct *handle, DIR *dirp,
> +                  SMB_STRUCT_STAT *sbuf)
> +{
> +     static char direntbuf[512];
> +     int ret;
> +     struct stat stat;
> +     struct dirent *dirent = 0;
> +
> +     ret = glfs_readdirplus_r ((void *)dirp, &stat, (void *)direntbuf,
> +                               &dirent);
> +     if (ret)
> +             dirent = NULL;
> +
> +     if (sbuf)
> +             smb_stat_ex_from_stat (sbuf, &stat);

Do you initialize the stat buf even in case of an error?

Regards,

Volker

-- 
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:address@hidden