The attached patch adds some extra logging to the virRun command. The full argv is now logged with DEBUG, as well as the commands stdout and stderr (if anything is found). Also, if the command returns an error, we raise the stderr content with the reported error msg. This will significantly help with debugging certain issues, particularly with the storage driver which makes heavy use of virRun.

--- a/src/util.c +++ b/src/util.c @@ -54,6 +54,9 @@ #include "memory.h" #include "util-lib.c" +#define DEBUG(fmt,...) VIR_DEBUG(__FILE__, fmt, __VA_ARGS__) +#define DEBUG0(msg) VIR_DEBUG(__FILE__, "%s", msg)

+ #ifndef NSIG # define NSIG 32 #endif @@ -404,10 +407,19 @@ int virRun(virConnectPtr conn, const char *const*argv, int *status) { - int childpid, exitstatus, ret; + int childpid, exitstatus, ret, i; + int errfd = -1, outfd = -1; + char out[256] = "\0", err[256] = "\0", cmd[512] = "\0";; + + /* Log argv */ + for (i = 0; argv[i] != NULL; ++i) { + strncat(cmd, " ", sizeof(cmd) - strlen(cmd) - 1); + strncat(cmd, argv[i], sizeof(cmd) - strlen(cmd) - 1); + }

+ DEBUG("Running command '%s'", cmd); if ((ret = virExec(conn, argv, NULL, NULL, - &childpid, -1, NULL, NULL, VIR_EXEC_NONE)) < 0) + &childpid, -1, &outfd, &errfd, VIR_EXEC_NONE)) < 0) return ret; while ((ret = waitpid(childpid, &exitstatus, 0) == -1) && errno == EINTR); @@ -418,16 +430,31 @@ virRun(virConnectPtr conn, return -1; } + /* Log command output */ + if (outfd) { + ret = saferead(outfd, out, sizeof(out)-1); + err[ret < 0 ? 0 : ret] = '\0'; + if (*out) + DEBUG("Command stdout: %s", out);

+ } + if (errfd) { + ret = saferead(errfd, err, sizeof(err)-1); + err[ret < 0 ? 0 : ret] = '\0'; + if (*err) + DEBUG("Command stderr: %s", err);

On Tue, Sep 23, 2008 at 04:56:45PM -0400, Cole Robinson wrote: > if ((ret = virExec(conn, argv, NULL, NULL, > - &childpid, -1, NULL, NULL, VIR_EXEC_NONE)) < 0) > + &childpid, -1, &outfd, &errfd, VIR_EXEC_NONE)) < 0) > return ret; > > while ((ret = waitpid(childpid, &exitstatus, 0) == -1) && errno == EINTR); > @@ -418,16 +430,31 @@ virRun(virConnectPtr conn, > return -1; > } > > + /* Log command output */ > + if (outfd) { > + ret = saferead(outfd, out, sizeof(out)-1); > + err[ret < 0 ? 0 : ret] = '\0'; > + if (*out) > + DEBUG("Command stdout: %s", out); > + } > + if (errfd) { > + ret = saferead(errfd, err, sizeof(err)-1); > + err[ret < 0 ? 0 : ret] = '\0'; > + if (*err) > + DEBUG("Command stderr: %s", err); > + } > + I'm sure this works in tests, but I don't think it'll work reliably all the time. What happens if the command pauses for some time before sending output to stdout/stderr? You need to integrate these in the event loop, which is going to make everything a lot more complicated.