From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([208.118.235.92]:42709)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <aliguori@us.ibm.com>) id 1Swh1J-00018B-6F
	for qemu-devel@nongnu.org; Wed, 01 Aug 2012 18:03:58 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <aliguori@us.ibm.com>) id 1Swh1H-0002xQ-SX
	for qemu-devel@nongnu.org; Wed, 01 Aug 2012 18:03:57 -0400
Received: from e9.ny.us.ibm.com ([32.97.182.139]:49482)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <aliguori@us.ibm.com>) id 1Swh1H-0002xJ-Oj
	for qemu-devel@nongnu.org; Wed, 01 Aug 2012 18:03:55 -0400
Received: from /spool/local
	by e9.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only!
	Violators will be prosecuted
	for <qemu-devel@nongnu.org> from <aliguori@us.ibm.com>;
	Wed, 1 Aug 2012 18:03:54 -0400
Received: from d01relay05.pok.ibm.com (d01relay05.pok.ibm.com [9.56.227.237])
	by d01dlp01.pok.ibm.com (Postfix) with ESMTP id 3690638C8068
	for <qemu-devel@nongnu.org>; Wed,  1 Aug 2012 18:03:49 -0400 (EDT)
Received: from d01av01.pok.ibm.com (d01av01.pok.ibm.com [9.56.224.215])
	by d01relay05.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id
	q71M3m0U364760
	for <qemu-devel@nongnu.org>; Wed, 1 Aug 2012 18:03:48 -0400
Received: from d01av01.pok.ibm.com (loopback [127.0.0.1])
	by d01av01.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id
	q723Yeqx010541
	for <qemu-devel@nongnu.org>; Wed, 1 Aug 2012 23:34:41 -0400
From: Anthony Liguori <aliguori@us.ibm.com>
In-Reply-To: <1343796869.16975.48.camel@pasglop>
References: <1343796869.16975.48.camel@pasglop>
Date: Wed, 01 Aug 2012 17:03:40 -0500
Message-ID: <878vdy2rsz.fsf@codemonkey.ws>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Subject: Re: [Qemu-devel] virtio-blk: Something bizarre with
	VIRTIO_BLK_T_GET_ID
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: qemu-devel@nongnu.org

Benjamin Herrenschmidt <benh@kernel.crashing.org> writes:

> Hi Anthony !
>
> I was looking at virtio-blk.c as an example of some details regarding
> the use of virtio queues. One thing I'm implementing is a
> request/reponse model similar to what it does.
>
> One thing I noticed that sounds off to me but I might have missed
> something is the handling of the "GET_ID" request. Qemu does:
>
>     } else if (type & VIRTIO_BLK_T_GET_ID) {
>         VirtIOBlock *s = req->dev;
>
>         /*
>          * NB: per existing s/n string convention the string is
>          * terminated by '\0' only when shorter than buffer.
>          */
>         strncpy(req->elem.in_sg[0].iov_base,
>                 s->blk->serial ? s->blk->serial : "",
>                 MIN(req->elem.in_sg[0].iov_len, VIRTIO_BLK_ID_BYTES));
>         virtio_blk_req_complete(req, VIRTIO_BLK_S_OK);
>         g_free(req);
>     } ...
>
> So it basically writes up to VIRTIO_BLK_ID_BYTES bytes (which is 20)
> into the "in" iov (it doesn't walk the sg list, so it's a bit fishy,
> it assumes the guest is using a single entry here but that's not my
> problem).
>
> However, virtio_blk_req_complete() does:
>
>     virtqueue_push(s->vq, &req->elem, req->qiov.size + sizeof(*req->in));
>
> So it pushes into the queue req->qiov.size (which is 0) + sizeof(*req->in)
> which is as far as I can tell ... 16.

It's completely bogus.  Non-read/write commands probably need special
handling for push.

Regards,

Anthony Liguori

>
> So we don't push enough bytes out basically for the full 20 bytes allowed
> for the ID.
>
> Or am I missing something ?
>
> Cheers,
> Ben.