[PATCHES] fix flapping offline flag in gpfs module
Christof Schmitt
cs at samba.org
Fri Jul 11 15:03:31 MDT 2014
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
More information about the samba-technical
mailing list