[PATCHES] fix flapping offline flag in gpfs module

[Michael Adam]

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...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20140711/2430bd7b/attachment.pgp>