Re: nmount, mountd drops high order MNT_xx flags during a mount update
David Boyd wrote: > On Sun, 2015-05-10 at 09:07 -0400, Rick Macklem wrote: > > Oops, I did my usual and forgot to attach the patch. > > > > Here it is, rick > > > > - Original Message - > > > Doug Rabson wrote: > > > > You could add a single integer-valued vfsopt which holds the > > > > high-order > > > > bits of f_flags? > > > > > > > I have created a patch that does this. It is on > > > https://reviews.freebsd.org/D2506 > > > (I have also attached it here, for those who don't use > > > Phabricator.) > > > > > > Please review/comment on this. (Feel free to add yourself as a > > > reviewer, etc.) > > > > > > Also, David, if you can test this patch, it would be appreciated. > > > > > > Thanks for the suggestion Doug, rick > > > > > > > On 7 May 2015 at 02:10, Rick Macklem > > > > wrote: > > > > > > > > > David Boyd reported a problem to freebsd-current@ w.r.t. the > > > > > MNT_AUTOMOUNTED > > > > > flag getting cleared by mountd. > > > > > http://docs.FreeBSD.org/cgi/mid.cgi?1429477202.29812.38.camel > > > > > I just took a look at this and it's kinda ugly... > > > > > > > > > > mountd acquires a list of mounts via getmntinfo() and then > > > > > does > > > > > an > > > > > nmount() for each of them to get rid of any exports. It uses > > > > > f_flags from the statfs returned by getmntinfo() as the 3rd > > > > > argument to nmount(). > > > > > --> Since nmount()s 3rd argument is a "int", it silently > > > > > drops > > > > > any > > > > > MNT_xx flag in the high order 32bits of f_flags. At this > > > > > time, > > > > > it happens that MNT_AUTOMOUNTED is the only one, but... > > > > > > > > > > I can think of a couple of ways to fix this, but I don't like > > > > > any > > > > > of > > > > > them;-) > > > > > > > > > > 1) I suppose mountd could check for each flag set in f_flags > > > > > and > > > > > generate > > > > > a vfsopts string for it, but that means that every time a new > > > > > flag > > > > > that > > > > > can be updated is added to MNT_xx, this mountd.c code would > > > > > have > > > > > to > > > > > updated. > > > > > --> Ugly!! > > > > > > > > > > 2) Require that all flags in MNT_UPDATEMASK be in the low > > > > > order > > > > > 32bits. > > > > >I wouldn`t mind this except in means that the MNT_xx flags > > > > >must > > > > >be > > > > > redefined > > > > >and I don`t think that could get MFC`d. > > > > > > > > > > 3) Add a new newernmount(2) which has a 64bit flags argument > > > > > instead of > > > > > int. > > > > > > > > > > Hopefully someone can think of a nice way to fix this, rick > > > > > ___ > > > > > freebsd-current@freebsd.org mailing list > > > > > http://lists.freebsd.org/mailman/listinfo/freebsd-current > > > > > To unsubscribe, send any mail to > > > > > "freebsd-current-unsubscr...@freebsd.org" > > > > > > > > > ___ > > > > freebsd-current@freebsd.org mailing list > > > > http://lists.freebsd.org/mailman/listinfo/freebsd-current > > > > To unsubscribe, send any mail to > > > > "freebsd-current-unsubscr...@freebsd.org" > > > > > > > ___ > > > freebsd-current@freebsd.org mailing list > > > http://lists.freebsd.org/mailman/listinfo/freebsd-current > > > To unsubscribe, send any mail to > > > "freebsd-current-unsubscr...@freebsd.org" > > > > > ___ > > freebsd-current@freebsd.org mailing list > > http://lists.freebsd.org/mailman/listinfo/freebsd-current > > To unsubscribe, send any mail to > > "freebsd-current-unsubscr...@freebsd.org" > > Rick, Edward, et al, > > I have applied your patches to several (four) servers and have not > noticed any more instances of the errant behavior. > Thanks for testing it and confirming that the problem is what I thought it was. > I think (hope) that this is the correct fix and wonder if this fix > might > make it's way into an errata. > Well, at this point, it isn't obvious if it should even go in head/current. (See https://reviews.freebsd.org/D2506) The patch already in head/current (r281699) should fix your problem. (It stops mountd from updating non-exported mounts.) It has not been MFC'd, but I will look into that. I don't know if it could also be an errata. Since it now appears to be a fix for an observed bug and not just an optimization, maybe? rick > Thanks for the quick responses. > > David Boyd. > > ___ > freebsd-current@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-current > To unsubscribe, send any mail to > "freebsd-current-unsubscr...@freebsd.org" > ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: nmount, mountd drops high order MNT_xx flags during a mount update
On Sun, 2015-05-10 at 09:07 -0400, Rick Macklem wrote: > Oops, I did my usual and forgot to attach the patch. > > Here it is, rick > > - Original Message - > > Doug Rabson wrote: > > > You could add a single integer-valued vfsopt which holds the > > > high-order > > > bits of f_flags? > > > > > I have created a patch that does this. It is on > > https://reviews.freebsd.org/D2506 > > (I have also attached it here, for those who don't use Phabricator.) > > > > Please review/comment on this. (Feel free to add yourself as a > > reviewer, etc.) > > > > Also, David, if you can test this patch, it would be appreciated. > > > > Thanks for the suggestion Doug, rick > > > > > On 7 May 2015 at 02:10, Rick Macklem wrote: > > > > > > > David Boyd reported a problem to freebsd-current@ w.r.t. the > > > > MNT_AUTOMOUNTED > > > > flag getting cleared by mountd. > > > > http://docs.FreeBSD.org/cgi/mid.cgi?1429477202.29812.38.camel > > > > I just took a look at this and it's kinda ugly... > > > > > > > > mountd acquires a list of mounts via getmntinfo() and then does > > > > an > > > > nmount() for each of them to get rid of any exports. It uses > > > > f_flags from the statfs returned by getmntinfo() as the 3rd > > > > argument to nmount(). > > > > --> Since nmount()s 3rd argument is a "int", it silently drops > > > > any > > > > MNT_xx flag in the high order 32bits of f_flags. At this > > > > time, > > > > it happens that MNT_AUTOMOUNTED is the only one, but... > > > > > > > > I can think of a couple of ways to fix this, but I don't like any > > > > of > > > > them;-) > > > > > > > > 1) I suppose mountd could check for each flag set in f_flags and > > > > generate > > > > a vfsopts string for it, but that means that every time a new > > > > flag > > > > that > > > > can be updated is added to MNT_xx, this mountd.c code would have > > > > to > > > > updated. > > > > --> Ugly!! > > > > > > > > 2) Require that all flags in MNT_UPDATEMASK be in the low order > > > > 32bits. > > > >I wouldn`t mind this except in means that the MNT_xx flags > > > >must > > > >be > > > > redefined > > > >and I don`t think that could get MFC`d. > > > > > > > > 3) Add a new newernmount(2) which has a 64bit flags argument > > > > instead of > > > > int. > > > > > > > > Hopefully someone can think of a nice way to fix this, rick > > > > ___ > > > > freebsd-current@freebsd.org mailing list > > > > http://lists.freebsd.org/mailman/listinfo/freebsd-current > > > > To unsubscribe, send any mail to > > > > "freebsd-current-unsubscr...@freebsd.org" > > > > > > > ___ > > > freebsd-current@freebsd.org mailing list > > > http://lists.freebsd.org/mailman/listinfo/freebsd-current > > > To unsubscribe, send any mail to > > > "freebsd-current-unsubscr...@freebsd.org" > > > > > ___ > > freebsd-current@freebsd.org mailing list > > http://lists.freebsd.org/mailman/listinfo/freebsd-current > > To unsubscribe, send any mail to > > "freebsd-current-unsubscr...@freebsd.org" > > > ___ > freebsd-current@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-current > To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org" Rick, Edward, et al, I have applied your patches to several (four) servers and have not noticed any more instances of the errant behavior. I think (hope) that this is the correct fix and wonder if this fix might make it's way into an errata. Thanks for the quick responses. David Boyd. ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: nmount, mountd drops high order MNT_xx flags during a mount update
Oops, I did my usual and forgot to attach the patch.
Here it is, rick
- Original Message -
> Doug Rabson wrote:
> > You could add a single integer-valued vfsopt which holds the
> > high-order
> > bits of f_flags?
> >
> I have created a patch that does this. It is on
> https://reviews.freebsd.org/D2506
> (I have also attached it here, for those who don't use Phabricator.)
>
> Please review/comment on this. (Feel free to add yourself as a
> reviewer, etc.)
>
> Also, David, if you can test this patch, it would be appreciated.
>
> Thanks for the suggestion Doug, rick
>
> > On 7 May 2015 at 02:10, Rick Macklem wrote:
> >
> > > David Boyd reported a problem to freebsd-current@ w.r.t. the
> > > MNT_AUTOMOUNTED
> > > flag getting cleared by mountd.
> > > http://docs.FreeBSD.org/cgi/mid.cgi?1429477202.29812.38.camel
> > > I just took a look at this and it's kinda ugly...
> > >
> > > mountd acquires a list of mounts via getmntinfo() and then does
> > > an
> > > nmount() for each of them to get rid of any exports. It uses
> > > f_flags from the statfs returned by getmntinfo() as the 3rd
> > > argument to nmount().
> > > --> Since nmount()s 3rd argument is a "int", it silently drops
> > > any
> > > MNT_xx flag in the high order 32bits of f_flags. At this
> > > time,
> > > it happens that MNT_AUTOMOUNTED is the only one, but...
> > >
> > > I can think of a couple of ways to fix this, but I don't like any
> > > of
> > > them;-)
> > >
> > > 1) I suppose mountd could check for each flag set in f_flags and
> > > generate
> > > a vfsopts string for it, but that means that every time a new
> > > flag
> > > that
> > > can be updated is added to MNT_xx, this mountd.c code would have
> > > to
> > > updated.
> > > --> Ugly!!
> > >
> > > 2) Require that all flags in MNT_UPDATEMASK be in the low order
> > > 32bits.
> > >I wouldn`t mind this except in means that the MNT_xx flags
> > >must
> > >be
> > > redefined
> > >and I don`t think that could get MFC`d.
> > >
> > > 3) Add a new newernmount(2) which has a 64bit flags argument
> > > instead of
> > > int.
> > >
> > > Hopefully someone can think of a nice way to fix this, rick
> > > ___
> > > freebsd-current@freebsd.org mailing list
> > > http://lists.freebsd.org/mailman/listinfo/freebsd-current
> > > To unsubscribe, send any mail to
> > > "freebsd-current-unsubscr...@freebsd.org"
> > >
> > ___
> > freebsd-current@freebsd.org mailing list
> > http://lists.freebsd.org/mailman/listinfo/freebsd-current
> > To unsubscribe, send any mail to
> > "freebsd-current-unsubscr...@freebsd.org"
> >
> ___
> freebsd-current@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-current
> To unsubscribe, send any mail to
> "freebsd-current-unsubscr...@freebsd.org"
>
--- kern/vfs_mount.c.savmnt 2015-05-09 19:30:14.0 -0400
+++ kern/vfs_mount.c 2015-05-10 08:34:45.0 -0400
@@ -537,6 +537,7 @@ vfs_donmount(struct thread *td, uint64_t
struct vfsopt *opt, *tmp_opt;
char *fstype, *fspath, *errmsg;
int error, fstypelen, fspathlen, errmsg_len, errmsg_pos;
+ uint64_t fsflag_high;
errmsg = fspath = NULL;
errmsg_len = fspathlen = 0;
@@ -653,6 +654,20 @@ vfs_donmount(struct thread *td, uint64_t
fsflags |= MNT_AUTOMOUNTED;
vfs_freeopt(optlist, opt);
}
+ else if (strcmp(opt->name, "fsflaghigh") == 0) {
+ /*
+ * This option allows a process doing a mount update
+ * to set all the high order 32bits of mnt_flag with
+ * one option. This is needed, since nmount(2)
+ * accepts an "int" option and, therefore, only takes
+ * the low order 32bits.
+ */
+ if (vfs_copyopt(optlist, "fsflaghigh",
+ &fsflag_high, sizeof(fsflag_high)) == 0 &&
+ (fsflag_high & 0xULL) == 0)
+fsflags |= fsflag_high;
+ vfs_freeopt(optlist, opt);
+ }
}
/*
--- usr.sbin/mountd/mountd.c.orig 2015-05-09 22:04:15.0 -0400
+++ usr.sbin/mountd/mountd.c 2015-05-09 22:27:06.0 -0400
@@ -1649,6 +1649,7 @@ get_exportlist(void)
int iovlen;
int done;
struct nfsex_args eargs;
+ uint64_t fsflag_high;
if (suspend_nfsd != 0)
(void)nfssvc(NFSSVC_SUSPENDNFSD, NULL);
@@ -1706,6 +1707,8 @@ get_exportlist(void)
build_iovec(&iov, &iovlen, "update", NULL, 0);
build_iovec(&iov, &iovlen, "export", &export, sizeof(export));
build_iovec(&iov, &iovlen, "errmsg", errmsg, sizeof(errmsg));
+ build_iovec(&iov, &iovlen, "fsflaghigh", &fsflag_high,
+ sizeof(fsflag_high));
}
for (i = 0; i < num; i++) {
@@ -1736,6 +1739,9 @@ get_exportlist(void)
iov[3].iov_len = strlen(fsp->f_mntonname) + 1;
iov[5].iov_base = fsp->f_mntfromname;
iov[5].iov_len = strlen(fsp->f_mntfromname) + 1;
+ /* Use "fsflaghigh" to set the high order bits of f_flags. */
+ fsflag_high = (fsp->f_flags & 0xULL);
+
errmsg[0] = '\0';
/*
@@ -2394
Re: nmount, mountd drops high order MNT_xx flags during a mount update
Doug Rabson wrote: > You could add a single integer-valued vfsopt which holds the > high-order > bits of f_flags? > I have created a patch that does this. It is on https://reviews.freebsd.org/D2506 (I have also attached it here, for those who don't use Phabricator.) Please review/comment on this. (Feel free to add yourself as a reviewer, etc.) Also, David, if you can test this patch, it would be appreciated. Thanks for the suggestion Doug, rick > On 7 May 2015 at 02:10, Rick Macklem wrote: > > > David Boyd reported a problem to freebsd-current@ w.r.t. the > > MNT_AUTOMOUNTED > > flag getting cleared by mountd. > > http://docs.FreeBSD.org/cgi/mid.cgi?1429477202.29812.38.camel > > I just took a look at this and it's kinda ugly... > > > > mountd acquires a list of mounts via getmntinfo() and then does an > > nmount() for each of them to get rid of any exports. It uses > > f_flags from the statfs returned by getmntinfo() as the 3rd > > argument to nmount(). > > --> Since nmount()s 3rd argument is a "int", it silently drops any > > MNT_xx flag in the high order 32bits of f_flags. At this time, > > it happens that MNT_AUTOMOUNTED is the only one, but... > > > > I can think of a couple of ways to fix this, but I don't like any > > of > > them;-) > > > > 1) I suppose mountd could check for each flag set in f_flags and > > generate > > a vfsopts string for it, but that means that every time a new flag > > that > > can be updated is added to MNT_xx, this mountd.c code would have to > > updated. > > --> Ugly!! > > > > 2) Require that all flags in MNT_UPDATEMASK be in the low order > > 32bits. > >I wouldn`t mind this except in means that the MNT_xx flags must > >be > > redefined > >and I don`t think that could get MFC`d. > > > > 3) Add a new newernmount(2) which has a 64bit flags argument > > instead of > > int. > > > > Hopefully someone can think of a nice way to fix this, rick > > ___ > > freebsd-current@freebsd.org mailing list > > http://lists.freebsd.org/mailman/listinfo/freebsd-current > > To unsubscribe, send any mail to > > "freebsd-current-unsubscr...@freebsd.org" > > > ___ > freebsd-current@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-current > To unsubscribe, send any mail to > "freebsd-current-unsubscr...@freebsd.org" > ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: nmount, mountd drops high order MNT_xx flags during a mount update
Doug Rabson wrote: > You could add a single integer-valued vfsopt which holds the > high-order > bits of f_flags? > Yes, that sounds better than any of my ideas. I'll code up a patch and put it up for review unless someone has a better solution. Thanks Doug, rick > On 7 May 2015 at 02:10, Rick Macklem wrote: > > > David Boyd reported a problem to freebsd-current@ w.r.t. the > > MNT_AUTOMOUNTED > > flag getting cleared by mountd. > > http://docs.FreeBSD.org/cgi/mid.cgi?1429477202.29812.38.camel > > I just took a look at this and it's kinda ugly... > > > > mountd acquires a list of mounts via getmntinfo() and then does an > > nmount() for each of them to get rid of any exports. It uses > > f_flags from the statfs returned by getmntinfo() as the 3rd > > argument to nmount(). > > --> Since nmount()s 3rd argument is a "int", it silently drops any > > MNT_xx flag in the high order 32bits of f_flags. At this time, > > it happens that MNT_AUTOMOUNTED is the only one, but... > > > > I can think of a couple of ways to fix this, but I don't like any > > of > > them;-) > > > > 1) I suppose mountd could check for each flag set in f_flags and > > generate > > a vfsopts string for it, but that means that every time a new flag > > that > > can be updated is added to MNT_xx, this mountd.c code would have to > > updated. > > --> Ugly!! > > > > 2) Require that all flags in MNT_UPDATEMASK be in the low order > > 32bits. > >I wouldn`t mind this except in means that the MNT_xx flags must > >be > > redefined > >and I don`t think that could get MFC`d. > > > > 3) Add a new newernmount(2) which has a 64bit flags argument > > instead of > > int. > > > > Hopefully someone can think of a nice way to fix this, rick > > ___ > > freebsd-current@freebsd.org mailing list > > http://lists.freebsd.org/mailman/listinfo/freebsd-current > > To unsubscribe, send any mail to > > "freebsd-current-unsubscr...@freebsd.org" > > > ___ > freebsd-current@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-current > To unsubscribe, send any mail to > "freebsd-current-unsubscr...@freebsd.org" > ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: nmount, mountd drops high order MNT_xx flags during a mount update
You could add a single integer-valued vfsopt which holds the high-order bits of f_flags? On 7 May 2015 at 02:10, Rick Macklem wrote: > David Boyd reported a problem to freebsd-current@ w.r.t. the > MNT_AUTOMOUNTED > flag getting cleared by mountd. > http://docs.FreeBSD.org/cgi/mid.cgi?1429477202.29812.38.camel > I just took a look at this and it's kinda ugly... > > mountd acquires a list of mounts via getmntinfo() and then does an > nmount() for each of them to get rid of any exports. It uses > f_flags from the statfs returned by getmntinfo() as the 3rd > argument to nmount(). > --> Since nmount()s 3rd argument is a "int", it silently drops any > MNT_xx flag in the high order 32bits of f_flags. At this time, > it happens that MNT_AUTOMOUNTED is the only one, but... > > I can think of a couple of ways to fix this, but I don't like any of > them;-) > > 1) I suppose mountd could check for each flag set in f_flags and generate > a vfsopts string for it, but that means that every time a new flag that > can be updated is added to MNT_xx, this mountd.c code would have to > updated. > --> Ugly!! > > 2) Require that all flags in MNT_UPDATEMASK be in the low order 32bits. >I wouldn`t mind this except in means that the MNT_xx flags must be > redefined >and I don`t think that could get MFC`d. > > 3) Add a new newernmount(2) which has a 64bit flags argument instead of > int. > > Hopefully someone can think of a nice way to fix this, rick > ___ > freebsd-current@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-current > To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org" > ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
nmount, mountd drops high order MNT_xx flags during a mount update
David Boyd reported a problem to freebsd-current@ w.r.t. the MNT_AUTOMOUNTED flag getting cleared by mountd. http://docs.FreeBSD.org/cgi/mid.cgi?1429477202.29812.38.camel I just took a look at this and it's kinda ugly... mountd acquires a list of mounts via getmntinfo() and then does an nmount() for each of them to get rid of any exports. It uses f_flags from the statfs returned by getmntinfo() as the 3rd argument to nmount(). --> Since nmount()s 3rd argument is a "int", it silently drops any MNT_xx flag in the high order 32bits of f_flags. At this time, it happens that MNT_AUTOMOUNTED is the only one, but... I can think of a couple of ways to fix this, but I don't like any of them;-) 1) I suppose mountd could check for each flag set in f_flags and generate a vfsopts string for it, but that means that every time a new flag that can be updated is added to MNT_xx, this mountd.c code would have to updated. --> Ugly!! 2) Require that all flags in MNT_UPDATEMASK be in the low order 32bits. I wouldn`t mind this except in means that the MNT_xx flags must be redefined and I don`t think that could get MFC`d. 3) Add a new newernmount(2) which has a 64bit flags argument instead of int. Hopefully someone can think of a nice way to fix this, rick ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
