Re: [PATCH 01/35] libxl: add LIBXL_LOGD_* and LOG*D function families.

[Wei Liu <wei.liu2@citrix.com>]

On Tue, Nov 15, 2016 at 11:18:39AM +0100, Cédric Bosdonnat wrote:
> From: Cédric Bosdonnat <cedric.bosdonnat@free.fr>
> 
> These functions should be used to log messages when the domain
> id is known. libxl__log will now prepend the log message by
> "Domain %PRIu32:" if the domain id is a valid one.
> 
> This aims at helping consumers filter logs on domain IDs.
> 
> Signed-off-by: Cédric Bosdonnat <cbosdonnat@suse.com>
> ---
>  tools/libxl/libxl_event.c    |  6 +++---
>  tools/libxl/libxl_internal.c | 16 ++++++++++------
>  tools/libxl/libxl_internal.h | 38 +++++++++++++++++++++++++++++++-------
>  3 files changed, 44 insertions(+), 16 deletions(-)
> 
> diff --git a/tools/libxl/libxl_event.c b/tools/libxl/libxl_event.c
> index 02b39e6..fc9bdc9 100644
> --- a/tools/libxl/libxl_event.c
> +++ b/tools/libxl/libxl_event.c
> @@ -1362,7 +1362,7 @@ void libxl__event_disaster(libxl__egc *egc, const char *msg, int errnoval,
>  {
>      EGC_GC;
>  
> -    libxl__log(CTX, XTL_CRITICAL, errnoval, file, line, func,
> +    libxl__log(CTX, XTL_CRITICAL, errnoval, file, line, func, INVALID_DOMID,
>                 "DISASTER in event loop: %s%s%s%s",
>                 msg,
>                 type ? " (relates to event type " : "",
> @@ -1943,7 +1943,7 @@ libxl__ao *libxl__ao_create(libxl_ctx *ctx, uint32_t domid,
>          ao->poller = libxl__poller_get(&ao->gc);
>          if (!ao->poller) goto out;
>      }
> -    libxl__log(ctx,XTL_DEBUG,-1,file,line,func,
> +    libxl__log(ctx,XTL_DEBUG,-1,file,line,func,domid,
>                 "ao %p: create: how=%p callback=%p poller=%p",
>                 ao, how, ao->how.callback, ao->poller);
>  
> @@ -1968,7 +1968,7 @@ int libxl__ao_inprogress(libxl__ao *ao,
>      assert(ao->in_initiator);
>      ao->constructing = 0;
>  
> -    libxl__log(CTX,XTL_DEBUG,-1,file,line,func,
> +    libxl__log(CTX,XTL_DEBUG,-1,file,line,func,INVALID_DOMID,

You should be able to use ao->domid here, right?

>                 "ao %p: inprogress: poller=%p, flags=%s%s%s%s",
>                 ao, ao->poller,
>                 ao->constructing ? "o" : "",
> diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c
> index 448dd61..78ed2f6 100644
> --- a/tools/libxl/libxl_internal.c
> +++ b/tools/libxl/libxl_internal.c
> @@ -22,7 +22,7 @@ void libxl__alloc_failed(libxl_ctx *ctx, const char *func,
>  #define M "libxl: FATAL ERROR: memory allocation failure"
>  #define L (size ? M " (%s, %lu x %lu)\n" : M " (%s)\n"), \
>            func, (unsigned long)nmemb, (unsigned long)size
> -    libxl__log(ctx, XTL_CRITICAL, ENOMEM, 0,0, func, L);
> +    libxl__log(ctx, XTL_CRITICAL, ENOMEM, 0,0, func, INVALID_DOMID, L);
>      fprintf(stderr, L);
>      fflush(stderr);
>      _exit(-1);
> @@ -202,7 +202,7 @@ char *libxl__dirname(libxl__gc *gc, const char *s)
>  
>  void libxl__logv(libxl_ctx *ctx, xentoollog_level msglevel, int errnoval,
>               const char *file, int line, const char *func,
> -             const char *fmt, va_list ap)
> +             uint32_t domid, const char *fmt, va_list ap)
>  {
>      /* WARNING this function may not call any libxl-provided
>       * memory allocation function, as those may
> @@ -211,6 +211,7 @@ void libxl__logv(libxl_ctx *ctx, xentoollog_level msglevel, int errnoval,
>      char *base = NULL;
>      int rc, esave;
>      char fileline[256];
> +    char domain[256];
>  
>      esave = errno;
>  
> @@ -221,22 +222,25 @@ void libxl__logv(libxl_ctx *ctx, xentoollog_level msglevel, int errnoval,
>      if (file) snprintf(fileline, sizeof(fileline), "%s:%d",file,line);
>      fileline[sizeof(fileline)-1] = 0;
>  
> +    domain[0] = 0;
> +    if (domid != INVALID_DOMID) snprintf(domain, sizeof(domain), "Domain %"PRIu32":", domid);

Coding style issue, please put snprintf in a new line.

> +    domain[sizeof(domain)-1] = 0;

This probably does do what you want it to do - sizeof(domain) -> 256.  I
don't think you need this at all because IIRC snprintf puts a \0 at the
end.

Overall I think this is in line with what we discussed before.

Wei.

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