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

[Dr. David Alan Gilbert]

* Daniel P. Berrange (address@hidden) wrote:
> 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.

Not all uses of qemu trace are vast outputs; some of them are just
a handful per run (e.g. 'did we hit the ..... case' or 'did we fail before
or after the ....').  I'd agree that lossy logging systems are a pain;
I can see why you'd want to do this.

Dave

> 
> > 
> > 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 :|
> 
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK