Re: [spice-common PATCH v2 00/17] subdomain filtering with glib logging

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

 



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




[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]