[PATCH 3/3 v4] vfs_glusterfs: Samba VFS module for glusterfs

Simo idra at samba.org
Fri May 24 19:42:43 MDT 2013


On 05/24/2013 07:17 PM, Anand Avati wrote:
> On 5/24/13 5:18 AM, Simo wrote:
>> By using an array you are forced to create this cleanup function,
>> however if you use a doubly linked list you could simply free the fs and
>> use a talloc destructor to perform the clean up. Have you though about
>> doing that ?
>
> I did not look around enough in the samba code to find a list template 
> and did not want to write list primitives from scratch. Is there a 
> specific list template in samba which you recommend?

look for dlinklist.h

> I'm not sure I completely understand what a sub request is (or what an 
> appropriate tevent function call is). I believe this call is required 
> to implement AIO, which we plan to next (as mentioned in the TODO 
> above). Can we leave this as-is for now as the code path is not 
> executed? It will anyways get addressed during the AIO support.

Ok, maybe just add a comment then that this will be supported in the 
near future.

>> Are you sure you do not want to implement this one ?
>> If you return ENOTSUP samba will use vfs_slow_fallocate() which will use
>> pwrite to send a ton of zeros to your filesystem. Is this the most
>> efficient way on gluster ?
>
> fallocate() support is currently under review in glusterfs (likely to 
> be merged soon). This vfs_slow_fallocate() behavior is not a new issue 
> as this has been how it worked with the FUSE mount as well. Once 
> fallocate() support is merged, I will send a new patch to use the API 
> call.
>

Ok, it will definitely make a big difference.

>>> +static int vfs_gluster_kernel_flock(struct vfs_handle_struct *handle,
>>> +                    files_struct *fsp, uint32 share_mode,
>>> +                    uint32_t access_mask)
>>> +{
>>> +    return 0;
>>> +}
>>
>> Lieing about the success of locking is not really a good idea.
>> I think you should return -1 here and advice users of gluster vfs to set
>> "kernel share modes" to false instead.
>
> Does this flock() strictly have to be in a separate lock space from 
> fcntl()? In Linux they are separete, but how does samba deal with 
> other OSes where they compete in the same lockspace? In gluster we 
> have a single lockspace. Will errno = ENOSYS make samba autodetect the 
> lack of 'kernel share modes' support?

As far as I can see in the code the errno is ignored.
A -1 will cause a NT_STATUS_SHARING_VIOLATION

Maybe we should patch the vfs to check the errno value and aout disable 
kernel share modes if ENOTSUP is returned ...

>
>> Are you sure you can always access 'trusted' attributes ?
>> IIRC trusted can only be accessed by root or a process with equivalent
>> capabilities, while samba performs file operations as the user, does
>> gluster do no checking on who is accessing the attributes ?
>> If gluster doesn't then a lot of the code in this module neeeds a ton of
>> access control applied on top.
>
> Ah, changed to "user." namespace. I have been testing as root and the 
> issue hadn't shown up (yet).

Gluster makes this available on both trusted. and user. namespace ? I am 
confused.

> Gluster's ACL is byte-compatible with Linux's POSIX ACL format. The 
> use of acl_copy_int() here is possibly dangerous on non-Linux servers 
> (though in reality most of the OS's POSIX ACL format is byte 
> compatible?). In that sense, maybe it makes sense to have a Gluster 
> format to smb_acl_t converter implemented natively in this module 
> (though it might look similar to smb_acl_to_internal).
>
> This would settle the debate of exposing other module internals too. 
> Thoughts?

Yes, this is probably best.

>> Also I am a bit concerned about how access control is performed when
>> using the glusterfs library directly.
>> Can you shed some light about that please ?
>>
>
> The Samba connects to gluster bricks as a trusted client (running in 
> the same trusted hosts as the bricks, verified by host authentication 
> by IP). This means the library performs getpid() etc. and presents 
> those values as the identity in the RPC request. The gluster bricks 
> perform access control by trusting the presented identity.
>
> Running Samba outside the gluster trusted storage pool requires some 
> more auth work, and we do not plan to support that yet.

I am not sure I understand how this works, if samba is 'trusted' does it 
mean samba is responsible to perform access control ? Because samba 
doesn't, it defers access control to the kernel.

Does gluster use geteuid()/getegid()/getgroups() to find out what are 
the current ids ?

Simo.



More information about the samba-technical mailing list