Re: [Xen-devel] [PATCH 31/31] libxl: log bootloader output

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2012-04-10 at 20:08 +0100, Ian Jackson wrote:
> This involves adding a new log feature to libxl__datacopier, and then
> using it.
> 
> If the bootloader exits nonzero we print the log filename in a log
> message from libxl.
> 
> Signed-off-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>

This has been an annoying omission for ages, thanks!

Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>

> ---
>  tools/libxl/libxl_aoutils.c    |   10 ++++++++++
>  tools/libxl/libxl_bootloader.c |   24 ++++++++++++++++++++++++
>  tools/libxl/libxl_internal.h   |    3 ++-
>  3 files changed, 36 insertions(+), 1 deletions(-)
> 
> diff --git a/tools/libxl/libxl_aoutils.c b/tools/libxl/libxl_aoutils.c
> index b33abe4..1b17c84 100644
> --- a/tools/libxl/libxl_aoutils.c
> +++ b/tools/libxl/libxl_aoutils.c
> @@ -118,6 +118,16 @@ static void datacopier_readable(libxl__egc *egc, 
> libxl__ev_fd *ev,
>              libxl__ev_fd_deregister(gc, &dc->toread);
>              break;
>          }
> +        if (dc->log) {
> +            int wrote = fwrite(buf->buf + buf->used, 1, r, dc->log);
> +            if (wrote != r) {
> +                assert(ferror(dc->log));
> +                assert(errno);
> +                LOGE(ERROR, "error logging %s", dc->copywhat);
> +                datacopier_callback(egc, dc, 0, errno);
> +                return;
> +            }
> +        }
>          buf->used += r;
>          dc->used += r;
>          assert(buf->used <= sizeof(buf->buf));
> diff --git a/tools/libxl/libxl_bootloader.c b/tools/libxl/libxl_bootloader.c
> index 1361117..47ced8a 100644
> --- a/tools/libxl/libxl_bootloader.c
> +++ b/tools/libxl/libxl_bootloader.c
> @@ -234,6 +234,10 @@ static void bootloader_cleanup(libxl__egc *egc, 
> libxl__bootloader_state *bl)
>          libxl__carefd_close(bl->ptys[i].master);
>          libxl__carefd_close(bl->ptys[i].slave);
>      }
> +    if (bl->display.log) {
> +        fclose(bl->display.log);
> +        bl->display.log = NULL;
> +    }
>  }
>  
>  static void bootloader_setpaths(libxl__gc *gc, libxl__bootloader_state *bl)
> @@ -256,6 +260,8 @@ void libxl__bootloader_run(libxl__egc *egc, 
> libxl__bootloader_state *bl)
>  {
>      STATE_AO_GC(bl);
>      libxl_domain_build_info *info = bl->info;
> +    uint32_t domid = bl->domid;
> +    char *logfile_tmp = NULL;
>      int rc, r;
>  
>      libxl__bootloader_init(bl);
> @@ -268,6 +274,22 @@ void libxl__bootloader_run(libxl__egc *egc, 
> libxl__bootloader_state *bl)
>  
>      bootloader_setpaths(gc, bl);
>  
> +    const char *logfile_leaf = GCSPRINTF("bootloader.%"PRIu32, domid);
> +    rc = libxl_create_logfile(CTX, logfile_leaf, &logfile_tmp);
> +    if (rc) goto out;
> +
> +    /* Transfer ownership of log filename to bl and the gc */
> +    bl->logfile = logfile_tmp;
> +    libxl__ptr_add(gc, logfile_tmp);
> +    logfile_tmp = NULL;
> +
> +    bl->display.log = fopen(bl->logfile, "a");
> +    if (!bl->display.log) {
> +        LOGE(ERROR, "failed to create bootloader logfile %s", bl->logfile);
> +        rc = ERROR_FAIL;
> +        goto out;
> +    }
> +
>      for (;;) {
>          r = mkdir(bl->outputdir, 0600);
>          if (!r) break;
> @@ -305,6 +327,7 @@ void libxl__bootloader_run(libxl__egc *egc, 
> libxl__bootloader_state *bl)
>      return;
>  
>   out:
> +    free(logfile_tmp);
>      assert(rc);
>      bootloader_callback(egc, bl, rc);
>  }
> @@ -462,6 +485,7 @@ static void bootloader_finished(libxl__egc *egc, 
> libxl__ev_child *child,
>      libxl__datacopier_kill(&bl->display);
>  
>      if (status) {
> +        LOG(ERROR, "bootloader failed - consult logfile %s", bl->logfile);
>          libxl_report_child_exitstatus(CTX, XTL_ERROR, "bootloader",
>                                        pid, status);
>          rc = ERROR_FAIL;
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index fd96b31..c846144 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -1485,6 +1485,7 @@ struct libxl__datacopier_state {
>      int readfd, writefd;
>      ssize_t maxsz;
>      const char *copywhat, *readwhat, *writewhat; /* for error msgs */
> +    FILE *log; /* gets a copy of everything */
>      libxl__datacopier_callback *callback;
>      /* remaining fields are private to datacopier */
>      libxl__ev_fd toread, towrite;
> @@ -1547,7 +1548,7 @@ struct libxl__bootloader_state {
>      libxl_device_disk *disk;
>      uint32_t domid;
>      /* private to libxl__run_bootloader */
> -    char *outputpath, *outputdir;
> +    char *outputpath, *outputdir, *logfile;
>      char *diskpath; /* not from gc, represents actually attached disk */
>      libxl__openpty_state openpty;
>      libxl__openpty_result ptys[2];  /* [0] is for bootloader */



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel