Re: [Qemu-devel] [PATCH v2] vhost-user: print original request on error

[Markus Armbruster]

"Michael S. Tsirkin" <address@hidden> writes:

> When we get an unexpected response, print out
> the original request.
> Helps debug protocol errors tremendously.
>
> Signed-off-by: Michael S. Tsirkin <address@hidden>
> ---
>
> Changes from v1: add missing .
>
>  hw/virtio/vhost-user.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index 3404b81..5bc6c45 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -121,8 +121,8 @@ static int vhost_user_read(struct vhost_dev *dev, 
> VhostUserMsg *msg)
>  
>      r = qemu_chr_fe_read_all(chr, p, size);
>      if (r != size) {
> -        error_report("Failed to read msg header. Read %d instead of %d.", r,
> -                size);
> +        error_report("Failed to read msg header. Read %d instead of %d."
> +                     " Original request %d.", r, size, msg->request);
>          goto fail;
>      }

Error message style nit: the error message proper (the thing emitted by
error_report()) should be a phrase, and it should be short and to the
point.  It can be followed by hints.  Compare:

    qemu-system-x86_64: Failed to read msg header. Read 11 instead of 12. 
Original request 1.

and

    qemu-system-x86_64: Failed to read msg header
    Read 11 instead of 12 for request 1.

I prefer the latter.  The error message proper is short and to the
point.  The hint adds information, which happens to be useful mainly to
developers.  Sensible line lengths.

By the way, the error.h API supports this message + hints convention
since commit 50b7b00.

See also
Message-ID: <address@hidden>
http://lists.gnu.org/archive/html/qemu-devel/2015-11/msg01662.html