[PATCH] vfs_glusterfs: Enable per client log file

[Ira Cooper]

I think you are really looking for: lp_parm_talloc_string, it is already
used in vfs_tsmsm.c.

It is already in use in vfs_tsmsm.c.  I hope this will work for everyone.

Thanks,

-Ira


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

> 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
>>>
>>
>>
>