Re: [Libguestfs] [nbdkit PATCH v2 10/17] plugins: Add .fork_safe field

On Fri, Aug 02, 2019 at 02:26:11PM -0500, Eric Blake wrote: > Allow a plugin field to declare whether a parallel plugin can tolerate > windows where fds are not CLOEXEC, or must take precautions to avoid > leaking fds if the plugin may fork. For safety reasons, the flag > defaults to off, but many in-tree plugins can set it to on (most > commonly because they don't fork after .config_complete; for libvirt > because it is documented to clean up fds on fork so it is immune to > anything we might leak; for libnbd because we don't use the API that > forks). Note that I did not choose to set the new field for many of > the various language bindings (it becomes a rather difficult task to > prove whether the third-party language binding code is itself using > atomic CLOEXEC or fd sanitization). However many of our languages are > still stuck as serialized, and the lack of .fork_safe won't impact > those thread model anyways. > > Update the testsuite to skip parallel tests that would otherwise fail > when the thread model is crippled. > > Upcoming patches will then fix the server to audit and fix places > where we currently leak fds, and then cripple the thread model only on > platforms where atomic CLOEXEC is not possible. My worry about this patch is we're adding a new plugin flag which we'll have to support forever, but IIUC it's only needed on one platform (ie. Haiku) which really ought to get fixed. In future we'll end up in a situation where we have this flag but it's no longer needed.

How about instead of this we simply restrict Haiku to the fully serialized mode. Sucks for them, but they can fix it by adding atomic CLOEXEC features ...

> + .fork_safe = 0, /* libguestfs uses fork(), unaudited for safety */ libguestfs does use fork and should be safe - we're pretty careful about using CLOEXEC, accept4, etc everywhere. (Also libguestfs doesn't run on Haiku and architecturally that's unlikely to ever happen).