[Spice-devel] [spice-common PATCH v1 0/13] subdomain filtering with glib logging

[Marc-André Lureau]

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...

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

> 
> 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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
>