On 10/19/2011 03:55 PM, J. Bruce Fields wrote: > On Wed, Oct 19, 2011 at 02:38:49PM -0400, Steve Dickson wrote: >> >> >> On 10/19/2011 01:36 PM, J. Bruce Fields wrote: >>> On Wed, Oct 19, 2011 at 01:30:58PM -0400, Steve Dickson wrote: >>>> >>>> >>>> On 10/19/2011 01:22 PM, Jeff Layton wrote: >>>>> On Wed, 19 Oct 2011 13:10:19 -0400 >>>>> Steve Dickson <SteveD@xxxxxxxxxx> wrote: >>>>> >>>>>> >>>>>> >>>>>> On 10/19/2011 12:36 PM, Jeff Layton wrote: >>>>>>> On Wed, 19 Oct 2011 11:34:30 -0400 >>>>>>> Steve Dickson <steved@xxxxxxxxxx> wrote: >>>>>>> >>>>>>>> This patch is a following on to commit 7a802337. Using the >>>>>>>> tool in https://bugzilla.redhat.com/show_bug.cgi?id=695916 >>>>>>>> caused the fflush() and fclose() to fail in turn causing >>>>>>>> corruption in the mtab. >>>>>>>> >>>>>>>> The failures were in the internals of both calls. Switch those >>>>>>>> calls with the actual system calls eliminated the failures. >>>>>>>> >>>>>>>> Signed-off-by: Steve Dickson <steved@xxxxxxxxxx> >>>>>>>> --- >>>>>>>> support/nfs/nfs_mntent.c | 4 ++-- >>>>>>>> 1 files changed, 2 insertions(+), 2 deletions(-) >>>>>>>> >>>>>>>> diff --git a/support/nfs/nfs_mntent.c b/support/nfs/nfs_mntent.c >>>>>>>> index a2118a2..b80f270 100644 >>>>>>>> --- a/support/nfs/nfs_mntent.c >>>>>>>> +++ b/support/nfs/nfs_mntent.c >>>>>>>> @@ -117,7 +117,7 @@ void >>>>>>>> nfs_endmntent (mntFILE *mfp) { >>>>>>>> if (mfp) { >>>>>>>> if (mfp->mntent_fp) >>>>>>>> - fclose(mfp->mntent_fp); >>>>>>>> + close(fileno(mfp->mntent_fp)); >>>>>>>> if (mfp->mntent_file) >>>>>>>> free(mfp->mntent_file); >>>>>>>> free(mfp); >>>>>>>> @@ -147,7 +147,7 @@ nfs_addmntent (mntFILE *mfp, struct mntent *mnt) { >>>>>>>> free(m3); >>>>>>>> free(m4); >>>>>>>> if (res >= 0) { >>>>>>>> - res = fflush(mfp->mntent_fp); >>>>>>>> + res = fsync(fileno(mfp->mntent_fp)); >>>>>>> >>>>>>> fsync doesn't imply an fflush. With this, I think you may end up >>>>>>> without everything being committed to disk if part or all of it is >>>>>>> still in the file stream buffer. You probably want to do an fflush() >>>>>>> and then an fsync here. >>>>>> The problem was with the fflush() call. The call was causing the >>>>>> mount to drop core in turn causing mtab corruption. Changing that >>>>>> call to a fsync() worked just fine... no corruption... every time! >>>>>> >>>>> >>>>> Ahh, then you have another problem here too then. Most likely it was >>>>> crashing because it caught a SIGXFSZ. Writing out the mtab should not >>>>> be affected by signals. >>>> So calling fflush() generates a SIGXFSZ and call fsync() does not... >>> >>> fflush() must hit this because it's calling write() to write out the >>> stream buffer.... >>> >>> But lock_mtab() should have set SIGXFSZ to be ignored; is that not >>> happening? >> This is the case. The following patch cause SIGXFSZ to be ignored. >> >> diff -up ./utils/mount/fstab.c.orig ./utils/mount/fstab.c >> --- ./utils/mount/fstab.c.orig 2011-10-19 13:28:57.318132000 -0400 >> +++ ./utils/mount/fstab.c 2011-10-19 14:02:07.715039000 -0400 >> @@ -387,8 +387,9 @@ lock_mtab (void) { >> sa.sa_flags = 0; >> sigfillset (&sa.sa_mask); >> >> - while (sigismember (&sa.sa_mask, ++sig) != -1 >> - && sig != SIGCHLD) { > > I'm very confused--the latest nfs-utils doesn't have this. > > Oh, I see: it was fixed by b3e190c4adfc9ec47567c968bd000d282d07b05e > "mount: improve signal management when locking mtab". > > So if you're working with an older version then you probably want to > backport that patch. Exactly... Read my last posting... steved. > > --b. > >> + while (sigismember (&sa.sa_mask, ++sig) != -1) { >> + if (sig == SIGCHLD) >> + continue; >> if (sig == SIGALRM) >> sa.sa_handler = setlkw_timeout; >> else >> >> Now all this does is cause mount to ignore the fact that >> the write() during the fflush() fails, which causes the >> following warning during the umount. >> >> [mntent]: warning: no final newline at the end of /etc/mtab >> [mntent]: warning: no final newline at the end of /etc/mtab >> [mntent]: warning: no final newline at the end of /etc/mtab >> >> I can live with those warnings.... >> >> steved. >> >>> >>>> I really don't see what the problem is is call simply calling fsync() >>>> which clearly works? >>> >>> We want to make sure the problem's really fixed. >>> >>> --b. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
