From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52062) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zybcy-0007ue-VV for qemu-devel@nongnu.org; Tue, 17 Nov 2015 03:28:38 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zybcu-00028x-LM for qemu-devel@nongnu.org; Tue, 17 Nov 2015 03:28:36 -0500 Received: from mx1.redhat.com ([209.132.183.28]:41473) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zybcu-00028f-GZ for qemu-devel@nongnu.org; Tue, 17 Nov 2015 03:28:32 -0500 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (Postfix) with ESMTPS id EC0A442E5AF for ; Tue, 17 Nov 2015 08:28:31 +0000 (UTC) Date: Tue, 17 Nov 2015 10:28:29 +0200 From: "Michael S. Tsirkin" Message-ID: <20151117102748-mutt-send-email-mst@redhat.com> References: <1447676789-7782-1-git-send-email-mst@redhat.com> <87lh9x2hc7.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87lh9x2hc7.fsf@blackfin.pond.sub.org> Subject: Re: [Qemu-devel] [PATCH v2] vhost-user: print original request on error List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: qemu-devel@nongnu.org On Tue, Nov 17, 2015 at 08:52:24AM +0100, Markus Armbruster wrote: > "Michael S. Tsirkin" writes: > > > When we get an unexpected response, print out > > the original request. > > Helps debug protocol errors tremendously. > > > > Signed-off-by: Michael S. Tsirkin > > --- > > > > 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.fsf@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