Hi, Thanks for your reply :) On Mon, Mar 14, 2016 at 02:02:26PM +0100, Marc-André Lureau wrote: > > 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. Sure. > I would find it harder to figure out the log domains, and I prefer to > simply grep. It shouldn't be hard. The subdomains would be prefixed in the logging so you could easily see which one you (usually) want to filter out. We could have something in the --help-spice related to which domains are available for completeness but I don't think it would be that useful or interesting. > > > 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) Sure. > > > 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? We can remove multiple choices, that's fine. What I would like to keep is a way to chose what you want to see and a way to filter out what you don't want to see. So, if SPICE_DEBUG=0 usually means no debug, we could extend that to have SPICE_DEBUG=0,usb:6 to have only usb related debug. > Does it mean that 0 will prevent critical/warnings from being printed? I Well, the way I first thought of it, yes. But taking in consideration that SPICE_DEBUG is a boolean, I think that every subdomain that is not specified should get warnings and above. > > > 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. Yes :) I don't mind taking care of it, I just don't want to do it if this series is not interesting to have (to others). > > > * 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. That was a mistake by may part. I don't think we can using g_debug, g_warning and so on in spice-gtk and spice/server and have the subdomain filtering as well... so, as far as I can see, we would need to stick with spice_ variants indeed. > > thanks Thank you! toso > > > > > 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@xxxxxxxxxxxxxxxxxxxxx > > https://lists.freedesktop.org/mailman/listinfo/spice-devel > > > > -- > Marc-André Lureau _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel