From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49177) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YVaRm-0002n9-EH for qemu-devel@nongnu.org; Wed, 11 Mar 2015 02:48:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YVaRj-00056r-1j for qemu-devel@nongnu.org; Wed, 11 Mar 2015 02:48:50 -0400 Received: from mx1.redhat.com ([209.132.183.28]:60712) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YVaRi-00056e-PR for qemu-devel@nongnu.org; Wed, 11 Mar 2015 02:48:46 -0400 Date: Wed, 11 Mar 2015 07:48:43 +0100 From: "Michael S. Tsirkin" Message-ID: <20150311074415-mutt-send-email-mst@redhat.com> References: <1426053572-21326-1-git-send-email-rusty@rustcorp.com.au> <1426053572-21326-3-git-send-email-rusty@rustcorp.com.au> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1426053572-21326-3-git-send-email-rusty@rustcorp.com.au> Subject: Re: [Qemu-devel] [PATCH 2/2] virtio-blk: fix length calculations for write operations. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Rusty Russell Cc: QEMU Developers On Wed, Mar 11, 2015 at 04:29:32PM +1030, Rusty Russell wrote: > We only fill in the 'req->qiov.size' bytes on a (successful) read, > not on a write. > > Signed-off-by: Rusty Russell > --- > hw/block/virtio-blk.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c > index 258bb4c..98d87a9 100644 > --- a/hw/block/virtio-blk.c > +++ b/hw/block/virtio-blk.c > @@ -50,11 +50,19 @@ static void virtio_blk_complete_request(VirtIOBlockReq *req, > { > VirtIOBlock *s = req->dev; > VirtIODevice *vdev = VIRTIO_DEVICE(s); > + int type = virtio_ldl_p(VIRTIO_DEVICE(req->dev), &req->out.type); > > trace_virtio_blk_req_complete(req, status); > > stb_p(&req->in->status, status); > - virtqueue_push(s->vq, &req->elem, req->qiov.size + sizeof(*req->in)); > + > + /* If we didn't succeed, we *may* have written more, but don't > + * count on it. */ I wonder about this. So length as you specify it is <= actually written length. What are the advantages of this approach? How about we do the reverse, specify that the length in descriptor is >= the size actually written? If we do this, all these buggy hosts suddenly become correct, which seems better. > + if (type == VIRTIO_BLK_T_IN && status == VIRTIO_BLK_S_OK) { > + virtqueue_push(s->vq, &req->elem, req->qiov.size + sizeof(*req->in)); > + } else { > + virtqueue_push(s->vq, &req->elem, sizeof(*req->in)); > + } > virtio_notify(vdev, s->vq); > } > > -- > 2.1.0