[PATCH] vfs_glusterfs: Enable per client log file

[Ira Cooper]

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
>