Page MenuHomeFreeBSD

Mount option to set high order mnt_flag bits
AbandonedPublic

Authored by rmacklem on May 10 2015, 12:54 PM.

Details

Reviewers
kib
trasz
Summary

Since nmount(2)'s flag argument is "int", the high order 32bits of mnt_flag are ignored by that argument.
This patch adds a mount option called "fsflaghigh" that allows processes to set all the high order 32bits
in one option during a mount (usually an update to an existing mount).
It is needed by mountd, so that high order bits don't get deleted when exports are updated.

Test Plan

I've already done minimal testing by setting "automounted" on a mount point and
seeing that it doesn't get deleted when exports are updated via mountd.
Hopefully David Boyd will be able to test the patch as well.

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

rmacklem updated this revision to Diff 5313.May 10 2015, 12:54 PM
rmacklem retitled this revision from to Mount option to set high order mnt_flag bits.
rmacklem updated this object.
rmacklem edited the test plan for this revision. (Show Details)
rmacklem added a reviewer: trasz.
dfr added a subscriber: dfr.May 10 2015, 2:40 PM

I know I suggested adding an option for the high flag bits but now I wonder if it would be cleaner to just have all 64 bits of flags in an option which would be or'ed with the flags argument from nmount. This would cut out most of the bit masking.

I thought of making the flag option all 64bits, but thought
it might be more confusing having the low order 32bits set in
two different ways.

However, I don't have a strong opinion w.r.t. this, so I'll go
with whatever the "collective" prefers.

trasz edited edge metadata.May 12 2015, 11:32 AM

I'm not sure I like this. Woulnd't it be better to just extend the "flags" argument to nmount(2) to uint64_t (as it is in "struct statfs") and provide a compatibility code for old binaries?

Yes, I agree that a new nmount(2) would be a cleaner fix. I hesitated to
do that, since it seemed a major change for a bug which seems like
a minor quirk to me but, again, it is up to the collective and not me.

Could a change that introduced a new nmount(2) syscall, retaining
the old one for compatibility be MFC'able?
(I'm not sure if this matters, since there is other ways around the
MNT_AUTOMOUNTED bug in mountd and it is the only high order
bit used by FreeBSD10.)

trasz added a comment.May 12 2015, 6:07 PM

Ok, so I went and made a quick prototype, https://reviews.freebsd.org/D2524. I prefer it to having two different ways of specifying binary flags, and I believe it would be MFC-able. However, at this point I'm not sure if it actually fixes any problem. Basically, I don't think there is any point in exporting automounted filesystems, and mountd(8) was already fixed to not touch filesystems without MNT_EXPORTED flags.

I agree that the immediate problem related to MNT_AUTOMOUNTED
being cleared by mountd is fixed by r281699. It just needs to be MFC'd.
This just needs to be resolved when another MNT_xx flag that can be
updated is added to the high order bits.

I have no strong opinion w.r.t. whether D2524 is the better fix, so
hopefully others will weigh in on this.
(I'll admit that I personally consider compatibility shims to be necessary
evil, since they seem to live forever, once in the source tree. Adding one
of these for this case is why I didn't go that way, but I do agree a 64bit arg.
to nmount() is a cleaner fix.)

trasz added a comment.Jul 2 2015, 1:29 PM

The actual fix r281699 was MFC-ed as r282915; perhaps you could mark this diff as abandoned?

rmacklem abandoned this revision.Jul 2 2015, 10:50 PM

As requested by trasz, abandon ship...