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@xxxxxxxxxx>
> > ---
> > @@ -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
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel