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

Re: [libvirt] [PATCH 3/4] qemu: monitor: Produce better errors on monitor hangup



On 09/19/2013 11:23 AM, Peter Krempa wrote:
> Change the monitor error code to add the ability to access the qemu log
> file using a file descriptor so that we can dig in it for a more useful
> error message. The error is now logged on monitor hangups and overwrites
> a possible lesser error. A hangup on the monitor usualy means that qemu
> has crashed and there's a significant chance it produced a useful error
> message.
> 
> The functionality will be latent until the next patch.
> ---
>  src/qemu/qemu_monitor.c | 70 ++++++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 64 insertions(+), 6 deletions(-)
> 
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index c3701fe..5369975 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -32,6 +32,8 @@
>  #include "qemu_monitor.h"
>  #include "qemu_monitor_text.h"
>  #include "qemu_monitor_json.h"
> +#include "qemu_domain.h"
> +#include "qemu_process.h"
>  #include "virerror.h"
>  #include "viralloc.h"
>  #include "virlog.h"
> @@ -331,6 +333,37 @@ qemuMonitorOpenPty(const char *monitor)
>  }
> 
> 
> +/* Get a possible error from qemu's log. This function closes the
> + * corresponding log fd */
> +static char *
> +qemuMonitorGetErrorFromLog(qemuMonitorPtr mon)
> +{
> +    int len;
> +    char *logbuf = NULL;
> +    int orig_errno = errno;
> +
> +    if (mon->logfd < 0)
> +        return NULL;
> +
> +    if (VIR_ALLOC_N(logbuf, 4096) < 0)
> +        goto error;
> +
> +    if ((len = qemuProcessReadLog(mon->logfd, logbuf, 4096 - 1, 0, true)) <= 0)
> +        goto error;
> +
> +    errno = orig_errno;
> +    VIR_FORCE_CLOSE(mon->logfd);
> +    return logbuf;
> +
> +error:
> +    virResetLastError();

What error is this resetting? qemuProcessReadLog doesn't set any and you can
use VIR_ALLOC_N_QUIET for the other one.

> +    VIR_FREE(logbuf);
> +    VIR_FORCE_CLOSE(mon->logfd);
> +    errno = orig_errno;
> +    return NULL;
> +}
> +
> +
>  /* This method processes data that has been received
>   * from the monitor. Looking for async events and
>   * replies/errors.
> @@ -564,6 +597,7 @@ qemuMonitorIO(int watch, int fd, int events, void *opaque) {
>      qemuMonitorPtr mon = opaque;
>      bool error = false;
>      bool eof = false;
> +    bool hangup = false;
> 
>      virObjectRef(mon);
> 
> @@ -614,12 +648,14 @@ qemuMonitorIO(int watch, int fd, int events, void *opaque) {
>              }
>          }
> 
> -        if (!error &&
> -            events & VIR_EVENT_HANDLE_HANGUP) {
> -            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> -                           _("End of file from monitor"));
> -            eof = true;
> -            events &= ~VIR_EVENT_HANDLE_HANGUP;
> +        if (events & VIR_EVENT_HANDLE_HANGUP) {
> +            hangup = true;
> +            if (!error) {
> +                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                               _("End of file from monitor"));
> +                eof = true;
> +                events &= ~VIR_EVENT_HANDLE_HANGUP;
> +            }
>          }
> 
>          if (!error && !eof &&
> @@ -638,6 +674,28 @@ qemuMonitorIO(int watch, int fd, int events, void *opaque) {
>      }
> 
>      if (error || eof) {
> +        if (hangup) {
> +            /* Check if an error message from qemu is available and if so, use
> +             * it to overwrite the actual message. It's done only on early

*in

> +             * startup phases where the message from qemu is certailny more

*certainly

> +             * interresting than a "connection reset by peer" message.

*interesting

> +             */
> +            char *qemuMessage;
> +
> +            if ((qemuMessage = qemuMonitorGetErrorFromLog(mon)) &&
> +                strlen(qemuMessage) > 0) {

qemuMonitorGetErrorFromLog returns NULL when nothing was read, strlen should
not be necessary.

> +
> +                virReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("early end of file from monitor: "
> +                                 "possible problem:\n%s"),
> +                               qemuMessage);
> +                virCopyLastError(&mon->lastError);
> +                virResetLastError();
> +            }
> +
> +            VIR_FREE(qemuMessage);
> +        }
> +
>          if (mon->lastError.code != VIR_ERR_OK) {
>              /* Already have an error, so clear any new error */
>              virResetLastError();
> 

Jan


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