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

[Christophe Fergeau]

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");

>  
> 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_CLIENT(mcc),
>                                                    SPICE_MSG_MAIN_MIGRATE_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
-------------- 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/20170616/70ee3e8a/attachment.sig>