Re: [patch] Cascaded VFS v.4
Hi, Apropos VFS patch shouldn't be -D_LARGEFILE64_SOURCE instead of -D_LARGEFILE63_SOURCE ? And another question: I used VFS to build a antivirus scanner and on close method I need (const struct files_struct*)-fsp_name which position depends by this _LARGEFILE64_SOURCE. If one of smbd and my scanner is build with this flag and other is not, on close method smbd with my scanner will crash. Could be rearranged files in this struct such as fields dependent by compilation flags be the last? Regards, Tudore
Re: [patch] Cascaded VFS v.4
On Tue, Jul 09, 2002 at 12:59:28PM +0300, TUDOR Coserea wrote: Hi, Hi, Tudor! Apropos VFS patch shouldn't be -D_LARGEFILE64_SOURCE instead of -D_LARGEFILE63_SOURCE ? There is no such define in new patch. It was, indeed, in old examples/VFS/block/Makefile but now it is replaced by correct code which is autogenerated with all needed CFLAGS from main configure (source/configure) And another question: I used VFS to build a antivirus scanner and on close method I need (const struct files_struct*)-fsp_name which position depends by this _LARGEFILE64_SOURCE. If one of smbd and my scanner is build with this flag and other is not, on close method smbd with my scanner will crash. Could be rearranged files in this struct such as fields dependent by compilation flags be the last? Could you please show how would this struct look correctly? There are files_struct entries (next,prev), various SMB_XXX (dev, inode, pos, size) which all are variable-sized... -- / Alexander Bokovoy --- Communications satellite used by the military for star wars.
Re: [patch] Cascaded VFS v.4
Hi Alexander,
Alexander Bokovoy [EMAIL PROTECTED] writes:
And another question:
I used VFS to build a antivirus scanner and on close method I
need (const struct files_struct*)-fsp_name which position
depends by this _LARGEFILE64_SOURCE. If one of smbd and my
scanner is build with this flag and other is not, on close
method smbd with my scanner will crash. Could be rearranged
files in this struct such as fields dependent by compilation
flags be the last?
Could you please show how would this struct look correctly? There are
files_struct entries (next,prev), various SMB_XXX (dev, inode, pos, size)
which all are variable-sized...
Like this:
typedef struct files_struct
{
struct files_struct *next, *prev;
int fnum;
struct connection_struct *conn;
int fd;
int print_jobid;
mode_t mode;
uint16 vuid;
write_bmpx_struct *wbmpx_ptr;
write_cache *wcp;
struct timeval open_time;
int share_mode;
uint32 desired_access;
time_t pending_modtime;
int oplock_type;
int sent_oplock_break;
unsigned long file_id;
BOOL can_lock;
BOOL can_read;
BOOL can_write;
BOOL print_file;
BOOL modified;
BOOL is_directory;
BOOL directory_delete_on_close;
BOOL delete_on_close;
char *fsp_name;
SMB_DEV_T dev;
SMB_INO_T inode;
SMB_OFF_T pos;
SMB_OFF_T size;
} files_struct;
now all fsp-mode ... fsp-fsp_name fields will not longer depend by compilation
_LARGEFILE64_SOURCE/_LARGEFILE_SOURCE/_FILE_OFFSET_BITS/_GNU_SOURCE defines.
--
/ Alexander Bokovoy
---
Communications satellite used by the military for star wars.
Re: [patch] Cascaded VFS v.4
On Tue, Jul 09, 2002 at 02:58:51PM +0300, TUDOR Coserea wrote:
Hi Alexander,
Alexander Bokovoy [EMAIL PROTECTED] writes:
And another question:
I used VFS to build a antivirus scanner and on close method I
need (const struct files_struct*)-fsp_name which position
depends by this _LARGEFILE64_SOURCE. If one of smbd and my
scanner is build with this flag and other is not, on close
method smbd with my scanner will crash. Could be rearranged
files in this struct such as fields dependent by compilation
flags be the last?
Could you please show how would this struct look correctly? There are
files_struct entries (next,prev), various SMB_XXX (dev, inode, pos, size)
which all are variable-sized...
Like this:
typedef struct files_struct
{
struct files_struct *next, *prev;
int fnum;
struct connection_struct *conn;
int fd;
int print_jobid;
mode_t mode;
uint16 vuid;
write_bmpx_struct *wbmpx_ptr;
write_cache *wcp;
struct timeval open_time;
int share_mode;
uint32 desired_access;
time_t pending_modtime;
int oplock_type;
int sent_oplock_break;
unsigned long file_id;
BOOL can_lock;
BOOL can_read;
BOOL can_write;
BOOL print_file;
BOOL modified;
BOOL is_directory;
BOOL directory_delete_on_close;
BOOL delete_on_close;
char *fsp_name;
SMB_DEV_T dev;
SMB_INO_T inode;
SMB_OFF_T pos;
SMB_OFF_T size;
} files_struct;
now all fsp-mode ... fsp-fsp_name fields will not longer depend by compilation
_LARGEFILE64_SOURCE/_LARGEFILE_SOURCE/_FILE_OFFSET_BITS/_GNU_SOURCE defines.
Fine for me. Now let's wait few hours when developers from other side of
the planet will wake up :)
--
/ Alexander Bokovoy
---
Nothing is illegal if one hundred businessmen decide to do it.
-- Andrew Young
[patch] Cascaded VFS v.4
Greetings!
Attached is the new revision (v.4) of cascaded VFS patch for HEAD.
Changes from Simo's revision:
-- connection_struct now contains a pointer to smb_vfs_handle_struct
instance which is an element of a linked list. Each element describes
completely one module [private data as void*, dl handle]. Linked list
is managed with appropriate DLIST_XXX macros.
Module developers receive an allocated smb_vfs_handle_struct via
vfs_init() third argument vfs_handle . If they want to store persisent
information and share it between module functions during connection live,
such information should be stored in vfs_handle-data and freed on
connection closing.
-- all modules are loaded w/o exporting their symbols to main smbd
namespace (RTLD_GLOBAL is gone). That means that modules must have only
two public functions: vfs_init() and vfs_done() which are called
directly from smbd's code but not imported into smbd's namespace (there
is no need for it at all -- they are used in place).
-- hence of that, initialization and finalization functions renamed back
to vfs_init() and vfs_done() functions, fstrings/pstrings straggle
removed from smbd/vfs.c code to follow up Samba 3.0 code policy.
-- CFLAGS/LDFLAGS propagation to examples/VFS/ added, along with new, generalised,
examples/VFS/Makefile.in which does not need to be changed to add support for new
modules.
Things not finished yet:
-- Move to lp_list in smbd/vfs.c:vfs_init() module list parsing.
-- Adoption of JFM's function registration proposal [needs brainstorming
on how to preserve operation types with one-function registration API].
As usual, look at comments in include/vfs.h and code in examples/VFs/*.c
to get more infomation and examples of usage.
--
/ Alexander Bokovoy
---
QOTD:
I've always wanted to work in the Federal Mint. And then go on
strike. To make less money.
diff -urN samba-3.0/examples/VFS/audit.c samba-3.0.current/examples/VFS/audit.c
--- samba-3.0/examples/VFS/audit.c 2002-03-20 12:43:38 +0200
+++ samba-3.0.current/examples/VFS/audit.c 2002-07-08 19:49:47 +0300
@@ -3,6 +3,7 @@
* facility.
*
* Copyright (C) Tim Potter, 1999-2000
+ * 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
@@ -47,134 +48,79 @@
/* Function prototypes */
-int audit_connect(struct connection_struct *conn, const char *svc, const char *user);
-void audit_disconnect(struct connection_struct *conn);
-DIR *audit_opendir(struct connection_struct *conn, const char *fname);
-int audit_mkdir(struct connection_struct *conn, const char *path, mode_t mode);
-int audit_rmdir(struct connection_struct *conn, const char *path);
-int audit_open(struct connection_struct *conn, const char *fname, int flags, mode_t
mode);
-int audit_close(struct files_struct *fsp, int fd);
-int audit_rename(struct connection_struct *conn, const char *old, const char *new);
-int audit_unlink(struct connection_struct *conn, const char *path);
-int audit_chmod(struct connection_struct *conn, const char *path, mode_t mode);
-int audit_chmod_acl(struct connection_struct *conn, const char *name, mode_t mode);
-int audit_fchmod(struct files_struct *fsp, int fd, mode_t mode);
-int audit_fchmod_acl(struct files_struct *fsp, int fd, mode_t mode);
+static int audit_connect(struct connection_struct *conn, const char *svc, const char
+*user);
+static void audit_disconnect(struct connection_struct *conn);
+static DIR *audit_opendir(struct connection_struct *conn, const char *fname);
+static int audit_mkdir(struct connection_struct *conn, const char *path, mode_t mode);
+static int audit_rmdir(struct connection_struct *conn, const char *path);
+static int audit_open(struct connection_struct *conn, const char *fname, int flags,
+mode_t mode);
+static int audit_close(struct files_struct *fsp, int fd);
+static int audit_rename(struct connection_struct *conn, const char *old, const char
+*new);
+static int audit_unlink(struct connection_struct *conn, const char *path);
+static int audit_chmod(struct connection_struct *conn, const char *path, mode_t mode);
+static int audit_chmod_acl(struct connection_struct *conn, const char *name, mode_t
+mode);
+static int audit_fchmod(struct files_struct *fsp, int fd, mode_t mode);
+static int audit_fchmod_acl(struct files_struct *fsp, int fd, mode_t mode);
/* VFS operations */
-extern struct vfs_ops default_vfs_ops; /* For passthrough operation */
+static struct vfs_ops default_vfs_ops; /* For passthrough operation */
+static struct smb_vfs_handle_struct *audit_handle;
-struct vfs_ops audit_ops = {
+static vfs_op_tuple audit_ops[] = {
/* Disk operations */
- audit_connect,
- audit_disconnect,
- NULL, /* disk free */
+ {audit_connect, SMB_VFS_OP_CONNECT
Re: Cascaded VFS ...
On Wed, Jul 03, 2002 at 12:32:50PM +0200, C.Lee Taylor wrote: Greetings ... Quick question, is the Cascaded VFS system applied to cvs? Not yet. I'm working on improving it w.r.t. all comments received during past few weeks on this ML. -- / Alexander Bokovoy --- If time heals all wounds, how come the belly button stays the same?
Cascaded VFS ...
Greetings ... Quick question, is the Cascaded VFS system applied to cvs? I don't remember seeing it been applied, but I might have missed it. If I wish to work with it, should I download the cvs or should I download the lastet head alpha tar ball and apply 1.1 patch that Alexander Bokovoy has made. Thanks Mailed Lee
Re: Cascaded VFS patch
On 17 Jun 2002, Simo Sorce wrote:
I've just reviewed Alexander Cascaded VFS patch and found it ok,
now I'm attaching a new diff against current CVS to make it easy to test
before I commit it into the tree.
Unfortunately I have not much time to test it so I have a little request
for list attendees.
Just a note about the VFS I wanted to discuss with alexander at sambaXP
but didn't have the time.
I don't like the idea that the vfs module see the vfs struct and muck with
it at module's load time.
I would prefer something along all the vfs internal tables hidden to the
vfs modules and only some vfs functions exported to the vfs modules.
What I'm thinking is the samba vfs core exports 2 functions:
vfs_register_module() and vfs_register_function(). On module loading (in
the dl_open() function or whatever), the module calls this functions like
that:
module_dlopen()
{
vfs_register_module(my_vfs_module);
vfs_register_function(my_vfs_module, open, my_module_open);
vfs_register_function(my_vfs_module, close, my_module_close);
vfs_register_function(my_vfs_module, read, my_module_read);
vfs_register_function(my_vfs_module, write, my_module_write);
}
I thought of that while listening to the VFS presentation at sambaXP, I
don't see why the vfs structs should be visible and more importantly it
would allow modules to run even if samba vfs struct changes. Vfs modules
maintainers wouldn't have to follow samba developement (and changes) as
close as it's required now.
J.F.
Re: Cascaded VFS patch
On Tue, 2002-06-18 at 09:05, Andrew Bartlett wrote:
Simo Sorce wrote:
+ /* Handles on dlopen() call */
+ smb_vfs_dl_handle *dl_handle;
+ void **vfs_private;
Why a void** ?
Because you really do not know what the module wants to put there, can
be anything.
+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.
I am and this is required for a relly functional cascading mode.
Trivial schemes will not address the needs of modules makers.
SAFE_FREE()
ok, ok
{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?
simple, generally parameters are 1024 bytes limited, leaving out the
path, make it both more readable and less space consuming and make u
easy to move modules to another path, or make it parametrical.
If this option does what I think it does, then it really should be
global.
no because you can have a different path for any different share, each
with it's own set of modules.
+ 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()?
nice catch, need to look more closely to this point.
-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...
if the dl_handle is null we have an error.
/
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.
it really depends on where they are, but I agree in this case, we have
to change these bits, before a commit.
I'm really not conviced that we want to maintain 'backward
compatibility'. It sets a precedent, and I'm not comfortable with it.
I think it is important.
we already are backward compatible for lot of things, why not modules
that will likely be built outside the samba team and need a bit more
stable interface OR backwards compatibility?
Watch that indenting - 8 space tabs please.
oh andrew ...
+ 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' */
Re: Cascaded VFS patch
Jean Francois Micouleau wrote:
On 17 Jun 2002, Simo Sorce wrote:
I've just reviewed Alexander Cascaded VFS patch and found it ok,
now I'm attaching a new diff against current CVS to make it easy to test
before I commit it into the tree.
Unfortunately I have not much time to test it so I have a little request
for list attendees.
Just a note about the VFS I wanted to discuss with alexander at sambaXP
but didn't have the time.
I don't like the idea that the vfs module see the vfs struct and muck with
it at module's load time.
I would prefer something along all the vfs internal tables hidden to the
vfs modules and only some vfs functions exported to the vfs modules.
I'm certainly a fan of hiding structs - there is a lot less damage these
things can do when they don't have access to 'the wrong things'.
What I'm thinking is the samba vfs core exports 2 functions:
vfs_register_module() and vfs_register_function(). On module loading (in
the dl_open() function or whatever), the module calls this functions like
that:
module_dlopen()
{
vfs_register_module(my_vfs_module);
vfs_register_function(my_vfs_module, open, my_module_open);
vfs_register_function(my_vfs_module, close, my_module_close);
vfs_register_function(my_vfs_module, read, my_module_read);
vfs_register_function(my_vfs_module, write, my_module_write);
}
I thought of that while listening to the VFS presentation at sambaXP, I
don't see why the vfs structs should be visible and more importantly it
would allow modules to run even if samba vfs struct changes. Vfs modules
maintainers wouldn't have to follow samba developement (and changes) as
close as it's required now.
This sounds interesting, and would apply equally well for pdb and auth
modules.
It also works much better for the builtin-and-or-external case.
My only issue is with the complete loss of compile-time type checking.
How should that be addressed?
Andrew Bartlett,
--
Andrew Bartlett [EMAIL PROTECTED]
Manager, Authentication Subsystems, Samba Team [EMAIL PROTECTED]
Student Network Administrator, Hawker College [EMAIL PROTECTED]
http://samba.org http://build.samba.org http://hawkerc.net
Re: Cascaded VFS patch
Simo Sorce wrote:
On Tue, 2002-06-18 at 09:05, Andrew Bartlett wrote:
Simo Sorce wrote:
+ /* Handles on dlopen() call */
+ smb_vfs_dl_handle *dl_handle;
+ void **vfs_private;
Why a void** ?
Because you really do not know what the module wants to put there, can
be anything.
I would have a void* thats all.
+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.
I am and this is required for a relly functional cascading mode.
Trivial schemes will not address the needs of modules makers.
SAFE_FREE()
ok, ok
{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?
simple, generally parameters are 1024 bytes limited, leaving out the
path, make it both more readable and less space consuming and make u
easy to move modules to another path, or make it parametrical.
If this option does what I think it does, then it really should be
global.
no because you can have a different path for any different share, each
with it's own set of modules.
I'll defer on this one - its not a major issue.
+ 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()?
nice catch, need to look more closely to this point.
-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...
if the dl_handle is null we have an error.
I don't like signaling errors/status by changing a paratmer - I think it
is better to actually look at the return value. Thats the point I was
trying to make.
/
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.
it really depends on where they are, but I agree in this case, we have
to change these bits, before a commit.
I'm really not conviced that we want to maintain 'backward
compatibility'. It sets a precedent, and I'm not comfortable with it.
I think it is important.
we already are backward compatible for lot of things, why not modules
that will likely be built outside the samba team and need a bit more
stable interface OR backwards
