[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



On 12/02/2010 07:08 AM, Daniel P. Berrange wrote:
> On Tue, Nov 23, 2010 at 04:50:01PM -0700, Eric Blake wrote:
>> From: Daniel P. Berrange <berrange redhat com>
>>
>> ---
>>
>> v2: rearrange to later in the series; no other change.  Passes
>> for me with macvtap compilation enabled, so I'm not sure if it
>> still suffers from the same problem as the v1 complaint:
>> https://www.redhat.com/archives/libvir-list/2010-November/msg00834.html
>>
>>  src/conf/domain_conf.c |    1 -
>>  src/util/hooks.c       |    1 -
>>  2 files changed, 0 insertions(+), 2 deletions(-)
> 
> ACK

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).

> 
> The problem I hit was actually with removing
> 
> diff --git a/src/util/macvtap.c b/src/util/macvtap.c
> index 5dcc9e1..eb4ea8f 100644
> --- a/src/util/macvtap.c
> +++ b/src/util/macvtap.c
> @@ -49,7 +49,6 @@
>  # include "logging.h"
>  # include "macvtap.h"
>  # include "interface.h"
> -# include "conf/domain_conf.h"
> 
> 
> Because the 'util' directory must never depend on the 'conf' directory.

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 redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]