On Tue, Nov 17, 2015 at 08:52:24AM +0100, Markus Armbruster wrote:
> "Michael S. Tsirkin" <m...@redhat.com> writes:
>
> > When we get an unexpected response, print out
> > the original request.
> > Helps debug protocol errors tremendously.
> >
> > Signed-off-by: Michael S. Tsirkin <m...@redhat.com>
> > ---
> >
> > 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: <87oaf3jww7....@blackfin.pond.sub.org>
> http://lists.gnu.org/archive/html/qemu-devel/2015-11/msg01662.html
Oh that's nice. There are more cases like this in vhost-user.
Let's rework them all past 2.5?
--
MST
