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

[Stefan Hajnoczi <stefanha@gmail.com>]

On Fri, Dec 07, 2012 at 07:04:39PM +0100, Kevin Wolf wrote:
> 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.

Fixed, thanks.

Stefan