Improve virExec error reporting

This commit is contained in:
Daniel P. Berrange 2008-08-20 08:30:04 +00:00
parent 5cd1d3c53c
commit f2172946e5
2 changed files with 101 additions and 36 deletions

View File

@ -1,3 +1,7 @@
Wed Aug 20 09:28:33 BST 2008 Daniel P. Berrange <berrange@redhat.com>
* src/util.c: Re-arrange virExec() to improve error reporting
Mon Aug 18 10:22:33 BST 2008 Daniel P. Berrange <berrange@redhat.com> Mon Aug 18 10:22:33 BST 2008 Daniel P. Berrange <berrange@redhat.com>
* src/libvirt.c: Remove duplicate call to virInitialize() in * src/libvirt.c: Remove duplicate call to virInitialize() in

View File

@ -64,9 +64,12 @@
#ifndef PROXY #ifndef PROXY
static void static void
ReportError(virConnectPtr conn, ReportError(virConnectPtr conn,
virDomainPtr dom, int code, const char *fmt, ...)
virNetworkPtr net, ATTRIBUTE_FORMAT(printf, 3, 4);
int code, const char *fmt, ...) {
static void
ReportError(virConnectPtr conn,
int code, const char *fmt, ...) {
va_list args; va_list args;
char errorMessage[MAX_ERROR_LEN]; char errorMessage[MAX_ERROR_LEN];
@ -77,7 +80,7 @@ ReportError(virConnectPtr conn,
} else { } else {
errorMessage[0] = '\0'; errorMessage[0] = '\0';
} }
__virRaiseError(conn, dom, net, VIR_FROM_NONE, code, VIR_ERR_ERROR, __virRaiseError(conn, NULL, NULL, VIR_FROM_NONE, code, VIR_ERR_ERROR,
NULL, NULL, NULL, -1, -1, "%s", errorMessage); NULL, NULL, NULL, -1, -1, "%s", errorMessage);
} }
@ -112,7 +115,7 @@ _virExec(virConnectPtr conn,
int pipeerr[2] = {-1,-1}; int pipeerr[2] = {-1,-1};
if ((null = open(_PATH_DEVNULL, O_RDONLY)) < 0) { if ((null = open(_PATH_DEVNULL, O_RDONLY)) < 0) {
ReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, ReportError(conn, VIR_ERR_INTERNAL_ERROR,
_("cannot open %s: %s"), _("cannot open %s: %s"),
_PATH_DEVNULL, strerror(errno)); _PATH_DEVNULL, strerror(errno));
goto cleanup; goto cleanup;
@ -120,13 +123,41 @@ _virExec(virConnectPtr conn,
if ((outfd != NULL && pipe(pipeout) < 0) || if ((outfd != NULL && pipe(pipeout) < 0) ||
(errfd != NULL && pipe(pipeerr) < 0)) { (errfd != NULL && pipe(pipeerr) < 0)) {
ReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, ReportError(conn, VIR_ERR_INTERNAL_ERROR,
_("cannot create pipe: %s"), strerror(errno)); _("cannot create pipe: %s"), strerror(errno));
goto cleanup; goto cleanup;
} }
if (outfd) {
if(non_block &&
virSetNonBlock(pipeout[0]) == -1) {
ReportError(conn, VIR_ERR_INTERNAL_ERROR,
_("Failed to set non-blocking file descriptor flag"));
goto cleanup;
}
if(virSetCloseExec(pipeout[0]) == -1) {
ReportError(conn, VIR_ERR_INTERNAL_ERROR,
_("Failed to set close-on-exec file descriptor flag"));
goto cleanup;
}
}
if (errfd) {
if(non_block &&
virSetNonBlock(pipeerr[0]) == -1) {
ReportError(conn, VIR_ERR_INTERNAL_ERROR,
_("Failed to set non-blocking file descriptor flag"));
goto cleanup;
}
if(virSetCloseExec(pipeerr[0]) == -1) {
ReportError(conn, VIR_ERR_INTERNAL_ERROR,
_("Failed to set close-on-exec file descriptor flag"));
goto cleanup;
}
}
if ((pid = fork()) < 0) { if ((pid = fork()) < 0) {
ReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, ReportError(conn, VIR_ERR_INTERNAL_ERROR,
_("cannot fork child process: %s"), strerror(errno)); _("cannot fork child process: %s"), strerror(errno));
goto cleanup; goto cleanup;
} }
@ -135,26 +166,10 @@ _virExec(virConnectPtr conn,
close(null); close(null);
if (outfd) { if (outfd) {
close(pipeout[1]); close(pipeout[1]);
if(non_block)
if(virSetNonBlock(pipeout[0]) == -1)
ReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
_("Failed to set non-blocking file descriptor flag"));
if(virSetCloseExec(pipeout[0]) == -1)
ReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
_("Failed to set close-on-exec file descriptor flag"));
*outfd = pipeout[0]; *outfd = pipeout[0];
} }
if (errfd) { if (errfd) {
close(pipeerr[1]); close(pipeerr[1]);
if(non_block)
if(virSetNonBlock(pipeerr[0]) == -1)
ReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
_("Failed to set non-blocking file descriptor flag"));
if(virSetCloseExec(pipeerr[0]) == -1)
ReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
_("Failed to set close-on-exec file descriptor flag"));
*errfd = pipeerr[0]; *errfd = pipeerr[0];
} }
*retpid = pid; *retpid = pid;
@ -163,23 +178,47 @@ _virExec(virConnectPtr conn,
/* child */ /* child */
if (pipeout[0] > 0 && close(pipeout[0]) < 0) /* Don't want to report errors against this accidentally, so
_exit(1); just discard it */
if (pipeerr[0] > 0 && close(pipeerr[0]) < 0) conn = NULL;
_exit(1); /* Remove any error callback too, so errors in child now
get sent to stderr where they stand a fighting chance
of being seen / logged */
virSetErrorFunc(NULL, NULL);
if (dup2(infd >= 0 ? infd : null, STDIN_FILENO) < 0) if (pipeout[0] > 0)
close(pipeout[0]);
if (pipeerr[0] > 0)
close(pipeerr[0]);
if (dup2(infd >= 0 ? infd : null, STDIN_FILENO) < 0) {
ReportError(conn, VIR_ERR_INTERNAL_ERROR,
_("failed to setup stdin file handle: %s"), strerror(errno));
_exit(1); _exit(1);
}
#ifndef ENABLE_DEBUG #ifndef ENABLE_DEBUG
if (dup2(pipeout[1] > 0 ? pipeout[1] : null, STDOUT_FILENO) < 0) if (dup2(pipeout[1] > 0 ? pipeout[1] : null, STDOUT_FILENO) < 0) {
ReportError(conn, VIR_ERR_INTERNAL_ERROR,
_("failed to setup stdout file handle: %s"), strerror(errno));
_exit(1); _exit(1);
if (dup2(pipeerr[1] > 0 ? pipeerr[1] : null, STDERR_FILENO) < 0) }
if (dup2(pipeerr[1] > 0 ? pipeerr[1] : null, STDERR_FILENO) < 0) {
ReportError(conn, VIR_ERR_INTERNAL_ERROR,
_("failed to setup stderr file handle: %s"), strerror(errno));
_exit(1); _exit(1);
}
#else /* ENABLE_DEBUG */ #else /* ENABLE_DEBUG */
if (pipeout[1] > 0 && dup2(pipeout[1], STDOUT_FILENO) < 0) if (pipeout[1] > 0 && dup2(pipeout[1], STDOUT_FILENO) < 0) {
ReportError(conn, VIR_ERR_INTERNAL_ERROR,
_("failed to setup stderr file handle: %s"), strerror(errno));
_exit(1); _exit(1);
if (pipeerr[1] > 0 && dup2(pipeerr[1], STDERR_FILENO) < 0) }
if (pipeerr[1] > 0 && dup2(pipeerr[1], STDERR_FILENO) < 0) {
ReportError(conn, VIR_ERR_INTERNAL_ERROR,
_("failed to setup stdout file handle: %s"), strerror(errno));
_exit(1); _exit(1);
}
#endif /* ENABLE_DEBUG */ #endif /* ENABLE_DEBUG */
close(null); close(null);
@ -190,11 +229,21 @@ _virExec(virConnectPtr conn,
execvp(argv[0], (char **) argv); execvp(argv[0], (char **) argv);
ReportError(conn, VIR_ERR_INTERNAL_ERROR,
_("cannot execute binary '%s': %s"),
argv[0], strerror(errno));
_exit(1); _exit(1);
return 0; return 0;
cleanup: cleanup:
/* This is cleanup of parent process only - child
should never jump here on error */
/* NB we don't ReportError() on any failures here
because the code which jumped hre already raised
an error condition which we must not overwrite */
if (pipeerr[0] > 0) if (pipeerr[0] > 0)
close(pipeerr[0]); close(pipeerr[0]);
if (pipeerr[1] > 0) if (pipeerr[1] > 0)
@ -249,12 +298,24 @@ virRun(virConnectPtr conn,
return ret; return ret;
while ((ret = waitpid(childpid, &exitstatus, 0) == -1) && errno == EINTR); while ((ret = waitpid(childpid, &exitstatus, 0) == -1) && errno == EINTR);
if (ret == -1) if (ret == -1) {
ReportError(conn, VIR_ERR_INTERNAL_ERROR,
_("cannot wait for '%s': %s"),
argv[0], strerror(errno));
return -1; return -1;
}
if (status == NULL) { if (status == NULL) {
errno = EINVAL; errno = EINVAL;
return (WIFEXITED(exitstatus) && WEXITSTATUS(exitstatus) == 0) ? 0 : -1; if (WIFEXITED(exitstatus) && WEXITSTATUS(exitstatus) == 0)
return 0;
ReportError(conn, VIR_ERR_INTERNAL_ERROR,
_("%s exited with non-zero status %d and signal %d"),
argv[0],
WIFEXITED(exitstatus) ? WEXITSTATUS(exitstatus) : 0,
WIFSIGNALED(exitstatus) ? WTERMSIG(exitstatus) : 0);
return -1;
} else { } else {
*status = exitstatus; *status = exitstatus;
return 0; return 0;
@ -271,7 +332,7 @@ virExec(virConnectPtr conn,
int *outfd ATTRIBUTE_UNUSED, int *outfd ATTRIBUTE_UNUSED,
int *errfd ATTRIBUTE_UNUSED) int *errfd ATTRIBUTE_UNUSED)
{ {
ReportError (conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, __FUNCTION__); ReportError (conn, VIR_ERR_INTERNAL_ERROR, __FUNCTION__);
return -1; return -1;
} }
@ -283,7 +344,7 @@ virExecNonBlock(virConnectPtr conn,
int *outfd ATTRIBUTE_UNUSED, int *outfd ATTRIBUTE_UNUSED,
int *errfd ATTRIBUTE_UNUSED) int *errfd ATTRIBUTE_UNUSED)
{ {
ReportError (conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, __FUNCTION__); ReportError (conn, VIR_ERR_INTERNAL_ERROR, __FUNCTION__);
return -1; return -1;
} }