Cascaded VFS patch

Andrew Bartlett abartlet at samba.org
Tue Jun 18 00:11:03 GMT 2002


Simo Sorce wrote:
> 
> oops, forgot to attach the patch ;)

I'm glad you did - I do have issues with this patch.

> Index: include/smb.h
> ===================================================================
> RCS file: /data/cvs/samba/source/include/smb.h,v
> retrieving revision 1.435
> diff -u -r1.435 smb.h
> --- include/smb.h       2002/06/15 12:38:10     1.435
> +++ include/smb.h       2002/06/17 08:46:49
> @@ -447,9 +447,9 @@
>         char *origpath;
> 
>         struct vfs_ops vfs_ops;                   /* Filesystem operations */
> -       /* Handle on dlopen() call */
> -       void *dl_handle;
> -       void *vfs_private;
> +       /* Handles on dlopen() call */
> +       smb_vfs_dl_handle *dl_handle;
> +       void **vfs_private;

Why a void** ?

>         char *user; /* name of user who *opened* this connection */
>         uid_t uid; /* uid of user who *opened* this connection */
> Index: include/vfs.h
> ===================================================================
> RCS file: /data/cvs/samba/source/include/vfs.h,v
> retrieving revision 1.25
> diff -u -r1.25 vfs.h
> --- include/vfs.h       2002/03/22 06:24:35     1.25
> +++ include/vfs.h       2002/06/17 08:46:49
> @@ -1,7 +1,8 @@
>  /*
>     Unix SMB/CIFS implementation.
>     VFS structures and parameters
> -   Copyright (C) Tim Potter 1999
> +   Copyright (C) Tim Potter                            1999
> +   Copyright (C) Alexander Bokovoy                     2002
> 
>     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
> @@ -16,6 +17,8 @@
>     You should have received a copy of the GNU General Public License
>     along with this program; if not, write to the Free Software
>     Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
> +
> +   This work was sponsored by Optifacio Software Services, Inc.
>  */
> 
>  #ifndef _VFS_H
> @@ -40,8 +43,42 @@
> 
>  /* Changed to version 2 for CIFS UNIX extensions (mknod and link added). JRA. */
>  /* Changed to version 3 for POSIX acl extensions. JRA. */
> -#define SMB_VFS_INTERFACE_VERSION 3
> +/* Changed to version 4 for cascaded VFS interface. Alexander Bokovoy. */
> +#define SMB_VFS_INTERFACE_VERSION 5
> +
> +/* Version of supported cascaded interface backward copmatibility.
> +   (version 4 corresponds to SMB_VFS_INTERFACE_VERSION 4)
> +   It is used in vfs_init_custom() to detect VFS modules which conform to cascaded
> +   VFS interface but implement elder version than current version of Samba uses.
> +   This allows to use old modules with new VFS interface as far as combined VFS operation
> +   set is coherent (will be in most cases).
> +*/
> +#define SMB_VFS_INTERFACE_CASCADED 4
> 
> +/*
> +    Each VFS module must provide following global functions:
> +    vfs_init_<modulename>      -- initialization function

Why _<modulename>.  We are doing a dlsym() on each indiviaul object
file, I would 
like the entrypoint to be conisistant.  Don't worry about compiled
in/not compiled in at this stage, thats better handled as an #ifdef
inside the module.

> +    vfs_done_<modulename>      -- finalization function

Why can't this be in the vfs_op_tuple[] array?

> +    vfs_init_<modulename> must return proper initialized vfs_op_tuple[] array
> +    which describes all operations this module claims to intercept. This function
> +    is called whenever module is loaded into smbd process using sys_dlopen().
> +
> +    vfs_done_<modulename> must perform finalization of the module. In particular,
> +    this function must free vfs_ops structure returned to module from smb_vfs_get_opaque_ops()
> +    function if it is used (see below). This function is called whenever module
> +    is unloaded from smbd process using sys_dlclose().
> +
> +    Prototypes:
> +    vfs_op_tuple *vfs_init_<modulename>(int *vfs_version, const struct vfs_ops *def_vfs_ops, int vfs_id);
> +    void         vfs_done_<modulename>(connection_struct *conn);
> +
> +    All intercepted VFS operations must be declared as static functions inside module source
> +    in order to keep smbd namespace unpolluted. See source of skel, audit, and recycle bin
> +    example VFS modules for more details.
> +
> +*/
> +
>  /* VFS operations structure */
> 
>  struct connection_struct;
> @@ -134,5 +171,164 @@
>      char *name;
>      char *value;
>  };
> +
> +/*
> +    Available VFS operations. These values must be in sync with vfs_ops struct.
> +    In particular, if new operations are added to vfs_ops, appropriate constants
> +    should be added to vfs_op_type so that order of them kept same as in vfs_ops.
> +*/
> +
> +typedef enum _vfs_op_type {
> +
> +       SMB_VFS_OP_NOOP = -1,
> +
> +       /* Disk operations */
> +
> +       SMB_VFS_OP_CONNECT = 0,
> +       SMB_VFS_OP_DISCONNECT,
> +       SMB_VFS_OP_DISK_FREE,
> +
> +       /* Directory operations */
> +
> +       SMB_VFS_OP_OPENDIR,
> +       SMB_VFS_OP_READDIR,
> +       SMB_VFS_OP_MKDIR,
> +       SMB_VFS_OP_RMDIR,
> +       SMB_VFS_OP_CLOSEDIR,
> +
> +       /* File operations */
> +
> +       SMB_VFS_OP_OPEN,
> +       SMB_VFS_OP_CLOSE,
> +       SMB_VFS_OP_READ,
> +       SMB_VFS_OP_WRITE,
> +       SMB_VFS_OP_LSEEK,
> +       SMB_VFS_OP_RENAME,
> +       SMB_VFS_OP_FSYNC,
> +       SMB_VFS_OP_STAT,
> +       SMB_VFS_OP_FSTAT,
> +       SMB_VFS_OP_LSTAT,
> +       SMB_VFS_OP_UNLINK,
> +       SMB_VFS_OP_CHMOD,
> +       SMB_VFS_OP_FCHMOD,
> +       SMB_VFS_OP_CHOWN,
> +       SMB_VFS_OP_FCHOWN,
> +       SMB_VFS_OP_CHDIR,
> +       SMB_VFS_OP_GETWD,
> +       SMB_VFS_OP_UTIME,
> +       SMB_VFS_OP_FTRUNCATE,
> +       SMB_VFS_OP_LOCK,
> +       SMB_VFS_OP_SYMLINK,
> +       SMB_VFS_OP_READLINK,
> +       SMB_VFS_OP_LINK,
> +       SMB_VFS_OP_MKNOD,
> +       SMB_VFS_OP_REALPATH,
> +
> +       /* NT ACL operations. */
> +
> +       SMB_VFS_OP_FGET_NT_ACL,
> +       SMB_VFS_OP_GET_NT_ACL,
> +       SMB_VFS_OP_FSET_NT_ACL,
> +       SMB_VFS_OP_SET_NT_ACL,
> +
> +       /* POSIX ACL operations. */
> +
> +       SMB_VFS_OP_CHMOD_ACL,
> +       SMB_VFS_OP_FCHMOD_ACL,
> +
> +       SMB_VFS_OP_SYS_ACL_GET_ENTRY,
> +       SMB_VFS_OP_SYS_ACL_GET_TAG_TYPE,
> +       SMB_VFS_OP_SYS_ACL_GET_PERMSET,
> +       SMB_VFS_OP_SYS_ACL_GET_QUALIFIER,
> +       SMB_VFS_OP_SYS_ACL_GET_FILE,
> +       SMB_VFS_OP_SYS_ACL_GET_FD,
> +       SMB_VFS_OP_SYS_ACL_CLEAR_PERMS,
> +       SMB_VFS_OP_SYS_ACL_ADD_PERM,
> +       SMB_VFS_OP_SYS_ACL_TO_TEXT,
> +       SMB_VFS_OP_SYS_ACL_INIT,
> +       SMB_VFS_OP_SYS_ACL_CREATE_ENTRY,
> +       SMB_VFS_OP_SYS_ACL_SET_TAG_TYPE,
> +       SMB_VFS_OP_SYS_ACL_SET_QUALIFIER,
> +       SMB_VFS_OP_SYS_ACL_SET_PERMSET,
> +       SMB_VFS_OP_SYS_ACL_VALID,
> +       SMB_VFS_OP_SYS_ACL_SET_FILE,
> +       SMB_VFS_OP_SYS_ACL_SET_FD,
> +       SMB_VFS_OP_SYS_ACL_DELETE_DEF_FILE,
> +       SMB_VFS_OP_SYS_ACL_GET_PERM,
> +       SMB_VFS_OP_SYS_ACL_FREE_TEXT,
> +       SMB_VFS_OP_SYS_ACL_FREE_ACL,
> +       SMB_VFS_OP_SYS_ACL_FREE_QUALIFIER,
> +
> +       /* This should always be last enum value */
> +
> +       SMB_VFS_OP_LAST
> +} vfs_op_type;
> +
> +/*
> +    Possible VFS operation layers (per-operation)
> +
> +    These values are used by VFS subsystem when building vfs_ops for connection
> +    from multiple VFS modules. Internally, Samba differentiates only opaque and
> +    transparent layers at this process. Other types are used for providing better
> +    diagnosing facilities.
> +
> +    Most modules will provide transparent layers. Opaque layer is for modules
> +    which implement actual file system calls (like DB-based VFS). For example,
> +    default POSIX VFS which is built in into Samba is an opaque VFS module.
> +
> +    Other layer types (audit, splitter, scanner) were designed to provide different
> +    degree of transparency and for diagnosing VFS module behaviour.
> +
> +    Each module can implement several layers at the same time provided that only
> +    one layer is used per each operation.
> +
> +*/
> +
> +typedef enum _vfs_op_layer {
> +       SMB_VFS_LAYER_NOOP = -1,        /* - For using in VFS module to indicate end of array */
> +                                       /*   of operations description */
> +       SMB_VFS_LAYER_OPAQUE = 0,       /* - Final level, does not call anything beyond itself */
> +       SMB_VFS_LAYER_TRANSPARENT,      /* - Normal operation, calls underlying layer after */
> +                                       /*   possibly changing passed data */
> +       SMB_VFS_LAYER_LOGGER,           /* - Logs data, calls underlying layer, logging does not */
> +                                       /*   use Samba VFS */
> +       SMB_VFS_LAYER_SPLITTER,         /* - Splits operation, calls underlying layer _and_ own facility, */
> +                                       /*   then combines result */
> +       SMB_VFS_LAYER_SCANNER           /* - Checks data and possibly initiates additional */
> +                                       /*   file activity like logging to files _inside_ samba VFS */
> +} vfs_op_layer;

I'm still not conviced on this stuff.

> +/*
> +    VFS operation description. Each VFS module initialization function returns to VFS subsystem
> +    an array of vfs_op_tuple which describes all operations this module is willing to intercept.
> +    VFS subsystem initializes then vfs_ops using this information and passes it
> +    to next VFS module as underlying vfs_ops and to connection after all VFS modules are initialized.
> +*/
> +
> +typedef struct _vfs_op_tuple {
> +       void* op;
> +       vfs_op_type type;
> +       vfs_op_layer layer;
> +} vfs_op_tuple;
> +
> +/* VFS module handle is used in connection_struct for proper finalization of VFS module */
> +typedef struct _smb_vfs_dl_handle {
> +       void* handle;
> +       char* done_method;
> +} smb_vfs_dl_handle;
> +
> +/*
> +    Return vfs_ops filled with current opaque VFS operations. This function is designed to
> +    be called from VFS module initialization function for those modules which needs 'direct' VFS
> +    access (loggers or initiators of file operations other than connection asks for).
> +
> +    Returned vfs_ops must be cleaned up in VFS module's finalizer function (vfs_done_<module_name>)
> +    using safe_free().

SAFE_FREE()

> +    Prototype:
> +    struct vfs_ops *smb_vfs_get_opaque_ops();
> +
> +    This prototype will be available via include/proto.h
> +*/
> 
>  #endif /* _VFS_H */
> Index: param/loadparm.c
> ===================================================================
> RCS file: /data/cvs/samba/source/param/loadparm.c,v
> retrieving revision 1.415
> diff -u -r1.415 loadparm.c
> --- param/loadparm.c    2002/06/15 13:05:37     1.415
> +++ param/loadparm.c    2002/06/17 08:46:49
> @@ -312,6 +312,7 @@
>         char *fstype;
>         char *szVfsObjectFile;
>         char *szVfsOptions;
> +       char *szVfsPath;
>         int iMinPrintSpace;
>         int iMaxPrintJobs;
>         int iWriteCacheSize;
> @@ -429,6 +430,7 @@
>         NULL,                   /* fstype */
>         NULL,                   /* vfs object */
>         NULL,                   /* vfs options */
> +       NULL,                   /* vfs path */
>         0,                      /* iMinPrintSpace */
>         1000,                   /* iMaxPrintJobs */
>         0,                      /* iWriteCacheSize */
> @@ -1017,6 +1019,7 @@
> 
>         {"vfs object", P_STRING, P_LOCAL, &sDefault.szVfsObjectFile, handle_vfs_object, NULL, FLAG_SHARE},
>         {"vfs options", P_STRING, P_LOCAL, &sDefault.szVfsOptions, NULL, NULL, FLAG_SHARE},
> +       {"vfs path", P_STRING, P_LOCAL, &sDefault.szVfsPath, NULL, NULL, FLAG_SHARE},

Why do we need a new smb.conf option?  Why can't we just have full paths
on objects?

> 
>         {"msdfs root", P_BOOL, P_LOCAL, &sDefault.bMSDfsRoot, NULL, NULL, FLAG_SHARE},
> @@ -1636,6 +1639,7 @@
>  FN_LOCAL_STRING(lp_fstype, fstype)
>  FN_LOCAL_STRING(lp_vfsobj, szVfsObjectFile)
>  FN_LOCAL_STRING(lp_vfs_options, szVfsOptions)
> +FN_LOCAL_STRING(lp_vfs_path, szVfsPath)

If this option does what I think it does, then it really should be
global.  

>  static FN_LOCAL_STRING(lp_volume, volume)
>  FN_LOCAL_STRING(lp_mangled_map, szMangledMap)
>  FN_LOCAL_STRING(lp_veto_files, szVetoFiles)
> Index: smbd/conn.c
> ===================================================================
> RCS file: /data/cvs/samba/source/smbd/conn.c,v
> retrieving revision 1.17
> diff -u -r1.17 conn.c
> --- smbd/conn.c 2002/05/17 06:15:06     1.17
> +++ smbd/conn.c 2002/06/17 08:46:49
> @@ -2,6 +2,7 @@
>     Unix SMB/CIFS implementation.
>     Manage connections_struct structures
>     Copyright (C) Andrew Tridgell 1998
> +   Copyright (C) Alexander Bokovoy 2002
> 
>     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
> @@ -162,11 +163,25 @@
> 
>  void conn_free(connection_struct *conn)
>  {
> +       smb_vfs_dl_handle *dl_handle;
> +       void (*done_fptr)(connection_struct *conn);
> +
>         /* Free vfs_connection_struct */
> 
>         if (conn->dl_handle != NULL) {
> -               /* Close dlopen() handle */
> -               sys_dlclose(conn->dl_handle);
> +               for(dl_handle = conn->dl_handle; dl_handle->handle; dl_handle++) {
> +                       /* Close dlopen() handle */
> +                       done_fptr = (void (*)(connection_struct *))sys_dlsym(dl_handle->handle, dl_handle->done_method);
> +
> +                       if (done_fptr == NULL) {
> +                               DEBUG(3, ("No %s() symbol found in module with handle %p, ignoring\n", dl_handle->done_method, dl_handle->handle));
> +                       } else {
> +                               done_fptr(conn);
> +                       }
> +                       sys_dlclose(dl_handle->handle);
> +                       string_free(&dl_handle->done_method);

Why a string_free()?

> +               }
> +               SAFE_FREE(conn->dl_handle);
>         }
> 
>         DLIST_REMOVE(Connections, conn);
> Index: smbd/vfs.c
> ===================================================================
> RCS file: /data/cvs/samba/source/smbd/vfs.c,v
> retrieving revision 1.58
> diff -u -r1.58 vfs.c
> --- smbd/vfs.c  2002/05/17 06:15:06     1.58
> +++ smbd/vfs.c  2002/06/17 08:46:49
> @@ -3,6 +3,7 @@
>     Version 1.9.
>     VFS initialisation and support functions
>     Copyright (C) Tim Potter 1999
> +   Copyright (C) Alexander Bokovoy 2002
> 
>     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
> @@ -17,6 +18,8 @@
>     You should have received a copy of the GNU General Public License
>     along with this program; if not, write to the Free Software
>     Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
> +
> +   This work was sponsored by Optifacio Software Services, Inc.
>  */
> 
>  #include "includes.h"
> @@ -28,11 +31,17 @@
>         void *fptr;
>  };
> 
> +/*
> +  Opaque (final) vfs operations. This is a combination of first-met opaque vfs operations
> +  across all currently processed modules.  */
> +
> +static vfs_op_tuple vfs_opaque_ops[SMB_VFS_OP_LAST];
> +
>  /* Default vfs hooks.  WARNING: The order of these initialisers is
>     very important.  They must be in the same order as defined in
>     vfs.h.  Change at your own peril. */
> 
> -struct vfs_ops default_vfs_ops = {
> +static struct vfs_ops default_vfs_ops = {
> 
>         /* Disk operations */
> 
> @@ -117,58 +126,85 @@
>    initialise default vfs hooks
>  ****************************************************************************/
> 
> -static BOOL vfs_init_default(connection_struct *conn)
> +static void vfs_init_default(connection_struct *conn)
>  {
>         DEBUG(3, ("Initialising default vfs hooks\n"));
> 
>         memcpy(&conn->vfs_ops, &default_vfs_ops, sizeof(struct vfs_ops));
> -       return True;
> +       conn->dl_handle = NULL;
>  }

Why?  I think we should return errors on failure...

>  /****************************************************************************
>    initialise custom vfs hooks
>  ****************************************************************************/
> 
> -static BOOL vfs_init_custom(connection_struct *conn)
> +static BOOL vfs_init_custom(connection_struct *conn, int vfs_number, const char *vfs_object)
>  {
>         int vfs_version = -1;
> -       struct vfs_ops *ops, *(*init_fptr)(int *, struct vfs_ops *);
> +       vfs_op_tuple *ops, *(*init_fptr)(int *, const struct vfs_ops *, int);
> +       fstring method_init_name;
> +       fstring vfs_object_name;

fstrings are BAD.  We might use a lot of them, but *all* new code needs
to use 
asprintf() etc.  

> +       char *sep;
> +       int i;
> 
> -       DEBUG(3, ("Initialising custom vfs hooks from %s\n", lp_vfsobj(SNUM(conn))));
> +       DEBUG(3, ("Initialising custom vfs hooks from %s\n", vfs_object));
> 
>         /* Open object file */
> 
> -       if ((conn->dl_handle = sys_dlopen(lp_vfsobj(SNUM(conn)), RTLD_NOW | RTLD_GLOBAL)) == NULL) {
> -               DEBUG(0, ("Error opening %s: %s\n", lp_vfsobj(SNUM(conn)), sys_dlerror()));
> +       if ((conn->dl_handle[vfs_number].handle = sys_dlopen(vfs_object, RTLD_NOW | RTLD_GLOBAL)) == NULL) {
> +               DEBUG(0, ("Error opening %s: %s\n", vfs_object, sys_dlerror()));
>                 return False;
>         }
> 
> +       /* Prepare symbols for initializer and finalizer */
> +       fstrcpy(vfs_object_name, strrchr_m(vfs_object, '/')+1);
> +       sep = strrchr_m(vfs_object_name, '.');
> +       *sep = 0;
> +       asprintf(&(conn->dl_handle[vfs_number].done_method), "vfs_done_%s", vfs_object_name);
> +       fstr_sprintf(method_init_name, "vfs_init_%s", vfs_object_name);
> +
>         /* Get handle on vfs_init() symbol */
> 
> -       init_fptr = (struct vfs_ops *(*)(int *, struct vfs_ops *))sys_dlsym(conn->dl_handle, "vfs_init");
> +       init_fptr = (vfs_op_tuple *(*)(int *, const struct vfs_ops *, int))sys_dlsym(conn->dl_handle[vfs_number].handle, method_init_name);
> 
>         if (init_fptr == NULL) {
> -               DEBUG(0, ("No vfs_init() symbol found in %s\n", lp_vfsobj(SNUM(conn))));
> +               DEBUG(0, ("No %s() symbol found in %s\n", method_init_name, vfs_object));
>                 return False;
>         }
> 
>         /* Initialise vfs_ops structure */
> -
> -       conn->vfs_ops = default_vfs_ops;
> -
> -       if ((ops = init_fptr(&vfs_version, &default_vfs_ops)) == NULL) {
> -               DEBUG(0, ("vfs_init function from %s failed\n", lp_vfsobj(SNUM(conn))));
> -               return False;
> -       }
> -
> -       if (vfs_version != SMB_VFS_INTERFACE_VERSION) {
> -               DEBUG(0, ("vfs_init returned wrong interface version info (was %d, should be %d)\n",
> -                       vfs_version, SMB_VFS_INTERFACE_VERSION ));
> -               return False;
> -       }
> 
> -       if (ops != &conn->vfs_ops) {
> -               memcpy(&conn->vfs_ops, ops, sizeof(struct vfs_ops));
> +       if ((ops = init_fptr(&vfs_version, &conn->vfs_ops, vfs_number)) == NULL) {
> +               DEBUG(0, ("%s function from %s failed\n", method_init_name, vfs_object));
> +               return False;
> +       }
> +
> +       if ((vfs_version < SMB_VFS_INTERFACE_CASCADED)) {
> +               DEBUG(0, ("%s returned wrong interface version info (was %d, should be no less than %d)\n",
> +                       method_init_name, vfs_version, SMB_VFS_INTERFACE_VERSION ));
> +               return False;
> +       }
> +
> +       if ((vfs_version < SMB_VFS_INTERFACE_VERSION)) {
> +               DEBUG(0, ("Warning: %s states that module confirms interface version #%d, current interface version is #%d.\n\
> +Proceeding in compatibility mode, new operations (since version #%d) will fallback to default ones.\n",
> +                       method_init_name, vfs_version, SMB_VFS_INTERFACE_VERSION, vfs_version ));
> +               return False;
> +       }

I'm really not conviced that we want to maintain 'backward
compatibility'.  It sets a precedent, and I'm not comfortable with it.

> +       for(i=0; ops[i].op != NULL; i++) {
> +         DEBUG(3, ("Checking operation #%d (type %d, layer %d)\n", i, ops[i].type, ops[i].layer));
> +         if(ops[i].layer == SMB_VFS_LAYER_OPAQUE) {
> +           /* Check whether this operation was already made opaque by different module */
> +           if(vfs_opaque_ops[ops[i].type].op == ((void**)&default_vfs_ops)[ops[i].type]) {
> +             /* No, it isn't overloaded yet. Overload. */
> +             DEBUG(3, ("Making operation type %d opaque [module %s]\n", ops[i].type, vfs_object));
> +             vfs_opaque_ops[ops[i].type] = ops[i];
> +           }
> +         }
> +         /* Change current VFS disposition*/
> +         DEBUG(3, ("Accepting operation type %d from module %s\n", ops[i].type, vfs_object));
> +         ((void**)&conn->vfs_ops)[ops[i].type] = ops[i].op;
>         }

Watch that indenting - 8 space tabs please.

>         return True;
> @@ -180,21 +216,70 @@
> 
>  BOOL smbd_vfs_init(connection_struct *conn)
>  {
> +       char **vfs_objects, *vfsobj, *vfs_module, *vfs_path;
> +       int nobj, i;
> +
> +       /* Normal share - initialise with disk access functions */
> +       vfs_init_default(conn);
> +
> +       /* Override VFS functions if 'vfs object' was specified*/
>         if (*lp_vfsobj(SNUM(conn))) {
> -
> -               /* Loadable object file */
> -
> -               if (!vfs_init_custom(conn)) {
> -                       DEBUG(0, ("smbd_vfs_init: vfs_init_custom failed\n"));
> -                       return False;
> +               vfsobj = NULL;
> +               for(i=0; i<SMB_VFS_OP_LAST; i++) {
> +                 vfs_opaque_ops[i].op = ((void**)&default_vfs_ops)[i];
> +                 vfs_opaque_ops[i].type = i;
> +                 vfs_opaque_ops[i].layer = SMB_VFS_LAYER_OPAQUE;
>                 }
> -
> -               return True;
> +               if (string_set(&vfsobj, lp_vfsobj(SNUM(conn)))) {
> +                       /* Parse passed modules specification to array of modules */
> +                       set_first_token(vfsobj);
> +                       /* We are using default separators: ' \t\r\n' */
> +                       vfs_objects = toktocliplist(&nobj, NULL);

What are you trying here?  Why can't this be a normal lp_list?

> +                       if (vfs_objects) {
> +                               vfs_path = lp_vfs_path(SNUM(conn));
> +                               conn->dl_handle = smb_xmalloc((nobj+1)*sizeof(smb_vfs_dl_handle));
> +                               for(i=nobj-1; i>=0; i--) {
> +                                       /* Loadable object file */
> +                                       conn->dl_handle[i].handle = NULL;
> +                                       conn->dl_handle[i].done_method = NULL;
> +                                       vfs_module = NULL;
> +                                       if (vfs_path) {
> +                                               asprintf(&vfs_module, "%s/%s", vfs_path, vfs_objects[i]);
> +                                       } else {
> +                                               asprintf(&vfs_module, "%s", vfs_objects[i]);
> +                                       }
> +                                       if (!vfs_init_custom(conn, i, vfs_module)) {
> +                                               DEBUG(0, ("smbd_vfs_init: vfs_init_custom failed for %s\n", vfs_module));
> +                                               string_free(&vfsobj);
> +                                               SAFE_FREE(vfs_module);
> +                                               return False;
> +                                       }
> +                                       SAFE_FREE(vfs_module);
> +                               }
> +                               conn->dl_handle[nobj].handle = NULL;
> +                               conn->dl_handle[nobj].done_method = NULL;
> +                       }
> +                       string_free(&vfsobj);
> +                       return True;
> +               }
>         }
> -
> -       /* Normal share - initialise with disk access functions */
> -
> -       return vfs_init_default(conn);
> +       return True;
> +}
> +
> +/*******************************************************************
> + Create vfs_ops reflecting current vfs_opaque_ops
> +*******************************************************************/
> +struct vfs_ops *smb_vfs_get_opaque_ops()
> +{
> +  int i;
> +  struct vfs_ops *ops;
> +
> +  ops = smb_xmalloc(sizeof(struct vfs_ops));
> +
> +  for(i=0; i<SMB_VFS_OP_LAST; i++) {
> +    ((void**)ops)[i] = vfs_opaque_ops[i].op;
> +  }
> +  return ops;

As you can see, I'm not happy with a few things in this patch.  

idra:  Can you hold off on your commit?  While I don't mind (to much)
the vfsclient issue, these things need addressing.  Yes they are simple,
and yes, nobody will fix them later...

Andrew Bartlett

-- 
Andrew Bartlett                                 abartlet at pcug.org.au
Manager, Authentication Subsystems, Samba Team  abartlet at samba.org
Student Network Administrator, Hawker College   abartlet at hawkerc.net
http://samba.org     http://build.samba.org     http://hawkerc.net




More information about the samba-technical mailing list