On 8/2/19 2:26 PM, Eric Blake wrote: > Haiku unfortunately lacks pipe2, so we have to plan for a fallback at > each site that uses it. This also makes a good time to audit all > existing users of pipe, to see if they should be using pipe2. The > tests fork() but don't fail because of fd leaks; and the nbd plugin > doesn't fork() but was merely using pipe2 for convenience over > multiple fcntl calls. However, the server's quit_fd definitely needs > to be marked CLOEXEC (it's easy to use the sh plugin to show that we > are currently leaking it to children), although doing so can occur > without worrying about atomicity since it is called before threads > begin. Finally, it's also worth updating our set_cloexec helper > function to document that we still prefer atomicity where possible. > > Signed-off-by: Eric Blake <[email protected]> > ---
> +++ b/plugins/nbd/nbd.c
> @@ -431,11 +431,42 @@ nbdplug_open_handle (int readonly)
> nbdkit_error ("malloc: %m");
> return NULL;
> }
> +#ifdef HAVE_PIPE2
> if (pipe2 (h->fds, O_NONBLOCK)) {
> + nbdkit_error ("pipe2: %m");
> + free (h);
> + return NULL;
> + }
> +#else
> + /* This plugin doesn't fork, so we don't care about CLOEXEC. Our use
> + * of pipe2 is merely for convenience.
> + */
> + if (pipe (h->fds)) {
> nbdkit_error ("pipe: %m");
> free (h);
> return NULL;
> }
> + else {
> + int f;
> +
> + f = fcntl (h->fds[0], F_GETFL);
> + if (f == -1 || fcntl (h->fds[0], F_SETFL, f | O_NONBLOCK) == -1) {
> + nbdkit_error ("fcntl: %m");
Shoot - I wrote up set_nonblock earlier in the series, then forgot to
use it here. Consider this squashed in:
diff --git i/plugins/nbd/nbd.c w/plugins/nbd/nbd.c
index 95d910e7..f11e54d5 100644
--- i/plugins/nbd/nbd.c
+++ w/plugins/nbd/nbd.c
@@ -54,6 +54,7 @@
#include <nbdkit-plugin.h>
#include "byte-swapping.h"
#include "cleanup.h"
+#include "utils.h"
/* The per-transaction details */
struct transaction {
@@ -446,25 +447,15 @@ nbdplug_open_handle (int readonly)
free (h);
return NULL;
}
- else {
- int f;
-
- f = fcntl (h->fds[0], F_GETFL);
- if (f == -1 || fcntl (h->fds[0], F_SETFL, f | O_NONBLOCK) == -1) {
- nbdkit_error ("fcntl: %m");
- close (h->fds[0]);
- close (h->fds[1]);
- free (h);
- return NULL;
- }
- f = fcntl (h->fds[1], F_GETFL);
- if (f == -1 || fcntl (h->fds[1], F_SETFL, f | O_NONBLOCK) == -1) {
- nbdkit_error ("fcntl: %m");
- close (h->fds[0]);
- close (h->fds[1]);
- free (h);
- return NULL;
- }
+ if (set_nonblock (h->fds[0]) == -1) {
+ close (h->fds[1]);
+ free (h);
+ return NULL;
+ }
+ if (set_nonblock (h->fds[1]) == -1) {
+ close (h->fds[0]);
+ free (h);
+ return NULL;
}
#endif
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Libguestfs mailing list [email protected] https://www.redhat.com/mailman/listinfo/libguestfs
