[PATCHES] fix flapping offline flag in gpfs module
obnox at samba.org
Thu Jul 10 16:24:36 MDT 2014
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.
Cheers - Michael
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 198 bytes
Desc: Digital signature
More information about the samba-technical