[Spice-devel] [spice-server v4 5/5] Change some spice_printerr() to spice_debug()

[Christophe Fergeau]

On Mon, Feb 27, 2017 at 06:39:14AM -0500, Frediano Ziglio wrote:
> > 
> > On Wed, Feb 15, 2017 at 11:24:37AM -0500, Frediano Ziglio wrote:
> > > > 
> > > > 
> > > > > On 15 Feb 2017, at 12:45, Frediano Ziglio <fziglio at redhat.com> wrote:
> > > > > 
> > > > >> @@ -134,7 +134,7 @@ static void
> > > > >> red_qxl_display_migrate(RedChannelClient
> > > > >> *rcc)
> > > > >>     }
> > > > >>     g_object_get(channel, "channel-type", &type, "id", &id, NULL);
> > > > >>     dispatcher = (Dispatcher *)g_object_get_data(G_OBJECT(channel),
> > > > >>     "dispatcher");
> > > > >> -    spice_printerr("channel type %u id %u", type, id);
> > > > >> +    spice_debug("channel type %u id %u", type, id);
> > > > >>     payload.rcc = rcc;
> > > > >>     dispatcher_send_message(dispatcher,
> > > > >>                             RED_WORKER_MESSAGE_DISPLAY_MIGRATE,
> > > > > 
> > > > > Looks like there's lot of debugging on migration.
> > > > > Like we are not really sure the code is working.
> > > > 
> > > > Maybe someone once needed a lot of debug information, and kept it around
> > > > in
> > > > case things start going south. That probably means we have annotations
> > > > that
> > > > may end up being annoying for everybody debugging anything else than
> > > > migration.
> > > > 
> > > 
> > > I wrote that consideration thinking about customer reports.
> > > spice_printerr goes in the logs while spice_debug not (by default).
> > > If we have lot of bugs in migration removing from client logs could
> > > became an issue for us. It's hard to ask to the customer to enable all
> > > debugging (too invasive) and even if it could be enabled separately
> > > it could be too late (as hard to reproduce for the customer) or
> > > complicated (think about huge setups).
> > > 
> > > (CCing Jasa)
> > 
> > In my opinion, a library really should not be displaying anything on
> > stdout by default, and as little as possible on stderr. "We print this
> > debugging code to stderr because we want it to show up in the log by
> > default" does not sound very compelling. Why this debugging code and
> > not eg qxl display debugging code? Next step is suggesting to always
> > output all debugging code to stderr, but then we'd get too much :)
> > 
> > I'd just silence all the debugging code, or remove it, and work on a way
> > of making it easy to enable/filter/... if what we have now is not good
> > enough.
> > 
> > Christophe
> > 
> 
> Sure, however your comment is going OT.
> My comments was specific to migration problems.
> 

And I don't think debug for migration problems should be anything
special. It has no good reason to go to stderr.

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20170227/d18fb9de/attachment.sig>