[Spice-devel] [spice-common v3] use specific subdomains for better filtering
David Jaša
djasa at redhat.com
Fri Apr 15 14:22:05 UTC 2016
Hi, I like the approach a lot. It seems quite clear and nice to me,
but ...
On So, 2016-03-12 at 15:32 +0100, Victor Toso wrote:
> Hi! I've rebased this series and pushed to my remote branch in
> freedesktop [0] [1]. I'll try to clarify the ideia for working on this
> and if it does not get any positive feedback I'll take it as something
> not interesting to have...
>
> [0] (common) https://cgit.freedesktop.org/~victortoso/spice-common/log/?h=logs
> [1] (gtk) https://cgit.freedesktop.org/~victortoso/spice-gtk/log/?h=logs
> fdo: https://bugs.freedesktop.org/show_bug.cgi?id=91838
>
> So, first of all, I don't want to introduce something that everybody
> need to understand in order to get proper debug. The main idea is able
> to filter in/out stuff that you don't want to see without the need to
> | grep -v and | grep -i it.
>
> With that said, SPICE_DEBUG=6 should work to get G_LOG_LEVEL_DEBUG of
> all code that uses spice logging functions.
... this essentially breaks compatibility in a bad way. IMO it has
already "sunk in" that G_MESSAGES_DEBUG=(GSpice|all) SPICE_DEBUG=1 means
all debug output from spice_gtk. Having just a trickle instead of
expected flood would be surprising.
I suggest adding a brief message when SPICE_DEBUG is set outlining the
new usage so that you don't have to bother with backwards compatibility
and users are warned.
The same would be suitable eventually for spice server when it's
SPICE_DEBUG_LEVEL-controlled debugging is replaced with a new one.
> With all debug enable you would see all lines prefixed with
> "[subdomain-name]" so, in case you don't want to see it (filter out),
> you could:
>
> $SPICE_DEBUG=6,subdomain-name:- remote-viwer ...
>
> And voilà, nothing from it will be showed as '-' is the same as '0'
> which is *no debug*. The opposite of it would be '*' or '6'.
> * Both '-' and 0 would work and "none" as well;
> * Both '*' and 6 would work and "debug" as well;
>
+1 to Marc-André's suggestion to remove multiple choices.
> In case you only want to see some debug info, you could do something
> like:
>
> SPICE_DEBUG=audio:*,playback:*
>
> This would show spice-audio, spice-gstaduio, spice-pulse,
> channel-playback debugs and nothing else.
I don't like asterisks because they need to be escaped in shell. For
subdomains, "all" would be IMO better and for log levels, highest
verbosity level should be sufficient. The highest level could be printed
in the logging overview message (and either taken out of the actual
code, or standardized in coding style to avoid surprises).
HTH,
David
>
> Q: Do you want this just to avoid grep?
> A: No! We should not avoid inserting debug into the code because it
> could get verbose, we should smart handling it with something like this
> IMHO. We have a few #ifdef #DEBUG_THIS which is not useful when asking
> debug info to reporter in open bugs.
>
> I really think that this could be improved a lot yet but it would need a
> proper discussion. If this does not seem useful, then it's fine :)
>
> Another misc info:
> * I did not try this with spice server yet;
> * We can use spice logging tools or glib ones, I choosed spice first as
> I think we can have better control with it;
> * I did not try to fix the spice-common tests yet;
>
> changes from v2:
> * rebased to latest version upstream
> * Kept usage of SPICE_LOG_DOMAIN instead of G_LOG_DOMAIN that I
> previously proposed
> * moved some changes that should be in another commit
> * changed the static variable from SPICE_LOG_DOMAIN to
> SPICE_LOG_SUB_DOMAIN
>
> 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.
>
> [spice-common]
> Victor Toso (6):
> log: simplify log defines with SPICE_LOG
> log: include message log level for parity with glib
> log: allow filtering logs with subdomains
> log: create specific subdomains for filtering
> log: Disable multiple domains in Spice
> don't break the build with this wip patches
>
> common/canvas_base.c | 3 +-
> common/canvas_utils.c | 1 +
> common/log.c | 174 ++++++++++++++++++++++++++++++++++++++++++++++----
> common/log.h | 87 ++++++++++++++++---------
> common/lz.c | 2 +
> common/marshaller.c | 1 +
> common/mem.c | 1 +
> common/pixman_utils.c | 1 +
> common/quic.c | 4 +-
> common/region.c | 2 +
> common/rop3.c | 4 +-
> common/ssl_verify.c | 3 +-
> tests/test-logging.c | 3 +-
> 13 files changed, 238 insertions(+), 48 deletions(-)
>
> [spice-gtk]
> Victor Toso (13):
> 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 plain spice_debug instead of VNC_DEBUG
> log: use spice logging instead of (f)printf
> log: use spice_error instead of g_error
> Update spice-common submodule
> log: create subdomains for spice-gtk
> log: remove dup __FUNCTION__ argument from logs
>
> spice-common | 2 +-
> src/Makefile.am | 3 +
> src/bio-gio.c | 8 +-
> src/channel-base.c | 14 +--
> src/channel-cursor.c | 30 +++---
> src/channel-display-mjpeg.c | 4 +-
> src/channel-display.c | 58 +++++-----
> src/channel-inputs.c | 4 +-
> src/channel-main.c | 83 +++++++-------
> 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/coroutine_gthread.c | 8 +-
> src/coroutine_ucontext.c | 7 +-
> src/coroutine_winfibers.c | 9 +-
> src/decode-glz.c | 10 +-
> 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 | 191 ++++++++++++++++++---------------
> src/spice-client-glib-usb-acl-helper.c | 9 +-
> src/spice-grabsequence.c | 4 +-
> src/spice-gstaudio.c | 48 +++++----
> src/spice-gtk-session.c | 48 +++++----
> 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-egl.c | 18 ++--
> src/spice-widget.c | 96 +++++++++--------
> 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 ++++----
> 51 files changed, 675 insertions(+), 580 deletions(-)
>
More information about the Spice-devel
mailing list