So I've gone through the patch series briefly. I am sympathetic to the desire
for more control over the debugging output, but to be honest I'm not sure
whether the benefits justify the added complexity. I've used gstreamer a bit
(which has a similar debugging framework) and I always found the debug framework
overly complex and I didn't really like using it. For gstreamer it makes sense
to have a complex debug framework because it produces such a huge amount of
debug output. You can easily generate debug output that is several gigabytes.
But for a project like spice, I'm not sure we produce enough output to make it
necessary.
A couple minor things that I didn't really like much about gstreamer debugging:
- it's not always obvious what "domains" are available (you have to look in the
source file to figure out which domain to enable for the thing you want to
debug)
- If I haven't done it in a while, I often have to go look up the format for
specifying the debug specification. Are the domains separated with commas,
semicolons? do I need to specify numeric values for debug level? or do I use
strings ("warning", etc)?
So, I'm not totally opposed to this system, but I feel there's something nice
about keeping it simpler. I'd like to hear opinions from others though.
Jonathon
On Fri, 2016-01-08 at 09:51 +0100, Victor Toso wrote:
> So, another WIP series to gather feedback regarding this subdomain
> filtering. This patch is in top of Christophe's patches [0] to use glib
> log system. The idea is better explained in v1 [1] and commit logs.
>
> [0]
> http://lists.freedesktop.org/archives/spice-devel/2015-December/025012.html
> [1]
> http://lists.freedesktop.org/archives/spice-devel/2015-December/024893.html
>
> If this approach seems okay, I still need to:
> - fix logging tests that Christophe has introduced in last interaction;
> - implement some tests for subdomains filtering
> - deal with --spice-debug arg option (something like export
> SPICE_DEBUG=*:debug)
>
> changes from v1:
> - Use one-static-per-file approach to define each subdomain. This is
> similar to how libvirt does and seems much cleaner.
> - Removed f(printf) debugs
> - Created subdomains for spice-common as well as now this is a must.
>
> Victor Toso (4): (spice-common)
> 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 logs with subdomains
>
> common/Makefile.am | 1 +
> common/canvas_base.c | 3 +-
> common/canvas_utils.c | 1 +
> common/log.c | 184 +++++++++++++++++++++++++++++++++++++++++++++----
> -
> common/log.h | 84 ++++++++++++++---------
> common/lz.c | 2 +
> common/mem.c | 2 +
> common/pixman_utils.c | 1 +
> common/quic.c | 4 +-
> common/region.c | 2 +
> common/rop3.c | 4 +-
> common/ssl_verify.c | 3 +-
> 12 files changed, 237 insertions(+), 54 deletions(-)
>
> Victor Toso (13): (spice-gtk)
> log: use glib logging on testing tools
> log: use spice_debug instead of SPICE_DEBUG
> log: nitpick at channel name in CHANNEL_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: use plain spice_debug instead of VNC_DEBUG
> log: use spice logging instead of (f)printf
> log: remove __FUNCTION__ from debug
> log: use "Spice" domain
> log: use specific subdomains for better filtering
>
> src/Makefile.am | 2 +-
> src/bio-gio.c | 8 +-
> src/channel-base.c | 14 +--
> src/channel-cursor.c | 29 ++---
> src/channel-display-mjpeg.c | 4 +-
> src/channel-display.c | 58 +++++-----
> src/channel-inputs.c | 4 +-
> src/channel-main.c | 82 +++++++-------
> src/channel-playback.c | 20 ++--
> src/channel-port.c | 2 +
> src/channel-record.c | 10 +-
> src/channel-smartcard.c | 6 +-
> src/channel-usbredir.c | 8 +-
> src/channel-webdav.c | 16 +--
> src/continuation.c | 4 +-
> src/coroutine_gthread.c | 8 +-
> src/coroutine_ucontext.c | 9 +-
> src/coroutine_winfibers.c | 9 +-
> src/decode-glz.c | 9 +-
> src/decode-jpeg.c | 6 +-
> src/decode-zlib.c | 6 +-
> src/desktop-integration.c | 10 +-
> src/giopipe.c | 11 +-
> src/map-file | 1 +
> src/smartcard-manager.c | 20 ++--
> src/spice-audio.c | 4 +-
> src/spice-channel-priv.h | 2 +-
> src/spice-channel.c | 189 ++++++++++++++++++--------------
> -
> src/spice-client-glib-usb-acl-helper.c | 6 +-
> src/spice-grabsequence.c | 4 +-
> src/spice-gstaudio.c | 46 ++++----
> src/spice-gtk-session.c | 50 ++++-----
> src/spice-option.c | 4 +-
> src/spice-pulse.c | 142 +++++++++++++------------
> src/spice-session.c | 60 ++++++-----
> src/spice-uri.c | 2 +
> src/spice-util.c | 8 +-
> src/spice-util.h | 6 --
> src/spice-widget.c | 88 +++++++--------
> src/spicy-connect.c | 2 +-
> src/spicy-screenshot.c | 10 +-
> src/spicy-stats.c | 12 +--
> src/spicy.c | 34 +++---
> src/usb-device-manager.c | 64 +++++------
> src/usb-device-widget.c | 7 +-
> src/usbutil.c | 4 +-
> src/vmcstream.c | 14 +--
> src/vncdisplaykeymap.c | 46 ++++----
> src/win-usb-dev.c | 18 ++--
> src/win-usb-driver-install.c | 46 ++++----
> tests/Makefile.am | 2 +-
> 51 files changed, 658 insertions(+), 568 deletions(-)
>
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel