Benjamin Herrenschmidt <b...@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.
