Re: [Qemu-devel] [PATCH] trace: add syslog tracing backend

[Daniel P. Berrange]

On Tue, Aug 02, 2016 at 04:06:42PM +0100, Paul Durrant wrote:
> This patch adds a tracing backend which sends output using syslog().
> The syslog backend is limited to POSIX compliant systems.
> 
> openlog() is called with facility set to LOG_DAEMON, with the LOG_PID
> option. Trace events are logged at level LOG_INFO.

I'm not entirely convinced that sending trace output to syslog
is a great idea. Syslog is really for important system messages
at low/moderate volumes, while the QEMU trace feature is really
adhoc developer debugging at potentially huge message volume.
Many syslog impls will rate limit and either drop or merge messages
from the client. IMHO this makes syslog pretty undesirable as a
tracing backend in general.

> 
> Signed-off-by: Paul Durrant <address@hidden>
> Cc: Stefan Hajnoczi <address@hidden>
> ---
>  configure                           | 19 ++++++++++++++++
>  scripts/tracetool/backend/syslog.py | 45 
> +++++++++++++++++++++++++++++++++++++
>  trace/control.c                     |  7 ++++++
>  3 files changed, 71 insertions(+)
>  create mode 100644 scripts/tracetool/backend/syslog.py
> 
> diff --git a/configure b/configure
> index 879324b..fce00b8 100755
> --- a/configure
> +++ b/configure
> @@ -4189,6 +4189,18 @@ if compile_prog "" "" ; then
>  fi
>  
>  ##########################################
> +# check if we have posix_syslog
> +
> +posix_syslog=no
> +cat > $TMPC << EOF
> +#include <syslog.h>
> +int main(void) { openlog("qemu", LOG_PID, LOG_DAEMON); syslog(LOG_INFO, 
> "configure"); return 0; }
> +EOF
> +if compile_prog "" "" ; then
> +    posix_syslog=yes
> +fi
> +
> +##########################################
>  # check if trace backend exists
>  
>  $python "$source_path/scripts/tracetool.py" "--backends=$trace_backends" 
> --check-backends  > /dev/null 2> /dev/null
> @@ -5456,6 +5468,13 @@ if have_backend "ftrace"; then
>      feature_not_found "ftrace(trace backend)" "ftrace requires Linux"
>    fi
>  fi
> +if have_backend "syslog"; then
> +  if test "$posix_syslog" = "yes" ; then
> +    echo "CONFIG_TRACE_SYSLOG=y" >> $config_host_mak
> +  else
> +    feature_not_found "syslog(trace backend)" "syslog not available"
> +  fi
> +fi
>  echo "CONFIG_TRACE_FILE=$trace_file" >> $config_host_mak
>  
>  if test "$rdma" = "yes" ; then
> diff --git a/scripts/tracetool/backend/syslog.py 
> b/scripts/tracetool/backend/syslog.py
> new file mode 100644
> index 0000000..2668947
> --- /dev/null
> +++ b/scripts/tracetool/backend/syslog.py
> @@ -0,0 +1,45 @@
> +#!/usr/bin/env python
> +# -*- coding: utf-8 -*-
> +
> +"""
> +Syslog built-in backend.
> +"""
> +
> +__author__     = "Paul Durrant <address@hidden>"
> +__copyright__  = "Copyright 2016, Citrix Systems Inc."
> +__license__    = "GPL version 2 or (at your option) any later version"
> +
> +__maintainer__ = "Stefan Hajnoczi"
> +__email__      = "address@hidden"
> +
> +
> +from tracetool import out
> +
> +
> +PUBLIC = True
> +
> +
> +def generate_h_begin(events):
> +    out('#include "trace/control.h"',
> +        '#include <syslog.h>',
> +        '')
> +
> +
> +def generate_h(event):
> +    argnames = ", ".join(event.args.names())
> +    if len(event.args) > 0:
> +        argnames = ", " + argnames
> +
> +    if "vcpu" in event.properties:
> +        # already checked on the generic format code
> +        cond = "true"
> +    else:
> +        cond = "trace_event_get_state(%s)" % ("TRACE_" + event.name.upper())
> +
> +    out('        if (%(cond)s) {',
> +        '            syslog(LOG_INFO, "%(name)s " %(fmt)s %(argnames)s);',
> +        '        }',
> +        cond=cond,
> +        name=event.name,
> +        fmt=event.fmt.rstrip("\n"),
> +        argnames=argnames)
> diff --git a/trace/control.c b/trace/control.c
> index d173c09..b179cde 100644
> --- a/trace/control.c
> +++ b/trace/control.c
> @@ -19,6 +19,9 @@
>  #ifdef CONFIG_TRACE_LOG
>  #include "qemu/log.h"
>  #endif
> +#ifdef CONFIG_TRACE_SYSLOG
> +#include <syslog.h>
> +#endif
>  #include "qapi/error.h"
>  #include "qemu/error-report.h"
>  #include "qemu/config-file.h"
> @@ -250,6 +253,10 @@ bool trace_init_backends(void)
>      }
>  #endif
>  
> +#ifdef CONFIG_TRACE_SYSLOG
> +    openlog(NULL, LOG_PID, LOG_DAEMON);
> +#endif
> +
>      return true;
>  }
>  
> -- 
> 2.1.4
> 
> 

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|