From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([208.118.235.92]:35522)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <benh@kernel.crashing.org>) id 1SwQxW-0007KR-LM
	for qemu-devel@nongnu.org; Wed, 01 Aug 2012 00:54:59 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <benh@kernel.crashing.org>) id 1SwQxU-0003kC-BU
	for qemu-devel@nongnu.org; Wed, 01 Aug 2012 00:54:58 -0400
Received: from gate.crashing.org ([63.228.1.57]:47861)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <benh@kernel.crashing.org>) id 1SwQxR-0003iX-Db
	for qemu-devel@nongnu.org; Wed, 01 Aug 2012 00:54:56 -0400
Message-ID: <1343796869.16975.48.camel@pasglop>
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: Wed, 01 Aug 2012 14:54:29 +1000
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
Mime-Version: 1.0
Subject: [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: Anthony Liguori <aliguori@us.ibm.com>
Cc: qemu-devel@nongnu.org

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.

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.