Re: [libvirt] [PATCHv2 8/8] Remove bogus includes

On 12/02/2010 07:08 AM, Daniel P. Berrange wrote: All right; I'm now pushing the amended series; below are the actual differences from v2 that ended up being squashed in (mostly to 3/8, and mostly in response to Matthias' comments). That's a separate issue; it still needs to be resolved, but it is unrelated to virCommand (so much as it happened to be a patch on the git branch that I took from your tree when starting my improvements to virCommand). diff --git c/cfg.mk w/cfg.mk index 8a8da18..29de9d9 100644 --- c/cfg.mk +++ w/cfg.mk @@ -369,9 +369,9 @@ msg_gen_function += umlReportError msg_gen_function += vah_error msg_gen_function += vah_warning msg_gen_function += vboxError +msg_gen_function += virCommandError msg_gen_function += virConfError msg_gen_function += virDomainReportError -msg_gen_function += virSecurityReportError msg_gen_function += virHashError msg_gen_function += virLibConnError msg_gen_function += virLibDomainError @@ -380,6 +380,7 @@ msg_gen_function += virNodeDeviceReportError msg_gen_function += virRaiseError msg_gen_function += virReportErrorHelper msg_gen_function += virReportSystemError +msg_gen_function += virSecurityReportError msg_gen_function += virSexprError msg_gen_function += virStorageReportError msg_gen_function += virXMLError diff --git c/src/util/command.c w/src/util/command.c index 948a957..aa43f76 100644 --- c/src/util/command.c +++ w/src/util/command.c @@ -91,7 +91,7 @@ virCommandNew(const char *binary) /* * Create a new command with a NULL terminated - * set of args, taking binary from argv[0] + * set of args, taking binary from args[0] */ virCommandPtr virCommandNewArgs(const char *const*args) @@ -543,7 +543,7 @@ virCommandSetOutputBuffer(virCommandPtr cmd, char **outbuf) if (!cmd || cmd->has_error) return; - if (cmd->outfd != -1) { + if (cmd->outfdptr) { cmd->has_error = -1; VIR_DEBUG0("cannot specify output twice"); return; @@ -563,7 +563,7 @@ virCommandSetErrorBuffer(virCommandPtr cmd, char **errbuf) if (!cmd || cmd->has_error) return; - if (cmd->errfd != -1) { + if (cmd->errfdptr) { cmd->has_error = -1; VIR_DEBUG0("cannot specify stderr twice"); return; @@ -583,11 +583,16 @@ virCommandSetInputFD(virCommandPtr cmd, int infd) if (!cmd || cmd->has_error) return; - if (infd < 0 || cmd->inbuf) { + if (cmd->infd != -1 || cmd->inbuf) { cmd->has_error = -1; VIR_DEBUG0("cannot specify input twice"); return; } + if (infd < 0) { + cmd->has_error = -1; + VIR_DEBUG0("cannot specify invalid input fd"); + return; + } cmd->infd = infd; } @@ -602,7 +607,7 @@ virCommandSetOutputFD(virCommandPtr cmd, int *outfd) if (!cmd || cmd->has_error) return; - if (!outfd || cmd->outfd != -1) { + if (cmd->outfdptr) { cmd->has_error = -1; VIR_DEBUG0("cannot specify output twice"); return; @@ -621,7 +626,7 @@ virCommandSetErrorFD(virCommandPtr cmd, int *errfd) if (!cmd || cmd->has_error) return; - if (!errfd || cmd->errfd != -1) { + if (cmd->errfdptr) { cmd->has_error = -1; VIR_DEBUG0("cannot specify stderr twice"); return; @@ -642,6 +647,11 @@ virCommandSetPreExecHook(virCommandPtr cmd, virExecHook hook, void *opaque) if (!cmd || cmd->has_error) return; + if (cmd->hook) { + cmd->has_error = -1; + VIR_DEBUG0("cannot specify hook twice"); + return; + } cmd->hook = hook; cmd->opaque = opaque; } @@ -778,7 +788,7 @@ virCommandProcessIO(virCommandPtr cmd) if (nfds == 0) break; - if (poll(fds,nfds, -1) < 0) { + if (poll(fds, nfds, -1) < 0) { if ((errno == EAGAIN) || (errno == EINTR)) continue; virReportSystemError(errno, "%s", @@ -882,12 +892,25 @@ virCommandRun(virCommandPtr cmd, int *exitstatus) if (pipe(infd) < 0) { virReportSystemError(errno, "%s", _("unable to open pipe")); + cmd->has_error = -1; return -1; } cmd->infd = infd[0]; cmd->inpipe = infd[1]; } + /* If caller hasn't requested capture of stdout/err, then capture + * it ourselves so we can log it. + */ + if (!cmd->outfdptr) { + cmd->outfdptr = &cmd->outfd; + cmd->outbuf = &outbuf; + } + if (!cmd->errfdptr) { + cmd->errfdptr = &cmd->errfd; + cmd->errbuf = &errbuf; + } + if (virCommandRunAsync(cmd, NULL) < 0) { if (cmd->inbuf) { int tmpfd = infd[0]; @@ -897,26 +920,10 @@ virCommandRun(virCommandPtr cmd, int *exitstatus) if (VIR_CLOSE(infd[1]) < 0) VIR_DEBUG("ignoring failed close on fd %d", tmpfd); } + cmd->has_error = -1; return -1; } - /* If caller hasn't requested capture of - * stdout/err, then capture it ourselves - * so we can log it - */ - if (!cmd->outbuf && - !cmd->outfdptr) { - cmd->outfd = -1; - cmd->outfdptr = &cmd->outfd; - cmd->outbuf = &outbuf; - } - if (!cmd->errbuf && - !cmd->errfdptr) { - cmd->errfd = -1; - cmd->errfdptr = &cmd->errfd; - cmd->errbuf = &errbuf; - } - if (cmd->inbuf || cmd->outbuf || cmd->errbuf) ret = virCommandProcessIO(cmd); @@ -1012,7 +1019,7 @@ virCommandRunAsync(virCommandPtr cmd, pid_t *pid) } - str = virArgvToString((const char *const *)cmd->args); + str = virCommandToString(cmd); VIR_DEBUG("About to run %s", str ? str : cmd->args[0]); VIR_FREE(str); @@ -1090,7 +1097,7 @@ virCommandWait(virCommandPtr cmd, int *exitstatus) if (exitstatus == NULL) { if (status != 0) { virCommandError(VIR_ERR_INTERNAL_ERROR, - _("Intermediate daemon process exited with status %d."), + _("Child process exited with status %d."), WEXITSTATUS(status)); return -1; } diff --git c/tests/Makefile.am w/tests/Makefile.am index 357051b..e5c8d36 100644 --- c/tests/Makefile.am +++ w/tests/Makefile.am @@ -361,10 +361,6 @@ commandhelper_SOURCES = \ commandhelper_CFLAGS = -Dabs_builddir="\"$(abs_builddir)\"" commandhelper_LDADD = $(LDADDS) -statstest_SOURCES = \ - statstest.c testutils.h testutils.c -statstest_LDADD = $(LDADDS) - if WITH_SECDRIVER_SELINUX seclabeltest_SOURCES = \ seclabeltest.c diff --git c/tests/commandtest.c w/tests/commandtest.c index 46d6ae5..ace2f33 100644 --- c/tests/commandtest.c +++ w/tests/commandtest.c @@ -36,7 +36,7 @@ #include "command.h" #include "files.h" -#ifndef __linux__ +#ifdef WIN32 static int mymain(int argc ATTRIBUTE_UNUSED, char **argv ATTRIBUTE_UNUSED) @@ -143,11 +143,23 @@ cleanup: } /* - * Run program, no args, inherit all ENV, keep CWD. + * Run program (twice), no args, inherit all ENV, keep CWD. * Only stdin/out/err open */ static int test2(const void *unused ATTRIBUTE_UNUSED) { virCommandPtr cmd = virCommandNew(abs_builddir "/commandhelper"); + int ret; + + if (virCommandRun(cmd, NULL) < 0) { + virErrorPtr err = virGetLastError(); + printf("Cannot run child %s\n", err->message); + return -1; + } + + if ((ret = checkoutput("test2")) != 0) { + virCommandFree(cmd); + return ret; + } if (virCommandRun(cmd, NULL) < 0) { virErrorPtr err = virGetLastError(); @@ -625,6 +637,6 @@ mymain(int argc, char **argv) return(ret==0 ? EXIT_SUCCESS : EXIT_FAILURE); } -#endif /* __linux__ */ +#endif /* !WIN32 */ VIRT_TEST_MAIN(mymain) diff --git c/tests/qemuxml2argvtest.c w/tests/qemuxml2argvtest.c index ab4efbd..5fe91f1 100644 --- c/tests/qemuxml2argvtest.c +++ w/tests/qemuxml2argvtest.c @@ -116,9 +116,6 @@ static int testCompareXMLToArgvFiles(const char *xml, migrateFrom, NULL, VIR_VM_OP_CREATE))) goto fail; - if ((log = virtTestLogContentAndReset()) == NULL) - goto fail; - if (!!virGetLastError() != expectError) { if (virTestGetDebug() && (log = virtTestLogContentAndReset())) fprintf(stderr, "\n%s", log); @@ -133,6 +130,15 @@ static int testCompareXMLToArgvFiles(const char *xml, if (!(actualargv = virCommandToString(cmd))) goto fail; + if (emulator) { + /* Skip the abs_srcdir portion of replacement emulator. */ + char *start_skip = strstr(actualargv, abs_srcdir); + char *end_skip = strstr(actualargv, emulator); + if (!start_skip || !end_skip) + goto fail; + memmove(start_skip, end_skip, strlen(end_skip) + 1); + } + if (STRNEQ(expectargv, actualargv)) { virtTestDifference(stderr, expectargv, actualargv); goto fail; -- Eric Blake eblake(a)redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org