This addreses portability to Windows and standardizes error reporting. This fixes a number of places which failed to set O_CLOEXEC or failed to report errors. Signed-off-by: Daniel P. Berrangé <berrange@xxxxxxxxxx> --- src/libxl/libxl_migration.c | 6 ++---- src/locking/lock_daemon.c | 2 +- src/logging/log_daemon.c | 2 +- src/logging/log_handler.c | 6 ++---- src/lxc/lxc_process.c | 5 +---- src/qemu/qemu_migration.c | 25 +++++++++---------------- src/qemu/qemu_tpm.c | 5 +---- src/remote/remote_daemon.c | 2 +- src/rpc/virnetdaemon.c | 5 +---- src/rpc/virnetsocket.c | 5 +---- src/util/vircommand.c | 18 +++++------------- src/util/virfdstream.c | 5 +---- src/util/virfile.c | 5 +---- src/util/virpolkit.c | 2 +- src/util/virprocess.c | 5 +---- tests/commandtest.c | 4 ++-- tests/eventtest.c | 2 +- tests/testutils.c | 2 +- tools/virsh-domain.c | 8 ++++---- tools/vsh.c | 8 +++----- 20 files changed, 40 insertions(+), 82 deletions(-) diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c index 404c437a22..873b2b3e01 100644 --- a/src/libxl/libxl_migration.c +++ b/src/libxl/libxl_migration.c @@ -584,7 +584,7 @@ libxlDomainMigrationDstPrepareTunnel3(virConnectPtr dconn, * The data flow of tunnel3 migration in the dest side: * stream -> pipe -> recvfd of libxlDomainStartRestore */ - if (pipe(dataFD) < 0) + if (virPipe(dataFD) < 0) goto endjob; /* Stream data will be written to pipeIn */ @@ -916,10 +916,8 @@ libxlMigrationSrcStartTunnel(libxlDriverPrivatePtr driver, tc->dataFD[0] = -1; tc->dataFD[1] = -1; - if (pipe(tc->dataFD) < 0) { - virReportError(errno, "%s", _("Unable to make pipes")); + if (virPipe(tc->dataFD) < 0) goto out; - } arg = &tc->tmThread; /* Read from pipe */ diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index 65c38139c4..9c72db93fd 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -356,7 +356,7 @@ static int virLockDaemonForkIntoBackground(const char *argv0) { int statuspipe[2]; - if (pipe(statuspipe) < 0) + if (virPipeQuiet(statuspipe) < 0) return -1; pid_t pid = fork(); diff --git a/src/logging/log_daemon.c b/src/logging/log_daemon.c index 488f3b459d..2ce6eb7447 100644 --- a/src/logging/log_daemon.c +++ b/src/logging/log_daemon.c @@ -298,7 +298,7 @@ static int virLogDaemonForkIntoBackground(const char *argv0) { int statuspipe[2]; - if (pipe(statuspipe) < 0) + if (virPipeQuiet(statuspipe) < 0) return -1; pid_t pid = fork(); diff --git a/src/logging/log_handler.c b/src/logging/log_handler.c index 030c9d66e3..26ccfb4470 100644 --- a/src/logging/log_handler.c +++ b/src/logging/log_handler.c @@ -385,11 +385,9 @@ virLogHandlerDomainOpenLogFile(virLogHandlerPtr handler, } } - if (pipe(pipefd) < 0) { - virReportSystemError(errno, "%s", - _("Cannot open fifo pipe")); + if (virPipe(pipefd) < 0) goto error; - } + if (VIR_ALLOC(file) < 0) goto error; diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index af8593d6a5..32a237fadd 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -1364,11 +1364,8 @@ int virLXCProcessStart(virConnectPtr conn, goto cleanup; } - if (pipe(handshakefds) < 0) { - virReportSystemError(errno, "%s", - _("Unable to create pipe")); + if (virPipe(handshakefds) < 0) goto cleanup; - } if (!(cmd = virLXCProcessBuildControllerCmd(driver, vm, diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 76dcd36266..2323008699 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2488,11 +2488,8 @@ qemuMigrationDstPrepareAny(virQEMUDriverPtr driver, goto done; if (tunnel && - (pipe(dataFD) < 0 || virSetCloseExec(dataFD[1]) < 0)) { - virReportSystemError(errno, "%s", - _("cannot create pipe for tunnelled migration")); + virPipe(dataFD) < 0) goto stopjob; - } startFlags = VIR_QEMU_PROCESS_START_AUTODESTROY; @@ -3245,11 +3242,8 @@ qemuMigrationSrcStartTunnel(virStreamPtr st, qemuMigrationIOThreadPtr io = NULL; int wakeupFD[2] = { -1, -1 }; - if (pipe2(wakeupFD, O_CLOEXEC) < 0) { - virReportSystemError(errno, "%s", - _("Unable to make pipe")); + if (virPipe(wakeupFD) < 0) goto error; - } if (VIR_ALLOC(io) < 0) goto error; @@ -3863,10 +3857,12 @@ qemuMigrationSrcPerformTunnel(virQEMUDriverPtr driver, spec.dest.fd.qemu = -1; spec.dest.fd.local = -1; - if (pipe2(fds, O_CLOEXEC) == 0) { - spec.dest.fd.qemu = fds[1]; - spec.dest.fd.local = fds[0]; - } + if (virPipe(fds) < 0) + goto cleanup; + + spec.dest.fd.qemu = fds[1]; + spec.dest.fd.local = fds[0]; + if (spec.dest.fd.qemu == -1 || qemuSecuritySetImageFDLabel(driver->securityManager, vm->def, spec.dest.fd.qemu) < 0) { @@ -5228,11 +5224,8 @@ qemuMigrationSrcToFile(virQEMUDriverPtr driver, virDomainObjPtr vm, return -1; } - if (compressor && pipe(pipeFD) < 0) { - virReportSystemError(errno, "%s", - _("Failed to create pipe for migration")); + if (compressor && virPipe(pipeFD) < 0) return -1; - } /* All right! We can use fd migration, which means that qemu * doesn't have to open() the file, so while we still have to diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index 6741373583..17b800db1c 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -374,11 +374,8 @@ qemuTPMSetupEncryption(const unsigned char *secretuuid, &secret, &secret_len) < 0) goto error; - if (pipe(pipefd) == -1) { - virReportSystemError(errno, "%s", - _("Unable to create pipe")); + if (virPipe(pipefd) < 0) goto error; - } if (virCommandSetSendBuffer(cmd, pipefd[1], secret, secret_len) < 0) goto error; diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c index 524ee2cab9..f833cc1024 100644 --- a/src/remote/remote_daemon.c +++ b/src/remote/remote_daemon.c @@ -113,7 +113,7 @@ VIR_ENUM_IMPL(virDaemonErr, static int daemonForkIntoBackground(const char *argv0) { int statuspipe[2]; - if (pipe(statuspipe) < 0) + if (virPipeQuiet(statuspipe) < 0) return -1; pid_t pid = fork(); diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c index aa77e984ab..7f2226b086 100644 --- a/src/rpc/virnetdaemon.c +++ b/src/rpc/virnetdaemon.c @@ -671,11 +671,8 @@ virNetDaemonSignalSetup(virNetDaemonPtr dmn) if (dmn->sigwrite != -1) return 0; - if (pipe2(fds, O_CLOEXEC|O_NONBLOCK) < 0) { - virReportSystemError(errno, "%s", - _("Unable to create signal pipe")); + if (virPipeNonBlock(fds) < 0) return -1; - } if ((dmn->sigwatch = virEventAddHandle(fds[0], VIR_EVENT_HANDLE_READABLE, diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index 07733cee48..494b548948 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -794,11 +794,8 @@ int virNetSocketNewConnectCommand(virCommandPtr cmd, goto error; } - if (pipe(errfd) < 0) { - virReportSystemError(errno, "%s", - _("unable to create socket pair")); + if (virPipe(errfd) < 0) goto error; - } virCommandSetInputFD(cmd, sv[1]); virCommandSetOutputFD(cmd, &sv[1]); diff --git a/src/util/vircommand.c b/src/util/vircommand.c index 26b3488d6b..33028aa963 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -585,11 +585,8 @@ virExec(virCommandPtr cmd) if (cmd->outfdptr != NULL) { if (*cmd->outfdptr == -1) { - if (pipe2(pipeout, O_CLOEXEC) < 0) { - virReportSystemError(errno, - "%s", _("cannot create pipe")); + if (virPipe(pipeout) < 0) goto cleanup; - } if ((cmd->flags & VIR_EXEC_NONBLOCK) && virSetNonBlock(pipeout[0]) == -1) { @@ -612,11 +609,8 @@ virExec(virCommandPtr cmd) if (cmd->errfdptr == cmd->outfdptr) { childerr = childout; } else if (*cmd->errfdptr == -1) { - if (pipe2(pipeerr, O_CLOEXEC) < 0) { - virReportSystemError(errno, - "%s", _("Failed to create pipe")); + if (virPipe(pipeerr) < 0) goto cleanup; - } if ((cmd->flags & VIR_EXEC_NONBLOCK) && virSetNonBlock(pipeerr[0]) == -1) { @@ -2478,9 +2472,7 @@ virCommandRunAsync(virCommandPtr cmd, pid_t *pid) * virCommandDoAsyncIO. */ if (cmd->inbuf && cmd->infd == -1 && (synchronous || cmd->flags & VIR_EXEC_ASYNC_IO)) { - if (pipe2(infd, O_CLOEXEC) < 0) { - virReportSystemError(errno, "%s", - _("unable to open pipe")); + if (virPipe(infd) < 0) { cmd->has_error = -1; return -1; } @@ -2724,11 +2716,11 @@ void virCommandRequireHandshake(virCommandPtr cmd) return; } - if (pipe2(cmd->handshakeWait, O_CLOEXEC) < 0) { + if (virPipeQuiet(cmd->handshakeWait) < 0) { cmd->has_error = errno; return; } - if (pipe2(cmd->handshakeNotify, O_CLOEXEC) < 0) { + if (virPipeQuiet(cmd->handshakeNotify) < 0) { VIR_FORCE_CLOSE(cmd->handshakeWait[0]); VIR_FORCE_CLOSE(cmd->handshakeWait[1]); cmd->has_error = errno; diff --git a/src/util/virfdstream.c b/src/util/virfdstream.c index 1354d2ee52..3337fc2060 100644 --- a/src/util/virfdstream.c +++ b/src/util/virfdstream.c @@ -1274,11 +1274,8 @@ virFDStreamOpenFileInternal(virStreamPtr st, goto error; } - if (pipe(pipefds) < 0) { - virReportSystemError(errno, "%s", - _("Unable to create pipe")); + if (virPipe(pipefds) < 0) goto error; - } if (VIR_ALLOC(threadData) < 0) goto error; diff --git a/src/util/virfile.c b/src/util/virfile.c index 96778c61bd..2ab4ccb1d5 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -275,11 +275,8 @@ virFileWrapperFdNew(int *fd, const char *name, unsigned int flags) goto error; } - if (pipe2(pipefd, O_CLOEXEC) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unable to create pipe for %s"), name); + if (virPipe(pipefd) < 0) goto error; - } if (!(iohelper_path = virFileFindResource("libvirt_iohelper", abs_top_builddir "/src", diff --git a/src/util/virpolkit.c b/src/util/virpolkit.c index 0db7bf0747..5c2d0d4bd4 100644 --- a/src/util/virpolkit.c +++ b/src/util/virpolkit.c @@ -171,7 +171,7 @@ virPolkitAgentCreate(void) if (!isatty(STDIN_FILENO)) goto error; - if (pipe2(pipe_fd, 0) < 0) + if (virPipe(pipe_fd) < 0) goto error; if (VIR_ALLOC(agent) < 0) diff --git a/src/util/virprocess.c b/src/util/virprocess.c index 60419538e2..e33f5bdc7e 100644 --- a/src/util/virprocess.c +++ b/src/util/virprocess.c @@ -1175,11 +1175,8 @@ virProcessRunInFork(virProcessForkCallback cb, pid_t parent = getpid(); int errfd[2] = { -1, -1 }; - if (pipe2(errfd, O_CLOEXEC) < 0) { - virReportSystemError(errno, "%s", - _("Cannot create pipe for child")); + if (virPipe(errfd) < 0) return -1; - } if ((child = virFork()) < 0) goto cleanup; diff --git a/tests/commandtest.c b/tests/commandtest.c index 7df3ae0171..ae2598d5fd 100644 --- a/tests/commandtest.c +++ b/tests/commandtest.c @@ -1019,7 +1019,7 @@ static int test25(const void *unused G_GNUC_UNUSED) int ngroups; virCommandPtr cmd = virCommandNew("some/nonexistent/binary"); - if (pipe(pipeFD) < 0) { + if (virPipeQuiet(pipeFD) < 0) { fprintf(stderr, "Unable to create pipe\n"); goto cleanup; } @@ -1187,7 +1187,7 @@ static int test27(const void *unused G_GNUC_UNUSED) errexpect = g_strdup_printf(TEST27_ERREXPECT_TEMP, buffer0, buffer1, buffer2); - if (pipe(pipe1) < 0 || pipe(pipe2) < 0) { + if (virPipeQuiet(pipe1) < 0 || virPipeQuiet(pipe2) < 0) { printf("Could not create pipe: %s\n", g_strerror(errno)); goto cleanup; } diff --git a/tests/eventtest.c b/tests/eventtest.c index 1bda5efe6d..9855b578fb 100644 --- a/tests/eventtest.c +++ b/tests/eventtest.c @@ -321,7 +321,7 @@ mymain(void) char one = '1'; for (i = 0; i < NUM_FDS; i++) { - if (pipe(handles[i].pipeFD) < 0) { + if (virPipeQuiet(handles[i].pipeFD) < 0) { fprintf(stderr, "Cannot create pipe: %d", errno); return EXIT_FAILURE; } diff --git a/tests/testutils.c b/tests/testutils.c index 579b6fce1a..7b9a5ea05b 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -377,7 +377,7 @@ virTestCaptureProgramOutput(const char *const argv[], char **buf, int maxlen) int pipefd[2]; int len; - if (pipe(pipefd) < 0) + if (virPipeQuiet(pipefd) < 0) return -1; pid_t pid = fork(); diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 04ba44d4f2..a1e2409303 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -4422,7 +4422,7 @@ cmdSave(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptBool(cmd, "verbose")) verbose = true; - if (pipe(p) < 0) + if (virPipeQuiet(p) < 0) goto cleanup; data.ctl = ctl; @@ -4731,7 +4731,7 @@ cmdManagedSave(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptBool(cmd, "verbose")) verbose = true; - if (pipe(p) < 0) + if (virPipeQuiet(p) < 0) goto cleanup; data.ctl = ctl; @@ -5461,7 +5461,7 @@ cmdDump(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptBool(cmd, "verbose")) verbose = true; - if (pipe(p) < 0) + if (virPipeQuiet(p) < 0) goto cleanup; data.ctl = ctl; @@ -11032,7 +11032,7 @@ cmdMigrate(vshControl *ctl, const vshCmd *cmd) goto cleanup; } - if (pipe(p) < 0) + if (virPipeQuiet(p) < 0) goto cleanup; data.ctl = ctl; diff --git a/tools/vsh.c b/tools/vsh.c index 9c58e40d4f..2cf3dab054 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -2061,11 +2061,9 @@ vshEventStart(vshControl *ctl, int timeout_ms) assert(ctl->eventPipe[0] == -1 && ctl->eventPipe[1] == -1 && ctl->eventTimerId >= 0); - if (pipe2(ctl->eventPipe, O_CLOEXEC) < 0) { - char ebuf[1024]; - - vshError(ctl, _("failed to create pipe: %s"), - virStrerror(errno, ebuf, sizeof(ebuf))); + if (virPipe(ctl->eventPipe) < 0) { + vshSaveLibvirtError(); + vshReportError(ctl); return -1; } -- 2.24.1