[PATCH] vfs_glusterfs: Enable per client log file

Poornima Gurusiddaiah pgurusid at redhat.com
Tue Oct 22 00:11:34 MDT 2013


Attaching reworked patch, will submit a different patch for the cleanup suggested by Andreas(implementing get_current_domain()).

Thanks,
Poornima

----- Original Message -----
From: "Christopher R. Hertel" <crh at samba.org>
To: "Andreas Schneider" <asn at samba.org>
Cc: "Samba Technical" <samba-technical at lists.samba.org>
Sent: Tuesday, October 22, 2013 2:36:13 AM
Subject: Re: [PATCH]     vfs_glusterfs: Enable per client log file

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-vfs_glusterfs-Enable-per-client-log-file_master.patch
Type: text/x-patch
Size: 1806 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20131022/f7fcb72e/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-vfs_glusterfs-Enable-per-client-log-file_3.6.patch
Type: text/x-patch
Size: 1830 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20131022/f7fcb72e/attachment-0001.bin>


More information about the samba-technical mailing list