Hi ----- Original Message ----- > Hi, > > On Tue, Dec 01, 2015 at 07:19:58AM -0500, Marc-André Lureau wrote: > > ----- Original Message ----- > > > Hi, > > > > > > On Tue, Dec 01, 2015 at 06:41:47AM -0500, Marc-André Lureau wrote: > > > > ----- Original Message ----- > > > > > Hi > > > > > > > > > > ----- Original Message ----- > > > > > > usbredir on debug level is highly verbose. That does not help much > > > > > > when > > > > > > we are trying to debug the spice-gtk side of usbredir. > > > > > > > > > > > > This patch introduces SPICE_DEBUG_USBREDIR env var to set the log > > > > > > level > > > > > > for usbredir side. If this is not set, usbredir will behave the > > > > > > same > > > > > > way > > > > > > as it was by checking SPICE_DEBUG env var. > > > > > > > > > > > > WARNING level is the default when neither env var are set. > > > > > > > > > > Why not filtering out the strings you don't want with -v ? In > > > > > general, > > > > > with > > > > > the spice-gtk strings, you can easily strip down the log to what you > > > > > need, > > > > > and this is a lot more flexible than having various environment > > > > > variables. > > > > > > > > I mean filtering with "grep -v" > > > > > > I don't understand why do you think filtering the output is better then > > > not generating an useless output. > > > It is useless because if I'm not debugging usb packets, I don't really > > > need debug+ level for usbredir. > > > > > > Indeed, another env var is not nice. My main goal would be having > > > fdo#91838 solved. But I don't want to touch that while spice-server is > > > not yet GObjectify (to set proper domains). > > > > > > https://bugs.freedesktop.org/show_bug.cgi?id=91838 > > > > > > But I really want to understand why filtering is okay for you. Doesn't > > > it affect the runtime? printing all the time is not cheap I guess. > > > > At the rate spice-gtk prints, it is probably very very cheap, it is not > > relevant to talk about performance here. > > > > usbredir prints a lot when we redirect a webcam... > > > My point is greping (or other filtering tools) offers a lot more > > flexibility to configure what you want to see. > > Fair enough but I think this is more a personal opinion. > > In my opinion, something like below to work with usbredir seems much > better then filtering in/out the output: > > SPICE_DEBUG=spice-channel:*,channel-usbredir:* > > That seems doable even if only for spice-gtk. I'll try that later if > that's seems reasonable. As long as SPICE_DEBUG=1 keep working, and the patch is reasonable. However, I think in the case of usbredir, it would make sense to attibute its own GLog domain, and not print usbredir messages in SPICE_DEBUG=1 log. This can be done in usbredir_log(). > > (without, of course, losing the generic SPICE_DEBUG=<int>) > > Best, > > > > > > Cheers, > > > toso > > > > > > > > > > > > > > > > > > --- > > > > > > src/channel-usbredir.c | 12 +++++++++++- > > > > > > 1 file changed, 11 insertions(+), 1 deletion(-) > > > > > > > > > > > > diff --git a/src/channel-usbredir.c b/src/channel-usbredir.c > > > > > > index 0be72ba..84cabe0 100644 > > > > > > --- a/src/channel-usbredir.c > > > > > > +++ b/src/channel-usbredir.c > > > > > > @@ -202,6 +202,16 @@ static void > > > > > > channel_set_handlers(SpiceChannelClass > > > > > > *klass) > > > > > > > > > > > > /* > > > > > > ------------------------------------------------------------------ > > > > > > */ > > > > > > /* private api > > > > > > */ > > > > > > +static gint > > > > > > +usbredir_get_log_level(void) > > > > > > +{ > > > > > > + const gchar *debug_env = g_getenv("SPICE_DEBUG_USBREDIR"); > > > > > > + if (debug_env != NULL) { > > > > > > + return CLAMP(atoi(debug_env), usbredirparser_none, > > > > > > usbredirparser_debug_data); > > > > > > + } > > > > > > + > > > > > > + return spice_util_get_debug() ? usbredirparser_debug : > > > > > > usbredirparser_warning; > > > > > > +} > > > > > > > > > > > > G_GNUC_INTERNAL > > > > > > void spice_usbredir_channel_set_context(SpiceUsbredirChannel > > > > > > *channel, > > > > > > @@ -223,7 +233,7 @@ void > > > > > > spice_usbredir_channel_set_context(SpiceUsbredirChannel *channel, > > > > > > usbredir_unlock_lock, > > > > > > usbredir_free_lock, > > > > > > channel, PACKAGE_STRING, > > > > > > - spice_util_get_debug() ? > > > > > > usbredirparser_debug : usbredirparser_warning, > > > > > > + usbredir_get_log_level(), > > > > > > usbredirhost_fl_write_cb_owns_buffer); > > > > > > if (!priv->host) > > > > > > g_error("Out of memory allocating usbredirhost"); > > > > > > -- > > > > > > 2.5.0 > > > > > > > > > > > > _______________________________________________ > > > > > > Spice-devel mailing list > > > > > > Spice-devel@xxxxxxxxxxxxxxxxxxxxx > > > > > > http://lists.freedesktop.org/mailman/listinfo/spice-devel > > > > > > > > > > > > > > _______________________________________________ > > > Spice-devel mailing list > > > Spice-devel@xxxxxxxxxxxxxxxxxxxxx > > > http://lists.freedesktop.org/mailman/listinfo/spice-devel > > > > > _______________________________________________ > > Spice-devel mailing list > > Spice-devel@xxxxxxxxxxxxxxxxxxxxx > > http://lists.freedesktop.org/mailman/listinfo/spice-devel > _______________________________________________ > Spice-devel mailing list > Spice-devel@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/spice-devel > _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel