[Qemu-devel] virtio-blk: Something bizarre with VIRTIO_BLK_T_GET_ID

[Benjamin Herrenschmidt <benh@kernel.crashing.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.