[Spice-devel] [spice-common v3] use specific subdomains for better filtering

Marc-André Lureau marcandre.lureau at gmail.com
Mon Mar 14 13:02:26 UTC 2016


Hi

On Sat, Mar 12, 2016 at 3:32 PM, Victor Toso <victortoso at redhat.com> 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.

My opinion didn't change much on this series, I personally don't find
it useful since there isn't so much log going on. I would find it
harder to figure out the log domains, and I prefer to simply grep.

> With that said, SPICE_DEBUG=6 should work to get G_LOG_LEVEL_DEBUG of
> all code that uses spice logging functions.

Please keep SPICE_DEBUG=1 working for debugging. (as it, it's not a
level, but a bool)

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

Why so much choices? Does it mean that 0 will prevent
critical/warnings from being printed?

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

One problem is that update of just spice-common breaks the build, so
you'll need to address it at the same time to avoid preventing common
update.

> * 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;

I'd prefer if we stick with g_log functions if possible, but I don't
mind if we have to use spice_ variants.

thanks

>
> 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(-)
>
> --
> 2.5.0
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel



-- 
Marc-André Lureau


More information about the Spice-devel mailing list