Re: [Qemu-devel] [PATCH v5 10/11] dataplane: add virtio-blk data plane code

[Kevin Wolf <kwolf@redhat.com>]

Am 05.12.2012 21:47, schrieb Stefan Hajnoczi:
> virtio-blk-data-plane is a subset implementation of virtio-blk.  It only
> handles read, write, and flush requests.  It does this using a dedicated
> thread that executes an epoll(2)-based event loop and processes I/O
> using Linux AIO.
> 
> This approach performs very well but can be used for raw image files
> only.  The number of IOPS achieved has been reported to be several times
> higher than the existing virtio-blk implementation.
> 
> Eventually it should be possible to unify virtio-blk-data-plane with the
> main body of QEMU code once the block layer and hardware emulation is
> able to run outside the global mutex.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

> +static int process_request(IOQueue *ioq, struct iovec iov[],
> +                           unsigned int out_num, unsigned int in_num,
> +                           unsigned int head)
> +{
> +    VirtIOBlockDataPlane *s = container_of(ioq, VirtIOBlockDataPlane, ioqueue);
> +    struct iovec *in_iov = &iov[out_num];
> +    struct virtio_blk_outhdr outhdr;
> +    QEMUIOVector *inhdr;
> +    size_t in_size;
> +
> +    /* Copy in outhdr */
> +    if (unlikely(iov_to_buf(iov, out_num, 0, &outhdr,
> +                            sizeof(outhdr)) != sizeof(outhdr))) {
> +        error_report("virtio-blk request outhdr too short");
> +        return -EFAULT;
> +    }
> +    iov_discard(&iov, &out_num, sizeof(outhdr));
> +
> +    /* Grab inhdr for later */
> +    in_size = iov_size(in_iov, in_num);
> +    if (in_size < sizeof(struct virtio_blk_inhdr)) {
> +        error_report("virtio_blk request inhdr too short");
> +        return -EFAULT;
> +    }
> +    inhdr = g_slice_new(QEMUIOVector);
> +    qemu_iovec_init(inhdr, 1);
> +    qemu_iovec_concat_iov(inhdr, in_iov, in_num,
> +            in_size - sizeof(struct virtio_blk_inhdr),
> +            sizeof(struct virtio_blk_inhdr));
> +    iov_discard(&in_iov, &in_num, -sizeof(struct virtio_blk_inhdr));
> +
> +    /* TODO Linux sets the barrier bit even when not advertised! */
> +    outhdr.type &= ~VIRTIO_BLK_T_BARRIER;
> +
> +    struct iocb *iocb;
> +    switch (outhdr.type & (VIRTIO_BLK_T_OUT | VIRTIO_BLK_T_SCSI_CMD |
> +                           VIRTIO_BLK_T_FLUSH)) {
> +    case VIRTIO_BLK_T_IN:
> +        iocb = ioq_rdwr(ioq, true, in_iov, in_num, outhdr.sector * 512);
> +        break;
> +
> +    case VIRTIO_BLK_T_OUT:
> +        iocb = ioq_rdwr(ioq, false, iov, out_num, outhdr.sector * 512);
> +        break;
> +
> +    case VIRTIO_BLK_T_SCSI_CMD:
> +        /* TODO support SCSI commands */
> +        fail_request_early(s, head, inhdr, VIRTIO_BLK_S_UNSUPP);
> +        return 0;
> +
> +    case VIRTIO_BLK_T_FLUSH:
> +        /* TODO fdsync not supported by Linux AIO, do it synchronously here! */
> +        fdatasync(s->fd);

We shouldn't ignore errors here.

> +        fail_request_early(s, head, inhdr, VIRTIO_BLK_S_OK);
> +        return 0;
> +

Kevin