Re: [spice-common PATCH v1 0/13] subdomain filtering with glib logging

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

 



Hi,

On Fri, Dec 11, 2015 at 11:45:32AM -0500, Marc-André Lureau wrote:
> Hi
>
> ----- Original Message -----
> > Hi,
> >
> > So, I've been playing a bit with this domains for logging in top of
> > Christophe's patches [0] to use glib log system and this series is an
> > approach for that, I would love some feedback of this WIP.
> >
> > [0]
> > http://lists.freedesktop.org/archives/spice-devel/2015-December/024507.html
> >
> > So, instead of working with domains for glib, I thought about having one
> > domain for all, which is Spice, and handle the interesting logs by
> > subdomains.
> >
> > The subdomains can be filtered in/out also by level under SPICE_DEBUG
> > environment var such as:
> >
> > # Give all output from all subdomains
> > export SPICE_DEBUG=*:*
> > export SPICE_DEBUG=1
> >
> > # Give all output from audio, warning level for everything else
> > export SPICE_DEBUG=*:warning,audio=*
> >
> > # Give all from usb and usbredir and nothing else
> > export SPICE_DEBUG=usb=*,usbredir=*,*:none
>
> This really look overly complex and intrusive to me. This is
> something different from glib or qemu tracing, so it makes me feel
> quite dizzy...

Right, I really don't want to introduce something that is bad :)

Do you have any idea regarding the syntax so we could have it in a
simpler way?

Currently is a set of <domain>:<debug-level> separated by comma.

>
> But I should review your patches, will try to do that soon...

Many thanks!

>
> >
> > As for G_MESSAGES_DEBUG concern, if SPICE_DEBUG is set, common/log.c
> > will set always set Spice domain so the only env var to Spice is the
> > SPICE_DEBUG one.
> > 
> > Every channel got is own subdomain but other files tend to be grouped,
> > such as the case of spice-audio, spice-gstaudio and spice-pulse which
> > are under "audio" subdomain. Same for "usb" domain which has 4 files
> > usb-(device-manager, device-widget), win-usb-(dev, driver-install).
> > 
> > Default subdomain is log.
> > 
> > Seems that in other projects, the domain initialization is on its _init
> > function (like, spice_init()). We could have that, If it makes sense
> > otherwise what I did was initializing on the very first spice_log() and
> > memory of each domain should be freed by _finalize() of its object.
> > 
> > This is probably leaking right now with remote-viewer but I don't want
> > to dive into fixing things without the general idea being accepted. In
> > the case of this looks good, I believe some set of simple tests would
> > be nice to have as well.
> > 
> > PS: This was only tested with spice-gtk, mostly likely will break on
> > spice-server due the change on SPICE_LOG_DOMAIN type
> > 
> > PS2: There are some leftovers like printf, fprintf, VNC_DEBUG, ... that
> > I think we should remove as well.
> > 
> > Cheers,
> >   toso
> > 
> > 
> > Victor Toso (4):
> >   log: simplify log defines with SPICE_LOG
> >   log: include message log level for parity with glib
> >   log: Always use Spice domain for glib
> >   log: allow filtering debug using domains
> > 
> >  common/Makefile.am |   1 +
> >  common/log.c       | 245
> >  ++++++++++++++++++++++++++++++++++++++++++++++++++---
> >  common/log.h       |  85 +++++++++++++------
> >  3 files changed, 295 insertions(+), 36 deletions(-)
> > 
> > Victor Toso (9):
> >   log: use glib debug on testing tools
> >   log: use spice_debug instead of SPICE_DEBUG
> >   log: remove unused SPICE_DEBUG
> >   log: use spice_debug instead of g_debug
> >   log: use spice_warning instead of g_warning
> >   log: use spice_critical instead of g_critical
> >   log: use spice_error instead of g_error
> >   log: remove __FUNCTION__ from debug
> >   log: use domains for logging
> > 
> >  src/bio-gio.c                |   7 +-
> >  src/channel-base.c           |  13 ++--
> >  src/channel-cursor.c         |  19 ++++--
> >  src/channel-display-mjpeg.c  |   4 +-
> >  src/channel-display.c        |  65 +++++++++++--------
> >  src/channel-inputs.c         |  11 +++-
> >  src/channel-main.c           |  89 ++++++++++++++------------
> >  src/channel-playback.c       |  27 +++++---
> >  src/channel-port.c           |   9 +++
> >  src/channel-record.c         |  17 +++--
> >  src/channel-smartcard.c      |  13 +++-
> >  src/channel-usbredir.c       |  15 ++++-
> >  src/channel-webdav.c         |  22 +++++--
> >  src/continuation.c           |   3 +-
> >  src/coroutine_gthread.c      |   4 +-
> >  src/coroutine_ucontext.c     |   6 +-
> >  src/coroutine_winfibers.c    |   6 +-
> >  src/decode-glz.c             |   9 +--
> >  src/decode-jpeg.c            |   6 +-
> >  src/decode-zlib.c            |   6 +-
> >  src/desktop-integration.c    |  10 +--
> >  src/giopipe.c                |  10 +--
> >  src/map-file                 |   4 ++
> >  src/smartcard-manager.c      |  27 +++++---
> >  src/spice-audio.c            |  11 +++-
> >  src/spice-channel-priv.h     |   2 +-
> >  src/spice-channel.c          | 146
> >  ++++++++++++++++++++++--------------------
> >  src/spice-grabsequence.c     |   3 +-
> >  src/spice-gstaudio.c         |  52 ++++++++-------
> >  src/spice-gtk-session.c      |  57 ++++++++++-------
> >  src/spice-option.c           |   4 +-
> >  src/spice-pulse.c            | 148
> >  +++++++++++++++++++++++--------------------
> >  src/spice-session.c          |  67 +++++++++++---------
> >  src/spice-uri.c              |   1 +
> >  src/spice-util.c             |   8 ++-
> >  src/spice-util.h             |   6 --
> >  src/spice-widget.c           |  94 ++++++++++++++-------------
> >  src/spicy-connect.c          |   2 +-
> >  src/spicy-screenshot.c       |   2 +-
> >  src/spicy-stats.c            |   2 +-
> >  src/spicy.c                  |  34 +++++-----
> >  src/usb-device-manager.c     |  71 ++++++++++++---------
> >  src/usb-device-widget.c      |  16 ++++-
> >  src/usbutil.c                |   4 +-
> >  src/vmcstream.c              |  20 ++++--
> >  src/vncdisplaykeymap.c       |  30 +++++----
> >  src/win-usb-dev.c            |  25 +++++---
> >  src/win-usb-driver-install.c |  53 +++++++++-------
> >  48 files changed, 745 insertions(+), 515 deletions(-)
> > 
> > --
> > 2.5.0
> > 
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> > http://lists.freedesktop.org/mailman/listinfo/spice-devel
> > 
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]