Re: [Qemu-devel] [PATCH 02/14] nbd/client: More consistent error messages

[Vladimir Sementsov-Ogievskiy]

01.12.2018 1:03, Eric Blake wrote:
> Consolidate on using decimal (not hex) and on outputting the
> option reply name (not just value) when the client reports
> protocol discrepancies from the server.  While it won't affect
> normal operation, it makes debugging additions easier.
> 
> Signed-off-by: Eric Blake <address@hidden>
> ---
>   nbd/client.c | 21 ++++++++++++---------
>   1 file changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/nbd/client.c b/nbd/client.c
> index b4d457a19ad..b667a1b56fd 100644
> --- a/nbd/client.c
> +++ b/nbd/client.c
> @@ -132,8 +132,9 @@ static int nbd_receive_option_reply(QIOChannel *ioc, 
> uint32_t opt,
>           return -1;
>       }
>       if (reply->option != opt) {
> -        error_setg(errp, "Unexpected option type %x expected %x",
> -                   reply->option, opt);
> +        error_setg(errp, "Unexpected option type %u (%s) expected %u (%s)",
> +                   reply->option, nbd_opt_lookup(reply->option),
> +                   opt, nbd_opt_lookup(opt));
>           nbd_send_opt_abort(ioc);
>           return -1;
>       }
> @@ -265,8 +266,9 @@ static int nbd_receive_list(QIOChannel *ioc, const char 
> *want, bool *match,
>           }
>           return 0;
>       } else if (reply.type != NBD_REP_SERVER) {
> -        error_setg(errp, "Unexpected reply type %" PRIx32 " expected %x",
> -                   reply.type, NBD_REP_SERVER);
> +        error_setg(errp, "Unexpected reply type %u (%s) expected %u (%s)",
> +                   reply.type, nbd_rep_lookup(reply.type),
> +                   NBD_REP_SERVER, nbd_rep_lookup(NBD_REP_SERVER));
>           nbd_send_opt_abort(ioc);
>           return -1;
>       }
> @@ -378,9 +380,9 @@ static int nbd_opt_go(QIOChannel *ioc, const char 
> *wantname,
>               return 1;
>           }
>           if (reply.type != NBD_REP_INFO) {
> -            error_setg(errp, "unexpected reply type %" PRIu32
> -                       " (%s), expected %u",
> -                       reply.type, nbd_rep_lookup(reply.type), NBD_REP_INFO);
> +            error_setg(errp, "unexpected reply type %u (%s), expected %u 
> (%s)",

hmm, we are definitely inconsistent about having comma before "expected" word...

anyway,
Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>

> +                       reply.type, nbd_rep_lookup(reply.type),
> +                       NBD_REP_INFO, nbd_rep_lookup(NBD_REP_INFO));
>               nbd_send_opt_abort(ioc);
>               return -1;
>           }
> @@ -704,8 +706,9 @@ static int nbd_negotiate_simple_meta_context(QIOChannel 
> *ioc,
>       }
> 
>       if (reply.type != NBD_REP_ACK) {
> -        error_setg(errp, "Unexpected reply type %" PRIx32 " expected %x",
> -                   reply.type, NBD_REP_ACK);
> +        error_setg(errp, "Unexpected reply type %u (%s) expected %u (%s)",
> +                   reply.type, nbd_rep_lookup(reply.type),
> +                   NBD_REP_ACK, nbd_rep_lookup(NBD_REP_ACK));
>           nbd_send_opt_abort(ioc);
>           return -1;
>       }
> 


-- 
Best regards,
Vladimir