[PATCH] vfs_glusterfs: Enable per client log file
Christopher R. Hertel
crh at samba.org
Mon Oct 21 15:06:13 MDT 2013
Andreas,
As Poornima pointed out to me, current_user_info.domain is accessed directly
in a few places in Samba. Printing and messaging, in particular. If we
were to remove 'extern userdom_struct current_user_info;' and use
'get_current_username()', as you suggest, we would need to update a few (not
too many) other files as well.
I agree that it should be done, and Poornima has identified the right place
to to add the function, but I would like to separate out that fix from the
others.
Poornima's patch is about splitting up log files generated by the
vfs_glusterfs module. Your suggestion is about a broader (though quite
simple) cleanup in Samba. Once we have Poornima's patch accepted and
applied, I'll follow up with the cleanup you've suggested.
Andrew Bartlett: Yes, the marshalling fixes you requested a while back are
still on my radar.
Chris -)-----
On 10/18/2013 07:35 AM, Andreas Schneider wrote:
> On Friday 18 October 2013 06:51:23 Poornima Gurusiddaiah wrote:
>
> Could you please do some modifications to your patch?
>
>> +extern userdom_struct current_user_info;
>> +
>
> Please remove this.
>
>> /* Helpers to provide 'integer' fds */
>>
>> /* This is global. gfapi's FD operations do not
>>
>> @@ -205,13 +207,17 @@ static int vfs_gluster_connect(struct
>> vfs_handle_struct *handle,>
>> {
>>
>> const char *volfile_server;
>> const char *volume;
>>
>> - const char *logfile;
>> + const char *logfile_nosub;
>> + char logfile[PATH_MAX] = {0,};
>
> You can remove the , here. ^
>
>>
>> int loglevel;
>> glfs_t *fs;
>> int ret;
>>
>> - logfile = lp_parm_const_string(SNUM(handle->conn), "glusterfs",
>> + logfile_nosub = lp_parm_const_string(SNUM(handle->conn), "glusterfs",
>>
>> "logfile", NULL);
>>
>> + strncpy(logfile, logfile_nosub, (PATH_MAX-1));
>> + logfile[PATH_MAX] = '\0';
>> + standard_sub_basic(get_current_username(), current_user_info.domain,
>> logfile, (PATH_MAX-1));
>
> Please implement a get_current_domain() like get_current_username() cause we
> will try to get rid of the globals in future and using a function means we
> need to touch less code.
>
> Also I would suggest to use sizeof(logfile) - 1 instead of PATH_MAX here.
>
>> loglevel = lp_parm_int(SNUM(handle->conn), "glusterfs", "loglevel", -1);
>
>
> Cheers,
>
>
> -- andreas
>
--
"Implementing CIFS - the Common Internet FileSystem" ISBN: 013047116X
Samba Team -- http://www.samba.org/ -)----- Christopher R. Hertel
jCIFS Team -- http://jcifs.samba.org/ -)----- ubiqx development, uninq.
ubiqx Team -- http://www.ubiqx.org/ -)----- crh at ubiqx.mn.org
OnLineBook -- http://ubiqx.org/cifs/ -)----- crh at ubiqx.org
More information about the samba-technical
mailing list