[PATCHES] fix flapping offline flag in gpfs module

[Christof Schmitt]

On Fri, Jul 11, 2014 at 12:24:36AM +0200, Michael Adam wrote:
> On 2014-07-10 at 13:55 -0700, Christof Schmitt wrote:
> > Hi Michael,
> > 
> > On Thu, Jul 10, 2014 at 12:38:43AM +0200, Michael Adam wrote:
> > > There is this awful problem of the flapping offline flag
> > > when using the gpfs module and "gpfs:winattr = yes".
> > > It was pretty clear since a time that this must be due
> > > to uninitialized data (where the winattrs are stored),
> > > i.e. the vfs_private in the struct stat_ex.
> > > 
> > > I was recently able to debug this and found two causes
> > > of uninitialized data:
> > > 
> > > - One in vfs_gpfs_fstat, where it was imply forgotten to copy the
> > >   result of smbd_fget_gpfs_winattrs().
> > >   This one is present in the code since Samba 3.6 and easily fixed.
> > > 
> > > - The other is much more subtle:
> > >   in directory enumeration, commit
> > >   2a65e8befef004fd18d17853a1b72155752346c8 introduced an
> > >   optimization which made vfswrap_readdir() call
> > >   fstatat directly if available, marking the stat buffer valid.
> > >   The problem with this is that this does not go through vfs
> > >   and hence, (e.g.) the gpfs vfs module has no chance to
> > >   fill the vfs_private with valid info.
> > >   Hence the is_offline() function can not rely on the
> > >   vfs_private info if there is a valid stat buffer.
> > > 
> > >   - The fix attached simply ignores the stat buffer in
> > >     is_offline() an always calls get_gpfs_winattrs.
> > >     thereby at least partly sacrificing the optimization.
> > > 
> > >   - A proper fix might add fstatat to the vfs and
> > >     have vfs_gpfs implement it.
> > >     Should we do that? or leave it at the above fix?
> > >     I think we need the above fix in any case, since
> > >     we will want to port it to 4.1.
> > 
> > Yes, i agree that we need to fix this issue first, and then figure out
> > how to improve the code.
> > 
> > >   - I also added a patch to log a message when gpfs_is_offline()
> > >     detects a potential case of uninitialized buffer.
> > >     Maybe debug level 0 is too low?
> > 
> > Debug level 0 should be good. It should never happen, but if it does, we
> > want to know about it.
> > 
> > >   - finally, I added a patch that initializes the
> > >     stat buffer to 0 in directory enumeration, so
> > >     that after fstatat, removing the randomness in
> > >     failure of vfs_private to be filled by the vfs
> > >     modules.
> > > 
> > > Comments / reviews / push appreciated.
> > > 
> > > Thanks - Michael
> > > 
> > > PS: This is a very long mail, and very long commit messages
> > > for patches with very small diff, but I think this is important.  ;-)
> > 
> > Reviewed-by: Christof Schmitt <cs at samba.org>
> 
> Thanks, gonna push tomorrow if noone else did.

I just pushed them to autobuild.

Christof