[PATCH] vfs_glusterfs: Enable per client log file

[Ira Cooper]

My bad... that's a static function.

-Ira


On Mon, Nov 4, 2013 at 12:31 PM, Ira Cooper <ira at samba.org> wrote:

> Andrew:  it is in poor taste to drop the patch author, it makes it hard
> for me to follow up to you.
>
> All:
>
> On the "technical issue" here: Regardless of my opinions of the VFS going
> into Samba's guts or not, the answer is "Use the right API."
>
> Please look at lp_string(), and I think you'll see a satisfactory answer
> to what you are trying to do.
>
> Poornima:
>
> Please use your full name to commit to Samba, it makes tracking the
> copyright of the project much easier.
>
> Thanks,
>
> -Ira
>
>
>
> On Tue, Oct 22, 2013 at 2:11 AM, Poornima Gurusiddaiah <
> pgurusid at redhat.com> wrote:
>
>>
>> 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
>>
>
>