* [PATCH 1/1] exportfs: Stop racing exportfs on clusters @ 2012-03-07 19:56 Steve Dickson 2012-03-07 20:07 ` Chuck Lever 2012-03-07 20:36 ` J. Bruce Fields 0 siblings, 2 replies; 9+ messages in thread From: Steve Dickson @ 2012-03-07 19:56 UTC (permalink / raw) To: Linux NFS Mailing list This problem can occur when multiple cluster services fail over at the same time, causing missing high-available exports. Having a lot of nfs-exports will trigger this issue easier. Signed-off-by: Steve Dickson <steved@redhat.com> --- support/include/exportfs.h | 4 ++++ utils/exportfs/exportfs.c | 24 ++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 0 deletions(-) diff --git a/support/include/exportfs.h b/support/include/exportfs.h index 01e87dd..99916e5 100644 --- a/support/include/exportfs.h +++ b/support/include/exportfs.h @@ -32,6 +32,10 @@ enum { FSLOC_STUB }; +#ifndef EXP_LOCKFILE +#define EXP_LOCKFILE "/var/lib/nfs/export-lock" +#endif + typedef struct mclient { struct mclient * m_next; char * m_hostname; diff --git a/utils/exportfs/exportfs.c b/utils/exportfs/exportfs.c index 7432a65..769d438 100644 --- a/utils/exportfs/exportfs.c +++ b/utils/exportfs/exportfs.c @@ -16,6 +16,7 @@ #include <sys/stat.h> #include <sys/vfs.h> #include <sys/stat.h> +#include <sys/file.h> #include <unistd.h> #include <stdbool.h> #include <stdlib.h> @@ -44,6 +45,23 @@ static void validate_export(nfs_export *exp); static int matchhostname(const char *hostname1, const char *hostname2); static void export_d_read(const char *dname); +static const char *lockfile = EXP_LOCKFILE; +static int _lockfd = -1; + +void +grab_lockfile() +{ + _lockfd = open(lockfile, O_CREAT|O_RDWR, 0666); + if (_lockfd != -1) + lockf(_lockfd, F_LOCK, 0); +} +void +release_lockfile() +{ + if (_lockfd != -1) + lockf(_lockfd, F_ULOCK, 0); +} + int main(int argc, char **argv) { @@ -129,6 +147,12 @@ main(int argc, char **argv) return 0; } } + /* + * Serialize things as best we can + */ + grab_lockfile(); + atexit(release_lockfile); + if (f_export && ! f_ignore) { export_read(_PATH_EXPORTS); export_d_read(_PATH_EXPORTS_D); -- 1.7.7.6 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] exportfs: Stop racing exportfs on clusters 2012-03-07 19:56 [PATCH 1/1] exportfs: Stop racing exportfs on clusters Steve Dickson @ 2012-03-07 20:07 ` Chuck Lever 2012-03-07 20:19 ` Steve Dickson 2012-03-07 20:36 ` J. Bruce Fields 1 sibling, 1 reply; 9+ messages in thread From: Chuck Lever @ 2012-03-07 20:07 UTC (permalink / raw) To: Steve Dickson; +Cc: Linux NFS Mailing list On Mar 7, 2012, at 2:56 PM, Steve Dickson wrote: > This problem can occur when multiple cluster services fail over > at the same time, causing missing high-available exports. > Having a lot of nfs-exports will trigger this issue easier. Does this address https://bugzilla.linux-nfs.org/show_bug.cgi?id=224 ? > Signed-off-by: Steve Dickson <steved@redhat.com> > --- > support/include/exportfs.h | 4 ++++ > utils/exportfs/exportfs.c | 24 ++++++++++++++++++++++++ > 2 files changed, 28 insertions(+), 0 deletions(-) > > diff --git a/support/include/exportfs.h b/support/include/exportfs.h > index 01e87dd..99916e5 100644 > --- a/support/include/exportfs.h > +++ b/support/include/exportfs.h > @@ -32,6 +32,10 @@ enum { > FSLOC_STUB > }; > > +#ifndef EXP_LOCKFILE > +#define EXP_LOCKFILE "/var/lib/nfs/export-lock" > +#endif > + > typedef struct mclient { > struct mclient * m_next; > char * m_hostname; > diff --git a/utils/exportfs/exportfs.c b/utils/exportfs/exportfs.c > index 7432a65..769d438 100644 > --- a/utils/exportfs/exportfs.c > +++ b/utils/exportfs/exportfs.c > @@ -16,6 +16,7 @@ > #include <sys/stat.h> > #include <sys/vfs.h> > #include <sys/stat.h> > +#include <sys/file.h> > #include <unistd.h> > #include <stdbool.h> > #include <stdlib.h> > @@ -44,6 +45,23 @@ static void validate_export(nfs_export *exp); > static int matchhostname(const char *hostname1, const char *hostname2); > static void export_d_read(const char *dname); > > +static const char *lockfile = EXP_LOCKFILE; > +static int _lockfd = -1; > + > +void > +grab_lockfile() > +{ > + _lockfd = open(lockfile, O_CREAT|O_RDWR, 0666); > + if (_lockfd != -1) > + lockf(_lockfd, F_LOCK, 0); > +} > +void > +release_lockfile() > +{ > + if (_lockfd != -1) > + lockf(_lockfd, F_ULOCK, 0); > +} > + > int > main(int argc, char **argv) > { > @@ -129,6 +147,12 @@ main(int argc, char **argv) > return 0; > } > } > + /* > + * Serialize things as best we can > + */ > + grab_lockfile(); > + atexit(release_lockfile); > + > if (f_export && ! f_ignore) { > export_read(_PATH_EXPORTS); > export_d_read(_PATH_EXPORTS_D); > -- > 1.7.7.6 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Chuck Lever chuck[dot]lever[at]oracle[dot]com ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] exportfs: Stop racing exportfs on clusters 2012-03-07 20:07 ` Chuck Lever @ 2012-03-07 20:19 ` Steve Dickson 0 siblings, 0 replies; 9+ messages in thread From: Steve Dickson @ 2012-03-07 20:19 UTC (permalink / raw) To: Chuck Lever; +Cc: Linux NFS Mailing list On 03/07/2012 03:07 PM, Chuck Lever wrote: > > On Mar 7, 2012, at 2:56 PM, Steve Dickson wrote: > >> This problem can occur when multiple cluster services fail over >> at the same time, causing missing high-available exports. >> Having a lot of nfs-exports will trigger this issue easier. > > Does this address https://bugzilla.linux-nfs.org/show_bug.cgi?id=224 ? Yes, I believe so. It also addresses https://bugzilla.redhat.com/show_bug.cgi?id=800335 I guess I make this fix back in the RHEL4 days and the patch never made into upstream. steved. > >> Signed-off-by: Steve Dickson <steved@redhat.com> >> --- >> support/include/exportfs.h | 4 ++++ >> utils/exportfs/exportfs.c | 24 ++++++++++++++++++++++++ >> 2 files changed, 28 insertions(+), 0 deletions(-) >> >> diff --git a/support/include/exportfs.h b/support/include/exportfs.h >> index 01e87dd..99916e5 100644 >> --- a/support/include/exportfs.h >> +++ b/support/include/exportfs.h >> @@ -32,6 +32,10 @@ enum { >> FSLOC_STUB >> }; >> >> +#ifndef EXP_LOCKFILE >> +#define EXP_LOCKFILE "/var/lib/nfs/export-lock" >> +#endif >> + >> typedef struct mclient { >> struct mclient * m_next; >> char * m_hostname; >> diff --git a/utils/exportfs/exportfs.c b/utils/exportfs/exportfs.c >> index 7432a65..769d438 100644 >> --- a/utils/exportfs/exportfs.c >> +++ b/utils/exportfs/exportfs.c >> @@ -16,6 +16,7 @@ >> #include <sys/stat.h> >> #include <sys/vfs.h> >> #include <sys/stat.h> >> +#include <sys/file.h> >> #include <unistd.h> >> #include <stdbool.h> >> #include <stdlib.h> >> @@ -44,6 +45,23 @@ static void validate_export(nfs_export *exp); >> static int matchhostname(const char *hostname1, const char *hostname2); >> static void export_d_read(const char *dname); >> >> +static const char *lockfile = EXP_LOCKFILE; >> +static int _lockfd = -1; >> + >> +void >> +grab_lockfile() >> +{ >> + _lockfd = open(lockfile, O_CREAT|O_RDWR, 0666); >> + if (_lockfd != -1) >> + lockf(_lockfd, F_LOCK, 0); >> +} >> +void >> +release_lockfile() >> +{ >> + if (_lockfd != -1) >> + lockf(_lockfd, F_ULOCK, 0); >> +} >> + >> int >> main(int argc, char **argv) >> { >> @@ -129,6 +147,12 @@ main(int argc, char **argv) >> return 0; >> } >> } >> + /* >> + * Serialize things as best we can >> + */ >> + grab_lockfile(); >> + atexit(release_lockfile); >> + >> if (f_export && ! f_ignore) { >> export_read(_PATH_EXPORTS); >> export_d_read(_PATH_EXPORTS_D); >> -- >> 1.7.7.6 >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] exportfs: Stop racing exportfs on clusters 2012-03-07 19:56 [PATCH 1/1] exportfs: Stop racing exportfs on clusters Steve Dickson 2012-03-07 20:07 ` Chuck Lever @ 2012-03-07 20:36 ` J. Bruce Fields 2012-03-07 20:50 ` J. Bruce Fields 2012-03-07 20:53 ` Steve Dickson 1 sibling, 2 replies; 9+ messages in thread From: J. Bruce Fields @ 2012-03-07 20:36 UTC (permalink / raw) To: Steve Dickson; +Cc: Linux NFS Mailing list On Wed, Mar 07, 2012 at 02:56:30PM -0500, Steve Dickson wrote: > This problem can occur when multiple cluster services fail over > at the same time, causing missing high-available exports. > Having a lot of nfs-exports will trigger this issue easier. Isn't the locking in support/export/xtab.c supposed to take care of this? I may be confused--I forget what all these files are for.... --b. > > Signed-off-by: Steve Dickson <steved@redhat.com> > --- > support/include/exportfs.h | 4 ++++ > utils/exportfs/exportfs.c | 24 ++++++++++++++++++++++++ > 2 files changed, 28 insertions(+), 0 deletions(-) > > diff --git a/support/include/exportfs.h b/support/include/exportfs.h > index 01e87dd..99916e5 100644 > --- a/support/include/exportfs.h > +++ b/support/include/exportfs.h > @@ -32,6 +32,10 @@ enum { > FSLOC_STUB > }; > > +#ifndef EXP_LOCKFILE > +#define EXP_LOCKFILE "/var/lib/nfs/export-lock" > +#endif > + > typedef struct mclient { > struct mclient * m_next; > char * m_hostname; > diff --git a/utils/exportfs/exportfs.c b/utils/exportfs/exportfs.c > index 7432a65..769d438 100644 > --- a/utils/exportfs/exportfs.c > +++ b/utils/exportfs/exportfs.c > @@ -16,6 +16,7 @@ > #include <sys/stat.h> > #include <sys/vfs.h> > #include <sys/stat.h> > +#include <sys/file.h> > #include <unistd.h> > #include <stdbool.h> > #include <stdlib.h> > @@ -44,6 +45,23 @@ static void validate_export(nfs_export *exp); > static int matchhostname(const char *hostname1, const char *hostname2); > static void export_d_read(const char *dname); > > +static const char *lockfile = EXP_LOCKFILE; > +static int _lockfd = -1; > + > +void > +grab_lockfile() > +{ > + _lockfd = open(lockfile, O_CREAT|O_RDWR, 0666); > + if (_lockfd != -1) > + lockf(_lockfd, F_LOCK, 0); > +} > +void > +release_lockfile() > +{ > + if (_lockfd != -1) > + lockf(_lockfd, F_ULOCK, 0); > +} > + > int > main(int argc, char **argv) > { > @@ -129,6 +147,12 @@ main(int argc, char **argv) > return 0; > } > } > + /* > + * Serialize things as best we can > + */ > + grab_lockfile(); > + atexit(release_lockfile); > + > if (f_export && ! f_ignore) { > export_read(_PATH_EXPORTS); > export_d_read(_PATH_EXPORTS_D); > -- > 1.7.7.6 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] exportfs: Stop racing exportfs on clusters 2012-03-07 20:36 ` J. Bruce Fields @ 2012-03-07 20:50 ` J. Bruce Fields 2012-03-07 20:52 ` Chuck Lever 2012-03-07 20:53 ` Steve Dickson 1 sibling, 1 reply; 9+ messages in thread From: J. Bruce Fields @ 2012-03-07 20:50 UTC (permalink / raw) To: Steve Dickson; +Cc: Linux NFS Mailing list On Wed, Mar 07, 2012 at 03:36:49PM -0500, bfields wrote: > On Wed, Mar 07, 2012 at 02:56:30PM -0500, Steve Dickson wrote: > > This problem can occur when multiple cluster services fail over > > at the same time, causing missing high-available exports. > > Having a lot of nfs-exports will trigger this issue easier. > > Isn't the locking in support/export/xtab.c supposed to take care of > this? Huh, maybe that's just meant to prevent etab from getting corrupted, or mountd from seeing a partially-written file. But it probably doesn't prevent two exportfs processes from both reading etab, adding an export, and each writing out an etab with its one export added, with the last writer winning. Was that the problem? I think it'd be worth a comment above grab/release_lockfile(). --b. > > I may be confused--I forget what all these files are for.... > > --b. > > > > > Signed-off-by: Steve Dickson <steved@redhat.com> > > --- > > support/include/exportfs.h | 4 ++++ > > utils/exportfs/exportfs.c | 24 ++++++++++++++++++++++++ > > 2 files changed, 28 insertions(+), 0 deletions(-) > > > > diff --git a/support/include/exportfs.h b/support/include/exportfs.h > > index 01e87dd..99916e5 100644 > > --- a/support/include/exportfs.h > > +++ b/support/include/exportfs.h > > @@ -32,6 +32,10 @@ enum { > > FSLOC_STUB > > }; > > > > +#ifndef EXP_LOCKFILE > > +#define EXP_LOCKFILE "/var/lib/nfs/export-lock" > > +#endif > > + > > typedef struct mclient { > > struct mclient * m_next; > > char * m_hostname; > > diff --git a/utils/exportfs/exportfs.c b/utils/exportfs/exportfs.c > > index 7432a65..769d438 100644 > > --- a/utils/exportfs/exportfs.c > > +++ b/utils/exportfs/exportfs.c > > @@ -16,6 +16,7 @@ > > #include <sys/stat.h> > > #include <sys/vfs.h> > > #include <sys/stat.h> > > +#include <sys/file.h> > > #include <unistd.h> > > #include <stdbool.h> > > #include <stdlib.h> > > @@ -44,6 +45,23 @@ static void validate_export(nfs_export *exp); > > static int matchhostname(const char *hostname1, const char *hostname2); > > static void export_d_read(const char *dname); > > > > +static const char *lockfile = EXP_LOCKFILE; > > +static int _lockfd = -1; > > + > > +void > > +grab_lockfile() > > +{ > > + _lockfd = open(lockfile, O_CREAT|O_RDWR, 0666); > > + if (_lockfd != -1) > > + lockf(_lockfd, F_LOCK, 0); > > +} > > +void > > +release_lockfile() > > +{ > > + if (_lockfd != -1) > > + lockf(_lockfd, F_ULOCK, 0); > > +} > > + > > int > > main(int argc, char **argv) > > { > > @@ -129,6 +147,12 @@ main(int argc, char **argv) > > return 0; > > } > > } > > + /* > > + * Serialize things as best we can > > + */ > > + grab_lockfile(); > > + atexit(release_lockfile); > > + > > if (f_export && ! f_ignore) { > > export_read(_PATH_EXPORTS); > > export_d_read(_PATH_EXPORTS_D); > > -- > > 1.7.7.6 > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] exportfs: Stop racing exportfs on clusters 2012-03-07 20:50 ` J. Bruce Fields @ 2012-03-07 20:52 ` Chuck Lever 2012-03-08 15:11 ` Steve Dickson 0 siblings, 1 reply; 9+ messages in thread From: Chuck Lever @ 2012-03-07 20:52 UTC (permalink / raw) To: J. Bruce Fields; +Cc: Steve Dickson, Linux NFS Mailing list On Mar 7, 2012, at 3:50 PM, J. Bruce Fields wrote: > On Wed, Mar 07, 2012 at 03:36:49PM -0500, bfields wrote: >> On Wed, Mar 07, 2012 at 02:56:30PM -0500, Steve Dickson wrote: >>> This problem can occur when multiple cluster services fail over >>> at the same time, causing missing high-available exports. >>> Having a lot of nfs-exports will trigger this issue easier. >> >> Isn't the locking in support/export/xtab.c supposed to take care of >> this? > > Huh, maybe that's just meant to prevent etab from getting corrupted, or > mountd from seeing a partially-written file. > > But it probably doesn't prevent two exportfs processes from both reading > etab, adding an export, and each writing out an etab with its one export > added, with the last writer winning. > > Was that the problem? For bug 224, yes. After disparate concurrent exportfs commands race, the etab file ends up missing some exports. > > I think it'd be worth a comment above grab/release_lockfile(). > > --b. > >> >> I may be confused--I forget what all these files are for.... >> >> --b. >> >>> >>> Signed-off-by: Steve Dickson <steved@redhat.com> >>> --- >>> support/include/exportfs.h | 4 ++++ >>> utils/exportfs/exportfs.c | 24 ++++++++++++++++++++++++ >>> 2 files changed, 28 insertions(+), 0 deletions(-) >>> >>> diff --git a/support/include/exportfs.h b/support/include/exportfs.h >>> index 01e87dd..99916e5 100644 >>> --- a/support/include/exportfs.h >>> +++ b/support/include/exportfs.h >>> @@ -32,6 +32,10 @@ enum { >>> FSLOC_STUB >>> }; >>> >>> +#ifndef EXP_LOCKFILE >>> +#define EXP_LOCKFILE "/var/lib/nfs/export-lock" >>> +#endif >>> + >>> typedef struct mclient { >>> struct mclient * m_next; >>> char * m_hostname; >>> diff --git a/utils/exportfs/exportfs.c b/utils/exportfs/exportfs.c >>> index 7432a65..769d438 100644 >>> --- a/utils/exportfs/exportfs.c >>> +++ b/utils/exportfs/exportfs.c >>> @@ -16,6 +16,7 @@ >>> #include <sys/stat.h> >>> #include <sys/vfs.h> >>> #include <sys/stat.h> >>> +#include <sys/file.h> >>> #include <unistd.h> >>> #include <stdbool.h> >>> #include <stdlib.h> >>> @@ -44,6 +45,23 @@ static void validate_export(nfs_export *exp); >>> static int matchhostname(const char *hostname1, const char *hostname2); >>> static void export_d_read(const char *dname); >>> >>> +static const char *lockfile = EXP_LOCKFILE; >>> +static int _lockfd = -1; >>> + >>> +void >>> +grab_lockfile() >>> +{ >>> + _lockfd = open(lockfile, O_CREAT|O_RDWR, 0666); >>> + if (_lockfd != -1) >>> + lockf(_lockfd, F_LOCK, 0); >>> +} >>> +void >>> +release_lockfile() >>> +{ >>> + if (_lockfd != -1) >>> + lockf(_lockfd, F_ULOCK, 0); >>> +} >>> + >>> int >>> main(int argc, char **argv) >>> { >>> @@ -129,6 +147,12 @@ main(int argc, char **argv) >>> return 0; >>> } >>> } >>> + /* >>> + * Serialize things as best we can >>> + */ >>> + grab_lockfile(); >>> + atexit(release_lockfile); >>> + >>> if (f_export && ! f_ignore) { >>> export_read(_PATH_EXPORTS); >>> export_d_read(_PATH_EXPORTS_D); >>> -- >>> 1.7.7.6 >>> >>> -- >>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in >>> the body of a message to majordomo@vger.kernel.org >>> More majordomo info at http://vger.kernel.org/majordomo-info.html > -- > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Chuck Lever chuck[dot]lever[at]oracle[dot]com ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] exportfs: Stop racing exportfs on clusters 2012-03-07 20:52 ` Chuck Lever @ 2012-03-08 15:11 ` Steve Dickson 2012-03-08 18:53 ` J. Bruce Fields 0 siblings, 1 reply; 9+ messages in thread From: Steve Dickson @ 2012-03-08 15:11 UTC (permalink / raw) To: Chuck Lever; +Cc: J. Bruce Fields, Linux NFS Mailing list On 03/07/2012 03:52 PM, Chuck Lever wrote: > > On Mar 7, 2012, at 3:50 PM, J. Bruce Fields wrote: > >> On Wed, Mar 07, 2012 at 03:36:49PM -0500, bfields wrote: >>> On Wed, Mar 07, 2012 at 02:56:30PM -0500, Steve Dickson wrote: >>>> This problem can occur when multiple cluster services fail over >>>> at the same time, causing missing high-available exports. >>>> Having a lot of nfs-exports will trigger this issue easier. >>> >>> Isn't the locking in support/export/xtab.c supposed to take care of >>> this? >> >> Huh, maybe that's just meant to prevent etab from getting corrupted, or >> mountd from seeing a partially-written file. >> >> But it probably doesn't prevent two exportfs processes from both reading >> etab, adding an export, and each writing out an etab with its one export >> added, with the last writer winning. >> >> Was that the problem? > > For bug 224, yes. After disparate concurrent exportfs commands race, the etab file ends up missing some exports. This morning I took another look at this and there does indeed need to be some global lock to stop to serialize dueling exportfs commands. The locks in xtab.c are just to granular.... steved. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] exportfs: Stop racing exportfs on clusters 2012-03-08 15:11 ` Steve Dickson @ 2012-03-08 18:53 ` J. Bruce Fields 0 siblings, 0 replies; 9+ messages in thread From: J. Bruce Fields @ 2012-03-08 18:53 UTC (permalink / raw) To: Steve Dickson; +Cc: Chuck Lever, Linux NFS Mailing list On Thu, Mar 08, 2012 at 10:11:26AM -0500, Steve Dickson wrote: > > > On 03/07/2012 03:52 PM, Chuck Lever wrote: > > > > On Mar 7, 2012, at 3:50 PM, J. Bruce Fields wrote: > > > >> On Wed, Mar 07, 2012 at 03:36:49PM -0500, bfields wrote: > >>> On Wed, Mar 07, 2012 at 02:56:30PM -0500, Steve Dickson wrote: > >>>> This problem can occur when multiple cluster services fail over > >>>> at the same time, causing missing high-available exports. > >>>> Having a lot of nfs-exports will trigger this issue easier. > >>> > >>> Isn't the locking in support/export/xtab.c supposed to take care of > >>> this? > >> > >> Huh, maybe that's just meant to prevent etab from getting corrupted, or > >> mountd from seeing a partially-written file. > >> > >> But it probably doesn't prevent two exportfs processes from both reading > >> etab, adding an export, and each writing out an etab with its one export > >> added, with the last writer winning. > >> > >> Was that the problem? > > > > For bug 224, yes. After disparate concurrent exportfs commands race, the etab file ends up missing some exports. > This morning I took another look at this and there does indeed need to > be some global lock to stop to serialize dueling exportfs commands. The > locks in xtab.c are just to granular.... Right, makes sense. So just in hopes that I don't ask the same question next time this comes up, could we find a good spot for a comment, maybe something like this?: /* * If we aren't careful, changes made by exportfs can be lost * when multiple exports process run at once: * * exportfs process 1 exportfs process 2 * ------------------------------------------ * reads etab version A reads etab version A * adds new export B adds new export C * writes A+B writes A+C * The locking in support/export/xtab.c will prevent mountd from * seeing a partially written version of etab, and will prevent * the two writers above from writing simultaneously and * corrupting etab, but to prevent problems like the above we * need this additional lock: */ Uh, or maybe you can come up with something shorter. --b. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] exportfs: Stop racing exportfs on clusters 2012-03-07 20:36 ` J. Bruce Fields 2012-03-07 20:50 ` J. Bruce Fields @ 2012-03-07 20:53 ` Steve Dickson 1 sibling, 0 replies; 9+ messages in thread From: Steve Dickson @ 2012-03-07 20:53 UTC (permalink / raw) To: J. Bruce Fields; +Cc: Linux NFS Mailing list On 03/07/2012 03:36 PM, J. Bruce Fields wrote: > On Wed, Mar 07, 2012 at 02:56:30PM -0500, Steve Dickson wrote: >> This problem can occur when multiple cluster services fail over >> at the same time, causing missing high-available exports. >> Having a lot of nfs-exports will trigger this issue easier. > > Isn't the locking in support/export/xtab.c supposed to take care of > this? > > I may be confused--I forget what all these files are for.... hmm... your right... those do take out flock()s during I/O.. There must be a hole somewhere they can get corrupted... steved. > > --b. > >> >> Signed-off-by: Steve Dickson <steved@redhat.com> >> --- >> support/include/exportfs.h | 4 ++++ >> utils/exportfs/exportfs.c | 24 ++++++++++++++++++++++++ >> 2 files changed, 28 insertions(+), 0 deletions(-) >> >> diff --git a/support/include/exportfs.h b/support/include/exportfs.h >> index 01e87dd..99916e5 100644 >> --- a/support/include/exportfs.h >> +++ b/support/include/exportfs.h >> @@ -32,6 +32,10 @@ enum { >> FSLOC_STUB >> }; >> >> +#ifndef EXP_LOCKFILE >> +#define EXP_LOCKFILE "/var/lib/nfs/export-lock" >> +#endif >> + >> typedef struct mclient { >> struct mclient * m_next; >> char * m_hostname; >> diff --git a/utils/exportfs/exportfs.c b/utils/exportfs/exportfs.c >> index 7432a65..769d438 100644 >> --- a/utils/exportfs/exportfs.c >> +++ b/utils/exportfs/exportfs.c >> @@ -16,6 +16,7 @@ >> #include <sys/stat.h> >> #include <sys/vfs.h> >> #include <sys/stat.h> >> +#include <sys/file.h> >> #include <unistd.h> >> #include <stdbool.h> >> #include <stdlib.h> >> @@ -44,6 +45,23 @@ static void validate_export(nfs_export *exp); >> static int matchhostname(const char *hostname1, const char *hostname2); >> static void export_d_read(const char *dname); >> >> +static const char *lockfile = EXP_LOCKFILE; >> +static int _lockfd = -1; >> + >> +void >> +grab_lockfile() >> +{ >> + _lockfd = open(lockfile, O_CREAT|O_RDWR, 0666); >> + if (_lockfd != -1) >> + lockf(_lockfd, F_LOCK, 0); >> +} >> +void >> +release_lockfile() >> +{ >> + if (_lockfd != -1) >> + lockf(_lockfd, F_ULOCK, 0); >> +} >> + >> int >> main(int argc, char **argv) >> { >> @@ -129,6 +147,12 @@ main(int argc, char **argv) >> return 0; >> } >> } >> + /* >> + * Serialize things as best we can >> + */ >> + grab_lockfile(); >> + atexit(release_lockfile); >> + >> if (f_export && ! f_ignore) { >> export_read(_PATH_EXPORTS); >> export_d_read(_PATH_EXPORTS_D); >> -- >> 1.7.7.6 >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2012-03-08 18:53 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-03-07 19:56 [PATCH 1/1] exportfs: Stop racing exportfs on clusters Steve Dickson 2012-03-07 20:07 ` Chuck Lever 2012-03-07 20:19 ` Steve Dickson 2012-03-07 20:36 ` J. Bruce Fields 2012-03-07 20:50 ` J. Bruce Fields 2012-03-07 20:52 ` Chuck Lever 2012-03-08 15:11 ` Steve Dickson 2012-03-08 18:53 ` J. Bruce Fields 2012-03-07 20:53 ` Steve Dickson
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.