pidfile_open() usage in "mount"

[Rick Macklem]

Brandon Bergren wrote:
>> The pidfile_* stuff is meant for the daemon side.
>
>As written, agreed, it clearly is.  But alas "meant for" and "used
>for" appear to differ.  It looks like there's a client-side use in ZFS
>as well (cddl/compat/opensolaris/misc/fsshare.c).
>
>> Why not just regular open(2)?
>
>That's also a solution I'd be fine with submitting.  Certainly less
>work.  But there are a couple of reasons not to do that, mostly to do
>with code reuse.
>
>First, the pidfile library does some stuff around path handling that
>is useful not to try to reinvent.
>
>Second, while races do exist, use of pidfiles in this way probably
>isn't going away, so having the code that tries to minimize those
>races in one place makes a lot of sense, rather than having
>potentially multiple idiosyncratic approaches/cases on an ad-hoc
>basis.  ("Don't let the perfect be the enemy of the good" and all
>that.)
>
>The CDDL thing is actually a good example of this, because it looks
>lifted straight from mount.c, but when mount.c got updated with the
>mountdpid <= 0 check, fsshare.c didn't, and that's almost certainly a
>bug because it could lead to inadvertent kill( -1, SIGHUP ) with
>superuser permissions.  Whoops!  ZFS just randomly HUP'd every process
>on the system.
>
>Having mount HUP mountd (iff it's running) seems odd to me anyway, but
>today's filesystems are so weird and complex and diverse that I'm sure
>it's essential for somebody.
Yes, it is weird and I don't think I was the guilty party.
The reason is that, if the local file system being mounted happens to be in
/etc/exports, then /etc/exports has to be re-loaded for the update.

Ideally, all local file systems would be mounted and then one HUP would
be sent to mountd, but that isn't the way it has worked for a long time
and changing it would be a POLA violation, imho.

Imagine how many times this happens for a server with 20,000 file
systems that happen to be exported (such a server does exist, apparently).

I ended up coming up with an incremental exports update to improve this
and Peter (the guy that has the 20,000+ file system servers) has a patch that
uses a database instead of the flat exports file. (Hopefully a variant of his
patch can make it into ZFS someday. Now that the big switch of ZFS to ZoL
has happened, maybe someday soon?)

rick

Thanks!
_______________________________________________
freebsd-hackers at freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to "freebsd-hackers-unsubscribe at freebsd.org"