Date: Wed, 26 Aug 2020 21:15:20 +0000 From: Rick Macklem <rmacklem@uoguelph.ca> To: J David <j.david.lists@gmail.com>, Brandon Bergren <bdragon@FreeBSD.org> Cc: FreeBSD Hackers <freebsd-hackers@freebsd.org> Subject: Re: pidfile_open() usage in "mount" Message-ID: <QB1PR01MB3364C5F4640D161DEE96E579DD540@QB1PR01MB3364.CANPRD01.PROD.OUTLOOK.COM> In-Reply-To: <CABXB=RQe8Epvr=7oZv9C3D4NXyd9TRvRctxM0yy_c5oB_%2B7P4A@mail.gmail.com> References: <CABXB=RRM7YusQL1gGVGD4s9xi9AB4F5AR5-Jqbp4G1WTZWWA%2BQ@mail.gmail.com> <20200826144013.GV2551@kib.kiev.ua> <dcf65725-af6f-4a89-bcac-b1ca46d49cae@www.fastmail.com>, <CABXB=RQe8Epvr=7oZv9C3D4NXyd9TRvRctxM0yy_c5oB_%2B7P4A@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
Brandon Bergren wrote:=0A= >> The pidfile_* stuff is meant for the daemon side.=0A= >=0A= >As written, agreed, it clearly is. But alas "meant for" and "used=0A= >for" appear to differ. It looks like there's a client-side use in ZFS=0A= >as well (cddl/compat/opensolaris/misc/fsshare.c).=0A= >=0A= >> Why not just regular open(2)?=0A= >=0A= >That's also a solution I'd be fine with submitting. Certainly less=0A= >work. But there are a couple of reasons not to do that, mostly to do=0A= >with code reuse.=0A= >=0A= >First, the pidfile library does some stuff around path handling that=0A= >is useful not to try to reinvent.=0A= >=0A= >Second, while races do exist, use of pidfiles in this way probably=0A= >isn't going away, so having the code that tries to minimize those=0A= >races in one place makes a lot of sense, rather than having=0A= >potentially multiple idiosyncratic approaches/cases on an ad-hoc=0A= >basis. ("Don't let the perfect be the enemy of the good" and all=0A= >that.)=0A= >=0A= >The CDDL thing is actually a good example of this, because it looks=0A= >lifted straight from mount.c, but when mount.c got updated with the=0A= >mountdpid <=3D 0 check, fsshare.c didn't, and that's almost certainly a=0A= >bug because it could lead to inadvertent kill( -1, SIGHUP ) with=0A= >superuser permissions. Whoops! ZFS just randomly HUP'd every process=0A= >on the system.=0A= >=0A= >Having mount HUP mountd (iff it's running) seems odd to me anyway, but=0A= >today's filesystems are so weird and complex and diverse that I'm sure=0A= >it's essential for somebody.=0A= Yes, it is weird and I don't think I was the guilty party.=0A= The reason is that, if the local file system being mounted happens to be in= =0A= /etc/exports, then /etc/exports has to be re-loaded for the update.=0A= =0A= Ideally, all local file systems would be mounted and then one HUP would=0A= be sent to mountd, but that isn't the way it has worked for a long time=0A= and changing it would be a POLA violation, imho.=0A= =0A= Imagine how many times this happens for a server with 20,000 file=0A= systems that happen to be exported (such a server does exist, apparently).= =0A= =0A= I ended up coming up with an incremental exports update to improve this=0A= and Peter (the guy that has the 20,000+ file system servers) has a patch th= at=0A= uses a database instead of the flat exports file. (Hopefully a variant of h= is=0A= patch can make it into ZFS someday. Now that the big switch of ZFS to ZoL= =0A= has happened, maybe someday soon?)=0A= =0A= rick=0A= =0A= Thanks!=0A= _______________________________________________=0A= freebsd-hackers@freebsd.org mailing list=0A= https://lists.freebsd.org/mailman/listinfo/freebsd-hackers=0A= To unsubscribe, send any mail to "freebsd-hackers-unsubscribe@freebsd.org"= =0A= =0A=
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?QB1PR01MB3364C5F4640D161DEE96E579DD540>