On Thu, Aug 01, 2019 at 05:09:58AM -0500, Eric Blake wrote: > On 8/1/19 4:12 AM, Richard W.M. Jones wrote: > > On Wed, Jul 31, 2019 at 05:01:52PM -0500, Eric Blake wrote: > >> On 7/31/19 4:31 PM, Eric Blake wrote: > >>> The rate filter is potentially opening fds in one thread while another > >>> thread is processing a fork() in the plugin. Although the file is not > >>> open for long, we MUST atomically use CLOEXEC to avoid fd leaks. This > >>> one is a bit harder to observe using only the sh plugin, because the > >>> window is small; you'll have better success at catching the leak by > >>> using gdb or recompiling code to insert strategic sleeps. > >> > >> In fact, I have to tweak this commit message: you CAN'T observe this one > >> with the sh plugin unless you recompile it to use #define THREAD_MODEL > >> NBDKIT_THREAD_MODEL_SERIALIZE_REQUESTS, as well as introducing the > >> timing hacks mentioned above (that's because with our current > >> SERIALIZE_ALL_REQUESTS, there is never more than one thread in > >> filter/plugin code at a time). > > > > The current nbdkit-sh-plugin is only SERIALIZE_ALL_REQUESTS in order > > to make writing the shell scripts a bit more sane. I believe it could > > be fully PARALLEL. > > Other than the fact that it uses pipe() instead of pipe2(), I'm not > seeing any other strong reasons why it can't be parallel. I'll change > patch 9 along those lines. > > > > > (As an aside: Ideally in future we'll allow the thread model to be > > specified by the plugin dynamically. It's one of the things I thought > > I had listed in the TODO file - it wasn't there so I've added it now.) > > That's because we already have that: See commit afbcd070 and nearby. So > I'll just revert your TODO change :)
Indeed we do. Should add that to all the language plugins at some point ... Rich. > >> But it does raise an interesting point - if we hit platforms that are > >> unable to support atomic CLOEXEC, one possibility is a patch that forces > >> SERIALIZE_ALL_REQUESTS as the maximum parallelism allowed on that > >> platform (while remaining at our goal of PARALLEL on more competent > >> systems) - once we do that, the lacking systems will be serialized to > >> the point that there is no race window where one thread can fork() while > >> another is obtaining an fd. > > > > Yup. But probably better to encourage those platforms to support > > atomic CLOEXEC everywhere. > > Yes, it would be nice for Haiku to realize how much they are losing out > on by not providing it. > > -- > Eric Blake, Principal Software Engineer > Red Hat, Inc. +1-919-301-3226 > Virtualization: qemu.org | libvirt.org > -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html _______________________________________________ Libguestfs mailing list [email protected] https://www.redhat.com/mailman/listinfo/libguestfs
