[Spice-devel] [PATCH spice-server] Replace all uses of spice_printerr()

[Jonathon Jongsma]

On Fri, 2017-06-16 at 18:00 +0200, Christophe Fergeau wrote:
> On Thu, Jun 15, 2017 at 11:12:04AM -0500, Jonathon Jongsma wrote:
> > For those things that are actually errors or warnings, switch to
> > spice_warning(). For those things that are just informational,
> > switch to
> > spice_debug().  Currently, these messages are just indiscriminately
> > printed to stderr even if debugging isn't enabled, which can clog
> > up the
> > qemu monitor, for example.
> > 
> > Signed-off-by: Jonathon Jongsma <jjongsma at redhat.com>
> > ---
> > @@ -656,7 +656,7 @@ int inputs_channel_has_tablet(InputsChannel
> > *inputs)
> >  
> >  void inputs_channel_detach_tablet(InputsChannel *inputs,
> > SpiceTabletInstance *tablet)
> >  {
> > -    spice_printerr("");
> > +    spice_debug(NULL);
> >      inputs->tablet = NULL;
> >  }
> 
> There are various occurrences of spice_debug(NULL); throughout this
> patch, they should be replaced with spice_debug("trace");

Oops, this was a patch I had sitting around from before you changed
those and I forgot to rebase. I'll send a new version soon with the
changes.


> 
> >  
> > diff --git a/server/main-channel-client.c b/server/main-channel-
> > client.c
> > index ae8d2d5..b436564 100644
> > --- a/server/main-channel-client.c
> > +++ b/server/main-channel-client.c
> > @@ -442,7 +442,7 @@ void
> > main_channel_client_handle_migrate_connected(MainChannelClient
> > *mcc,
> >                                                    int seamless)
> >  {
> >      RedClient *client =
> > red_channel_client_get_client(RED_CHANNEL_CLIENT(mcc));
> > -    spice_printerr("client %p connected: %d seamless %d", client,
> > success, seamless);
> > +    spice_debug("client %p connected: %d seamless %d", client,
> > success, seamless);
> >      if (mcc->priv->mig_wait_connect) {
> >          RedChannel *channel =
> > red_channel_client_get_channel(RED_CHANNEL_CLIENT(mcc));
> >          MainChannel *main_channel = MAIN_CHANNEL(channel);
> > @@ -452,7 +452,7 @@ void
> > main_channel_client_handle_migrate_connected(MainChannelClient
> > *mcc,
> >          main_channel_on_migrate_connected(main_channel, success,
> > seamless);
> >      } else {
> >          if (success) {
> > -            spice_printerr("client %p MIGRATE_CANCEL", client);
> > +            spice_debug("client %p MIGRATE_CANCEL", client);
> >              red_channel_client_pipe_add_empty_msg(RED_CHANNEL_CLIE
> > NT(mcc),
> >                                                    SPICE_MSG_MAIN_M
> > IGRATE_CANCEL);
> >          }
> > @@ -502,9 +502,9 @@ void
> > main_channel_client_handle_pong(MainChannelClient *mcc,
> > SpiceMsgPing *ping,
> >          mcc->priv->net_test_id = 0;
> >          if (roundtrip <= mcc->priv->latency) {
> >              // probably high load on client or server result with
> > incorrect values
> > -            spice_printerr("net test: invalid values, latency %"
> > PRIu64
> > -                           " roundtrip %" PRIu64 ". assuming high"
> > -                           "bandwidth", mcc->priv->latency,
> > roundtrip);
> > +            spice_warning("net test: invalid values, latency %"
> > PRIu64
> > +                          " roundtrip %" PRIu64 ". assuming high"
> > +                          "bandwidth", mcc->priv->latency,
> > roundtrip);
> >              mcc->priv->latency = 0;
> >              mcc->priv->net_test_stage = NET_TEST_STAGE_INVALID;
> >              red_channel_client_start_connectivity_monitoring(rcc,
> > @@ -514,19 +514,19 @@ void
> > main_channel_client_handle_pong(MainChannelClient *mcc,
> > SpiceMsgPing *ping,
> >          mcc->priv->bitrate_per_sec = (uint64_t)(NET_TEST_BYTES *
> > 8) * 1000000
> >              / (roundtrip - mcc->priv->latency);
> >          mcc->priv->net_test_stage = NET_TEST_STAGE_COMPLETE;
> > -        spice_printerr("net test: latency %f ms, bitrate %"PRIu64"
> > bps (%f Mbps)%s",
> > -                       (double)mcc->priv->latency / 1000,
> > -                       mcc->priv->bitrate_per_sec,
> > -                       (double)mcc->priv->bitrate_per_sec / 1024 /
> > 1024,
> > -                       main_channel_client_is_low_bandwidth(mcc) ?
> > " LOW BANDWIDTH" : "");
> > +        spice_debug("net test: latency %f ms, bitrate %"PRIu64"
> > bps (%f Mbps)%s",
> > +                    (double)mcc->priv->latency / 1000,
> > +                    mcc->priv->bitrate_per_sec,
> > +                    (double)mcc->priv->bitrate_per_sec / 1024 /
> > 1024,
> > +                    main_channel_client_is_low_bandwidth(mcc) ? "
> > LOW BANDWIDTH" : "");
> 
> I think I sent a similar patch in the past, and there was some
> discussion around this kind of messages, whether they should be
> hidden
> or not? Personally I'm fine with your spice_debug/spice_warning()
> choices, so ack from me with the spice_debug(NULL) issues fixed.
> 
> Christophe