mirror of https://gitee.com/openkylin/libvirt.git
command: properly diagnose process exit via signal
Child processes don't always reach _exit(); if they die from a signal, then any messages should still be accurate. Most users either expect a 0 status (thankfully, if status==0, then WIFEXITED(status) is true and WEXITSTATUS(status)==0 for all known platforms) or were filtering on WIFEXITED before printing a status, but a few were missing this check. Additionally, nwfilter_ebiptables_driver was making an assumption that works on Linux (where WEXITSTATUS shifts and WTERMSIG just masks) but fails on other platforms (where WEXITSTATUS just masks and WTERMSIG shifts). * src/util/command.h (virCommandTranslateStatus): New helper. * src/libvirt_private.syms (command.h): Export it. * src/util/command.c (virCommandTranslateStatus): New function. (virCommandWait): Use it to also diagnose status from signals. * src/security/security_apparmor.c (load_profile): Likewise. * src/storage/storage_backend.c (virStorageBackendQEMUImgBackingFormat): Likewise. * src/util/util.c (virExecDaemonize, virRunWithHook) (virFileOperation, virDirCreate): Likewise. * daemon/remote.c (remoteDispatchAuthPolkit): Likewise. * src/nwfilter/nwfilter_ebiptables_driver.c (ebiptablesExecCLI): Likewise.
This commit is contained in:
parent
6ef1f6c2a1
commit
208a044a54
|
@ -60,6 +60,7 @@
|
|||
#include "uuid.h"
|
||||
#include "network.h"
|
||||
#include "libvirt/libvirt-qemu.h"
|
||||
#include "command.h"
|
||||
|
||||
#define VIR_FROM_THIS VIR_FROM_REMOTE
|
||||
#define REMOTE_DEBUG(fmt, ...) VIR_DEBUG(fmt, __VA_ARGS__)
|
||||
|
@ -4368,8 +4369,10 @@ remoteDispatchAuthPolkit (struct qemud_server *server,
|
|||
goto authfail;
|
||||
}
|
||||
if (status != 0) {
|
||||
VIR_ERROR(_("Policy kit denied action %s from pid %d, uid %d, result: %d"),
|
||||
action, callerPid, callerUid, status);
|
||||
char *tmp = virCommandTranslateStatus(status);
|
||||
VIR_ERROR(_("Policy kit denied action %s from pid %d, uid %d: %s"),
|
||||
action, callerPid, callerUid, NULLSTR(tmp));
|
||||
VIR_FREE(tmp);
|
||||
goto authdeny;
|
||||
}
|
||||
PROBE(CLIENT_AUTH_ALLOW, "fd=%d, auth=%d, username=%s",
|
||||
|
|
|
@ -123,6 +123,7 @@ virCommandSetPreExecHook;
|
|||
virCommandSetWorkingDirectory;
|
||||
virCommandToString;
|
||||
virCommandTransferFD;
|
||||
virCommandTranslateStatus;
|
||||
virCommandWait;
|
||||
virCommandWriteArgLog;
|
||||
|
||||
|
|
|
@ -1,6 +1,7 @@
|
|||
/*
|
||||
* nwfilter_ebiptables_driver.c: driver for ebtables/iptables on tap devices
|
||||
*
|
||||
* Copyright (C) 2011 Red Hat, Inc.
|
||||
* Copyright (C) 2010 IBM Corp.
|
||||
* Copyright (C) 2010 Stefan Berger
|
||||
*
|
||||
|
@ -2558,7 +2559,7 @@ err_exit:
|
|||
* ebiptablesExecCLI:
|
||||
* @buf : pointer to virBuffer containing the string with the commands to
|
||||
* execute.
|
||||
* @status: Pointer to an integer for returning the status of the
|
||||
* @status: Pointer to an integer for returning the WEXITSTATUS of the
|
||||
* commands executed via the script the was run.
|
||||
*
|
||||
* Returns 0 in case of success, != 0 in case of an error. The returned
|
||||
|
@ -2587,7 +2588,7 @@ ebiptablesExecCLI(virBufferPtr buf,
|
|||
|
||||
cmds = virBufferContentAndReset(buf);
|
||||
|
||||
VIR_DEBUG("%s", cmds);
|
||||
VIR_DEBUG("%s", NULLSTR(cmds));
|
||||
|
||||
if (!cmds)
|
||||
return 0;
|
||||
|
@ -2606,9 +2607,16 @@ ebiptablesExecCLI(virBufferPtr buf,
|
|||
|
||||
virMutexUnlock(&execCLIMutex);
|
||||
|
||||
*status >>= 8;
|
||||
if (rc == 0) {
|
||||
if (WIFEXITED(*status)) {
|
||||
*status = WEXITSTATUS(*status);
|
||||
} else {
|
||||
rc = -1;
|
||||
*status = 1;
|
||||
}
|
||||
}
|
||||
|
||||
VIR_DEBUG("rc = %d, status = %d",rc, *status);
|
||||
VIR_DEBUG("rc = %d, status = %d", rc, *status);
|
||||
|
||||
unlink(filename);
|
||||
|
||||
|
|
|
@ -1,5 +1,6 @@
|
|||
/*
|
||||
* AppArmor security driver for libvirt
|
||||
* Copyright (C) 2011 Red Hat, Inc.
|
||||
* Copyright (C) 2009-2010 Canonical Ltd.
|
||||
*
|
||||
* This library is free software; you can redistribute it and/or
|
||||
|
@ -36,6 +37,7 @@
|
|||
#include "hostusb.h"
|
||||
#include "files.h"
|
||||
#include "configmake.h"
|
||||
#include "command.h"
|
||||
|
||||
#define VIR_FROM_THIS VIR_FROM_SECURITY
|
||||
#define SECURITY_APPARMOR_VOID_DOI "0"
|
||||
|
@ -217,15 +219,19 @@ load_profile(virSecurityManagerPtr mgr,
|
|||
VIR_FORCE_CLOSE(pipefd[1]);
|
||||
rc = 0;
|
||||
|
||||
rewait:
|
||||
if (waitpid(child, &status, 0) != child) {
|
||||
if (errno == EINTR)
|
||||
goto rewait;
|
||||
|
||||
while ((ret = waitpid(child, &status, 0)) < 0 && errno == EINTR);
|
||||
if (ret < 0) {
|
||||
virReportSystemError(errno,
|
||||
_("Failed to reap virt-aa-helper pid %lu"),
|
||||
(unsigned long)child);
|
||||
rc = -1;
|
||||
} else if (status) {
|
||||
char *str = virCommandTranslateStatus(status);
|
||||
virSecurityReportError(VIR_ERR_INTERNAL_ERROR,
|
||||
_("Unexpected exit status from virt-aa-helper "
|
||||
"%d pid %lu"),
|
||||
WEXITSTATUS(status), (unsigned long)child);
|
||||
_("Unexpected status from virt-aa-helper "
|
||||
"pid %lu: %s"),
|
||||
(unsigned long)child, NULLSTR(str));
|
||||
VIR_FREE(str);
|
||||
rc = -1;
|
||||
}
|
||||
|
||||
|
|
|
@ -56,6 +56,7 @@
|
|||
#include "storage_backend.h"
|
||||
#include "logging.h"
|
||||
#include "files.h"
|
||||
#include "command.h"
|
||||
|
||||
#if WITH_STORAGE_LVM
|
||||
# include "storage_backend_logical.h"
|
||||
|
@ -631,18 +632,13 @@ static int virStorageBackendQEMUImgBackingFormat(const char *qemuimg)
|
|||
cleanup:
|
||||
VIR_FREE(help);
|
||||
VIR_FORCE_CLOSE(newstdout);
|
||||
rewait:
|
||||
if (child) {
|
||||
if (waitpid(child, &status, 0) != child) {
|
||||
if (errno == EINTR)
|
||||
goto rewait;
|
||||
|
||||
VIR_ERROR(_("Unexpected exit status from qemu %d pid %lu"),
|
||||
WEXITSTATUS(status), (unsigned long)child);
|
||||
}
|
||||
if (WEXITSTATUS(status) != 0) {
|
||||
VIR_WARN("Unexpected exit status '%d', qemu probably failed",
|
||||
WEXITSTATUS(status));
|
||||
while (waitpid(child, &status, 0) == -1 && errno == EINTR);
|
||||
if (status) {
|
||||
tmp = virCommandTranslateStatus(status);
|
||||
VIR_WARN("Unexpected status, qemu probably failed: %s",
|
||||
NULLSTR(tmp));
|
||||
VIR_FREE(tmp);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
@ -797,6 +797,26 @@ virCommandToString(virCommandPtr cmd)
|
|||
}
|
||||
|
||||
|
||||
/*
|
||||
* Translate an exit status into a malloc'd string. Generic helper
|
||||
* for virCommandRun and virCommandWait status argument, as well as
|
||||
* raw waitpid and older virRun status.
|
||||
*/
|
||||
char *
|
||||
virCommandTranslateStatus(int status)
|
||||
{
|
||||
char *buf;
|
||||
if (WIFEXITED(status)) {
|
||||
virAsprintf(&buf, _("exit status %d"), WEXITSTATUS(status));
|
||||
} else if (WIFSIGNALED(status)) {
|
||||
virAsprintf(&buf, _("fatal signal %d"), WTERMSIG(status));
|
||||
} else {
|
||||
virAsprintf(&buf, _("invalid value %d"), status);
|
||||
}
|
||||
return buf;
|
||||
}
|
||||
|
||||
|
||||
/*
|
||||
* Manage input and output to the child process.
|
||||
*/
|
||||
|
@ -958,6 +978,7 @@ virCommandRun(virCommandPtr cmd, int *exitstatus)
|
|||
struct stat st;
|
||||
bool string_io;
|
||||
bool async_io = false;
|
||||
char *str;
|
||||
|
||||
if (!cmd ||cmd->has_error == ENOMEM) {
|
||||
virReportOOMError();
|
||||
|
@ -1048,9 +1069,14 @@ virCommandRun(virCommandPtr cmd, int *exitstatus)
|
|||
if (virCommandWait(cmd, exitstatus) < 0)
|
||||
ret = -1;
|
||||
|
||||
VIR_DEBUG("Result stdout: '%s' stderr: '%s'",
|
||||
str = (exitstatus ? virCommandTranslateStatus(*exitstatus)
|
||||
: (char *) "status 0");
|
||||
VIR_DEBUG("Result %s, stdout: '%s' stderr: '%s'",
|
||||
NULLSTR(str),
|
||||
cmd->outbuf ? NULLSTR(*cmd->outbuf) : "(null)",
|
||||
cmd->errbuf ? NULLSTR(*cmd->errbuf) : "(null)");
|
||||
if (exitstatus)
|
||||
VIR_FREE(str);
|
||||
|
||||
/* Reset any capturing, in case caller runs
|
||||
* this identical command again */
|
||||
|
@ -1230,10 +1256,12 @@ virCommandWait(virCommandPtr cmd, int *exitstatus)
|
|||
if (exitstatus == NULL) {
|
||||
if (status != 0) {
|
||||
char *str = virCommandToString(cmd);
|
||||
char *st = virCommandTranslateStatus(status);
|
||||
virCommandError(VIR_ERR_INTERNAL_ERROR,
|
||||
_("Child process (%s) exited with status %d."),
|
||||
str ? str : cmd->args[0], WEXITSTATUS(status));
|
||||
_("Child process (%s) status unexpected: %s"),
|
||||
str ? str : cmd->args[0], NULLSTR(st));
|
||||
VIR_FREE(str);
|
||||
VIR_FREE(st);
|
||||
return -1;
|
||||
}
|
||||
} else {
|
||||
|
|
|
@ -1,7 +1,7 @@
|
|||
/*
|
||||
* command.h: Child command execution
|
||||
*
|
||||
* Copyright (C) 2010 Red Hat, Inc.
|
||||
* Copyright (C) 2010-2011 Red Hat, Inc.
|
||||
*
|
||||
* This library is free software; you can redistribute it and/or
|
||||
* modify it under the terms of the GNU Lesser General Public
|
||||
|
@ -242,6 +242,12 @@ void virCommandWriteArgLog(virCommandPtr cmd,
|
|||
*/
|
||||
char *virCommandToString(virCommandPtr cmd) ATTRIBUTE_RETURN_CHECK;
|
||||
|
||||
|
||||
/*
|
||||
* Translate an exit status into a malloc'd string.
|
||||
*/
|
||||
char *virCommandTranslateStatus(int exitstatus) ATTRIBUTE_RETURN_CHECK;
|
||||
|
||||
/*
|
||||
* Run the command and wait for completion.
|
||||
* Returns -1 on any error executing the
|
||||
|
|
|
@ -76,6 +76,7 @@
|
|||
#include "threads.h"
|
||||
#include "verify.h"
|
||||
#include "files.h"
|
||||
#include "command.h"
|
||||
|
||||
#ifndef NSIG
|
||||
# define NSIG 32
|
||||
|
@ -832,9 +833,11 @@ int virExecDaemonize(const char *const*argv,
|
|||
errno == EINTR);
|
||||
|
||||
if (childstat != 0) {
|
||||
char *str = virCommandTranslateStatus(childstat);
|
||||
virUtilError(VIR_ERR_INTERNAL_ERROR,
|
||||
_("Intermediate daemon process exited with status %d."),
|
||||
WEXITSTATUS(childstat));
|
||||
_("Intermediate daemon process status unexpected: %s"),
|
||||
NULLSTR(str));
|
||||
VIR_FREE(str);
|
||||
ret = -2;
|
||||
}
|
||||
|
||||
|
@ -903,13 +906,12 @@ virRunWithHook(const char *const*argv,
|
|||
|
||||
if (status == NULL) {
|
||||
errno = EINVAL;
|
||||
if (WIFEXITED(exitstatus) && WEXITSTATUS(exitstatus) != 0) {
|
||||
if (exitstatus) {
|
||||
char *str = virCommandTranslateStatus(exitstatus);
|
||||
virUtilError(VIR_ERR_INTERNAL_ERROR,
|
||||
_("'%s' exited with non-zero status %d and "
|
||||
"signal %d: %s"), argv_str,
|
||||
WIFEXITED(exitstatus) ? WEXITSTATUS(exitstatus) : 0,
|
||||
WIFSIGNALED(exitstatus) ? WTERMSIG(exitstatus) : 0,
|
||||
(errbuf ? errbuf : ""));
|
||||
_("'%s' exited unexpectedly: %s"),
|
||||
argv_str, NULLSTR(str));
|
||||
VIR_FREE(str);
|
||||
goto error;
|
||||
}
|
||||
} else {
|
||||
|
@ -1510,8 +1512,7 @@ int virFileOperation(const char *path, int openflags, mode_t mode,
|
|||
path);
|
||||
goto parenterror;
|
||||
}
|
||||
ret = -WEXITSTATUS(status);
|
||||
if (!WIFEXITED(status) || (ret == -EACCES)) {
|
||||
if (!WIFEXITED(status) || (ret = -WEXITSTATUS(status)) == -EACCES) {
|
||||
/* fall back to the simpler method, which works better in
|
||||
* some cases */
|
||||
return virFileOperationNoFork(path, openflags, mode, uid, gid,
|
||||
|
@ -1630,15 +1631,11 @@ int virDirCreate(const char *path, mode_t mode,
|
|||
path);
|
||||
goto parenterror;
|
||||
}
|
||||
ret = -WEXITSTATUS(status);
|
||||
if (!WIFEXITED(status) || (ret == -EACCES)) {
|
||||
if (!WIFEXITED(status) || (ret = -WEXITSTATUS(status)) == -EACCES) {
|
||||
/* fall back to the simpler method, which works better in
|
||||
* some cases */
|
||||
return virDirCreateNoFork(path, mode, uid, gid, flags);
|
||||
}
|
||||
if (ret < 0) {
|
||||
goto parenterror;
|
||||
}
|
||||
parenterror:
|
||||
return ret;
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue