[libvirt] [PATCH 2/3] Improve LXC startup error reporting
Peter Krempa
pkrempa at redhat.com
Thu Mar 7 15:40:05 UTC 2013
On 03/06/13 17:16, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange at redhat.com>
>
> Currently we rely on a VIR_ERROR message being logged by the
> virRaiseError function to report LXC startup errors. This gives
> the right message, but is rather ugly and can be truncated
> if lots of log messages are written. Change the LXC controller
> to explicitly print any virErrorPtr message to stderr. Then
> change the driver to skip over anything that looks like a log
> message.
>
> The result is that this
>
> error: Failed to start domain busy
> error: internal error guest failed to start: 2013-03-04 19:46:42.846+0000: 1734: info : libvirt version: 1.0.2
> 2013-03-04 19:46:42.846+0000: 1734: error : virFileLoopDeviceAssociate:600 : Unable to open /root/disk.raw: No such file or directory
>
> changes to
>
> error: Failed to start domain busy
> error: internal error guest failed to start: Unable to open /root/disk.raw: No such file or directory
> Signed-off-by: Daniel P. Berrange <berrange at redhat.com>
> ---
> src/lxc/lxc_controller.c | 7 ++-
> src/lxc/lxc_process.c | 130 ++++++++++++++++++++++++++++++++---------------
> 2 files changed, 94 insertions(+), 43 deletions(-)
>
> diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
> index 15aa334..78e8a70 100644
> --- a/src/lxc/lxc_controller.c
> +++ b/src/lxc/lxc_controller.c
> @@ -1706,7 +1706,6 @@ int main(int argc, char *argv[])
> rc = virLXCControllerRun(ctrl);
>
> cleanup:
> - virPidFileDelete(LXC_STATE_DIR, name);
From the code later on, it doesn't seem you want retain the pid file.
If you do so, errors won't be reported later on ... [1]
> if (ctrl)
> virLXCControllerDeleteInterfaces(ctrl);
> for (i = 0 ; i < nttyFDs ; i++)
> @@ -1715,5 +1714,11 @@ cleanup:
>
> virLXCControllerFree(ctrl);
>
> + if (rc < 0) {
> + virErrorPtr err = virGetLastError();
> + if (err && err->message)
> + fprintf(stderr, "%s\n", err->message);
> + }
Possible errors from the cleanup steps above this code could shadow the
real error message here.
> +
> return rc < 0? EXIT_FAILURE : EXIT_SUCCESS;
> }
> diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
> index aaa81a7..6020f6f 100644
> --- a/src/lxc/lxc_process.c
> +++ b/src/lxc/lxc_process.c
> @@ -24,6 +24,7 @@
> #include <unistd.h>
> #include <fcntl.h>
> #include <signal.h>
> +#include <regex.h>
This doesn't seem to be used anywhere in this patch.
>
> #include "lxc_process.h"
> #include "lxc_domain.h"
> @@ -690,6 +691,11 @@ int virLXCProcessStop(virLXCDriverPtr driver,
>
> VIR_DEBUG("Stopping VM name=%s pid=%d reason=%d",
> vm->def->name, (int)vm->pid, (int)reason);
> + if (!virDomainObjIsActive(vm)) {
> + VIR_DEBUG("VM '%s' not active", vm->def->name);
> + return 0;
> + }
> +
> if (vm->pid <= 0) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> _("Invalid PID %d for container"), vm->pid);
> @@ -813,50 +819,27 @@ cleanup:
> return NULL;
> }
>
> +
> static int
> -virLXCProcessReadLogOutput(virDomainObjPtr vm,
> - char *logfile,
> - off_t pos,
> - char *buf,
> - size_t buflen)
> +virLXCProcessReadLogOutputData(virDomainObjPtr vm,
> + int fd,
> + char *buf,
> + size_t buflen)
> {
> - int fd;
> - off_t off;
> - int whence;
> - int got = 0, ret = -1;
> int retries = 10;
> + int got = 0;
> + int ret = -1;
> + char *filter_next = buf;
>
> - if ((fd = open(logfile, O_RDONLY)) < 0) {
> - virReportSystemError(errno, _("failed to open logfile %s"),
> - logfile);
> - goto cleanup;
> - }
> -
> - if (pos < 0) {
> - off = 0;
> - whence = SEEK_END;
> - } else {
> - off = pos;
> - whence = SEEK_SET;
> - }
> -
> - if (lseek(fd, off, whence) < 0) {
> - if (whence == SEEK_END)
> - virReportSystemError(errno,
> - _("unable to seek to end of log for %s"),
> - logfile);
> - else
> - virReportSystemError(errno,
> - _("unable to seek to %lld from start for %s"),
> - (long long)off, logfile);
> - goto cleanup;
> - }
> + buf[0] = '\0';
>
> while (retries) {
> ssize_t bytes;
> int isdead = 0;
> + char *eol;
>
> - if (kill(vm->pid, 0) == -1 && errno == ESRCH)
> + if (vm->pid <= 0 ||
> + (kill(vm->pid, 0) == -1 && errno == ESRCH))
> isdead = 1;
>
> /* Any failures should be detected before we read the log, so we
> @@ -864,28 +847,85 @@ virLXCProcessReadLogOutput(virDomainObjPtr vm,
> bytes = saferead(fd, buf+got, buflen-got-1);
> if (bytes < 0) {
> virReportSystemError(errno, "%s",
> - _("Failure while reading guest log output"));
> + _("Failure while reading log output"));
> goto cleanup;
> }
>
> got += bytes;
> buf[got] = '\0';
>
> - if ((got == buflen-1) || isdead) {
> - break;
> + /* Filter out debug messages from intermediate libvirt process */
> + while ((eol = strchr(filter_next, '\n'))) {
> + *eol = '\0';
> + if (virLogProbablyLogMessage(filter_next)) {
> + memmove(filter_next, eol + 1, got - (eol - buf));
> + got -= eol + 1 - filter_next;
> + } else {
> + filter_next = eol + 1;
> + *eol = '\n';
> + }
> + }
> +
> + if (got == buflen-1) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Out of space while reading log output: %s"),
> + buf);
> + goto cleanup;
> + }
> +
> + if (isdead) {
> + ret = got;
> + goto cleanup;
> }
>
> usleep(100*1000);
> retries--;
> }
>
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Timed out while reading log output: %s"),
> + buf);
>
> - ret = got;
> cleanup:
> + return ret;
> +}
> +
> +
> +static int
> +virLXCProcessReadLogOutput(virDomainObjPtr vm,
> + char *logfile,
> + off_t pos,
> + char *buf,
> + size_t buflen)
> +{
> + int fd = -1;
> + int ret;
> +
> + if ((fd = open(logfile, O_RDONLY)) < 0) {
> + virReportSystemError(errno,
> + _("Unable to open log file %s"),
> + logfile);
> + return -1;
> + }
> +
> + if (lseek(fd, pos, SEEK_SET) < 0) {
> + virReportSystemError(errno,
> + _("Unable to seek log file %s to %llu"),
> + logfile, (unsigned long long)pos);
> + VIR_FORCE_CLOSE(fd);
> + return -1;
> + }
> +
> + ret = virLXCProcessReadLogOutputData(vm,
> + fd,
> + buf,
> + buflen);
> +
> VIR_FORCE_CLOSE(fd);
> return ret;
> }
>
> +
> /**
> * virLXCProcessStart:
> * @conn: pointer to connection
> @@ -1124,9 +1164,15 @@ int virLXCProcessStart(virConnectPtr conn,
>
> /* And get its pid */
> if ((r = virPidFileRead(driver->stateDir, vm->def->name, &vm->pid)) < 0) {
[1] ... here.
> - virReportSystemError(-r,
> - _("Failed to read pid file %s/%s.pid"),
> - driver->stateDir, vm->def->name);
> + char out[1024];
> +
> + if (virLXCProcessReadLogOutput(vm, logfile, pos, out, 1024) > 0)
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("guest failed to start: %s"), out);
> + else
> + virReportSystemError(-r,
> + _("Failed to read pid file %s/%s.pid"),
> + driver->stateDir, vm->def->name);
> goto cleanup;
> }
>
>
Peter
More information about the libvir-list
mailing list