Re: [Qemu-devel] [PATCH v2 3/3] hw/sd: use guest error logging rather than fprintf to stderr

[Peter Crosthwaite]

On Wed, Dec 16, 2015 at 11:02 AM, Andrew Baumann
<address@hidden> wrote:
> Some of these errors may be harmless (e.g. probing unimplemented
> commands, or issuing CMD12 in the wrong state), and may also be quite
> frequent. Spamming the standard error output isn't desirable in such
> cases.
>
> Signed-off-by: Andrew Baumann <address@hidden>
> ---
> It might also be desirable to have a squelch mechanism for these
> messages, but at least for my use-case, this is sufficient, since they
> only occur during boot time.
>
>  hw/sd/sd.c | 22 +++++++++++++---------
>  1 file changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 1011785..9b76b47 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -1234,16 +1234,18 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
>
>      default:
>      bad_cmd:
> -        fprintf(stderr, "SD: Unknown CMD%i\n", req.cmd);
> +        qemu_log_mask(LOG_GUEST_ERROR, "SD: Unknown CMD%i\n", req.cmd);
>          return sd_illegal;
>
>      unimplemented_cmd:
>          /* Commands that are recognised but not yet implemented in SPI mode. 
>  */
> -        fprintf(stderr, "SD: CMD%i not implemented in SPI mode\n", req.cmd);
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "SD: CMD%i not implemented in SPI mode\n",
> +                      req.cmd);

Are these unimplemented by specification or unimplemented in QEMU? If
they are unimplemented in QEMU it should be a LOG_UNIMP.

Otherwise:

Reviewed-by: Peter Crosthwaite <address@hidden>

Regards,
Peter

>          return sd_illegal;
>      }
>
> -    fprintf(stderr, "SD: CMD%i in a wrong state\n", req.cmd);
> +    qemu_log_mask(LOG_GUEST_ERROR, "SD: CMD%i in a wrong state\n", req.cmd);
>      return sd_illegal;
>  }
>
> @@ -1375,7 +1377,7 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
>          return sd_normal_command(sd, req);
>      }
>
> -    fprintf(stderr, "SD: ACMD%i in a wrong state\n", req.cmd);
> +    qemu_log_mask(LOG_GUEST_ERROR, "SD: ACMD%i in a wrong state\n", req.cmd);
>      return sd_illegal;
>  }
>
> @@ -1418,7 +1420,7 @@ int sd_do_command(SDState *sd, SDRequest *req,
>          if (!cmd_valid_while_locked(sd, req)) {
>              sd->card_status |= ILLEGAL_COMMAND;
>              sd->expecting_acmd = false;
> -            fprintf(stderr, "SD: Card is locked\n");
> +            qemu_log_mask(LOG_GUEST_ERROR, "SD: Card is locked\n");
>              rtype = sd_illegal;
>              goto send_response;
>          }
> @@ -1576,7 +1578,8 @@ void sd_write_data(SDState *sd, uint8_t value)
>          return;
>
>      if (sd->state != sd_receivingdata_state) {
> -        fprintf(stderr, "sd_write_data: not in Receiving-Data state\n");
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "sd_write_data: not in Receiving-Data state\n");
>          return;
>      }
>
> @@ -1695,7 +1698,7 @@ void sd_write_data(SDState *sd, uint8_t value)
>          break;
>
>      default:
> -        fprintf(stderr, "sd_write_data: unknown command\n");
> +        qemu_log_mask(LOG_GUEST_ERROR, "sd_write_data: unknown command\n");
>          break;
>      }
>  }
> @@ -1710,7 +1713,8 @@ uint8_t sd_read_data(SDState *sd)
>          return 0x00;
>
>      if (sd->state != sd_sendingdata_state) {
> -        fprintf(stderr, "sd_read_data: not in Sending-Data state\n");
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "sd_read_data: not in Sending-Data state\n");
>          return 0x00;
>      }
>
> @@ -1821,7 +1825,7 @@ uint8_t sd_read_data(SDState *sd)
>          break;
>
>      default:
> -        fprintf(stderr, "sd_read_data: unknown command\n");
> +        qemu_log_mask(LOG_GUEST_ERROR, "sd_read_data: unknown command\n");
>          return 0x00;
>      }
>
> --
> 2.5.3
>