[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2] vhost-user: print original request on error
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v2] vhost-user: print original request on error |
Date: |
Tue, 17 Nov 2015 11:23:12 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
"Michael S. Tsirkin" <address@hidden> writes:
> On Tue, Nov 17, 2015 at 08:52:24AM +0100, Markus Armbruster wrote:
>> "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
>
> Oh that's nice. There are more cases like this in vhost-user.
> Let's rework them all past 2.5?
Okay. Several ones in 2.6 is better than just one now :)