[PATCH] Fix Gluster bug - unable to create file/directory under root of the share due to empty dirpath during get_real_filename()

[Michael Adam]

On 2018-08-22 at 12:45 -0400, Ira Cooper wrote:
> Pardon.
> 
> It was me.
> 
> Reviewed-by: Ira Cooper <ira at samba.org>
> 
> I miss hit the reply/reply all button.

Ok, thanks for clarifying. :-)

Still, I see it does the job but don't understand
(given the commit message) why the corresponding
change at the vfs_glusterfs level wouldn't have been
more appropriate.

Thanks - Michael



> Thanks,
> 
> -Ira
> 
> On Wed, Aug 22, 2018 at 12:43 PM Michael Adam via samba-technical <
> samba-technical at lists.samba.org> wrote:
> 
> > On 2018-08-22 at 09:27 -0700, Jeremy Allison wrote:
> > > On Wed, Aug 22, 2018 at 06:09:25PM +0200, Michael Adam wrote:
> > > >
> > > > Conceptually, changing the return from "" to "."
> > > > and changing all existing callers that check for ""
> > > > afterwards is semantically not changing much really, is it?
> > > >
> > > > So I don't get the big concern about this change.
> > >
> > > Michael you have *no* idea :-).
> > >
> > > Look into the statcache code (which is *really* old)
> > > which also gets passed the dirpath to make sure that
> > > is still working if you make this change.
> > >
> > > Historically, working with the difference between
> > > "" and "." in pathnames has been the source of
> > > many, *many* Samba pathname bugs.
> > >
> > > I have (a lot of) historical context that teaches
> > > me to be wary here. It you want to get really
> > > scared ask Volker about this stuff also :-).
> >
> > Yeah, FUD helps convince me. :-)
> >
> > It's ok. But cleaning up (later) the areas of code that are
> > so scaring is probably a good thing.
> >
> > > This is why I ended up re-writing the patch
> > > to be a very simple fix after *really* careful
> > > review of the possible issues (and I'm still
> > > not sure I've gotten them all yet :-).
> > >
> > > When I post my cleanup patch for you to review
> > > you can be sure I've checked out all the possibilities
> > > I can think of :-).
> >
> > Looking forward to seeing it.
> >
> > > > We might want to change that function so that the
> > > > callers do not actually have to deal with checking
> > > > the dirpath at all but encode it in a return code
> > > > or bool or so... Just rambling, sorry.
> > > >
> > > > But I will buy the benefits of getting a more
> > > > isolated patch for the specific problem first.
> > >
> > > Thanks. Already pushed mine :-).
> >
> > Er. Where is the review+? (probably not on the list)
> > My mail was more of a review- until we sort out
> > this discussion.
> >
> > > Your gluster-specific fix, whilst not wrong, isn't general enough.
> >
> > Hm. You say it's a gluster-specific bug to be fixed.
> > And you fixed it in one layer above the call to
> > the gluster specific function. So yours is too
> > general (I'd say). But mine fixed it at the gluster level
> > and you say it's not general enough. How can that
> > be? There's exactly one call between the two places.
> >
> > Please explain...
> >
> > Thanks - Michael
> >
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 163 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20180822/83b497e0/signature.sig>