Re: [Qemu-devel] [PATCH v6 4/4] contrib/vhost-user-blk: introduce a vhost-user-blk sample application

[Stefan Hajnoczi <stefanha@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2273 bytes --]

> +static int vub_virtio_process_req(VubDev *vdev_blk,
> +                                     VuVirtq *vq)
> +{
> +    VugDev *gdev = &vdev_blk->parent;
> +    VuDev *vu_dev = &gdev->parent;
> +    VuVirtqElement *elem;
> +    uint32_t type;
> +    unsigned in_num;
> +    unsigned out_num;
> +    VubReq *req;
> +
> +    elem = vu_queue_pop(vu_dev, vq, sizeof(VuVirtqElement));
> +    if (!elem) {
> +        return -1;
> +    }
> +
> +    /* refer to hw/block/virtio_blk.c */
> +    if (elem->out_num < 1 || elem->in_num < 1) {
> +        fprintf(stderr, "virtio-blk request missing headers\n");
> +        free(elem);
> +        return -1;
> +    }
> +
> +    req = g_new0(VubReq, 1);
> +    req->vdev_blk = vdev_blk;
> +    req->vq = vq;
> +    req->elem = elem;
> +
> +    in_num = elem->in_num;
> +    out_num = elem->out_num;
> +
> +    /* don't support VIRTIO_F_ANY_LAYOUT and virtio 1.0 only */
> +    if (elem->out_sg[0].iov_len < sizeof(struct virtio_blk_outhdr)) {
> +        fprintf(stderr, "Invalid outhdr size\n");
> +        goto err;
> +    }

QEMU has iov_discard_front() and iov_discard_back().  They make it
pretty easy to support VIRTIO_F_ANY_LAYOUT.  If you have time, please
consider adding it to libvhost-user, but it's not a requirement for this
patch series.

> +    req->out = (struct virtio_blk_outhdr *)elem->out_sg[0].iov_base;
> +    out_num--;
> +
> +    if (elem->in_sg[in_num - 1].iov_len < sizeof(struct virtio_blk_inhdr)) {
> +        fprintf(stderr, "Invalid inhdr size\n");
> +        goto err;
> +    }
> +    req->in = (struct virtio_blk_inhdr *)elem->in_sg[in_num - 1].iov_base;
> +    in_num--;
> +
> +    type = le32toh(req->out->type);

Endianness is more complicated if you want to support both VIRTIO 1.0
and Legacy (0.9.7).  I guess it's okay to make libvhost-user code only
support VIRTIO 1.0.
> +static void vub_queue_set_started(VuDev *vu_dev, int idx, bool started)
> +{
> +    VuVirtq *vq;
> +
> +    assert(vu_dev);
> +
> +    if ((idx < 0) || (idx >= VHOST_MAX_NR_VIRTQUEUE)) {
> +        fprintf(stderr, "VQ Index out of range: %d\n", idx);
> +        vub_panic_cb(vu_dev, NULL);
> +        return;
> +    }

Is it necessary to check num_queues?  The vhost-user master should not
be able to enable idx >= num_queues.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]