Asias He <asias@xxxxxxxxxx> writes: > vhost-blk is an in-kernel virito-blk device accelerator. > > Due to lack of proper in-kernel AIO interface, this version converts > guest's I/O request to bio and use submit_bio() to submit I/O directly. > So this version any supports raw block device as guest's disk image, > e.g. /dev/sda, /dev/ram0. We can add file based image support to > vhost-blk once we have in-kernel AIO interface. There are some work in > progress for in-kernel AIO interface from Dave Kleikamp and Zach Brown: > > http://marc.info/?l=linux-fsdevel&m=133312234313122 OK, this generally looks quite neat. There is one significant bug, however: > +/* The block header is in the first and separate buffer. */ > +#define BLK_HDR 0 You need to do a proper pull off the iovec; you can't simply assume this. I'm working on fixing qemu, too. linux/drivers/vhost/net.c simply skips the header, you want something which actually copies it from userspace: /* Returns 0, -EFAULT or -EINVAL (too short) */ int copy_from_iovec_user(void *dst, size_t len, struct iovec *iov, int iov_nr); int copy_to_iovec_user(struct iovec *iov, int iov_nr, const void *src, size_t len); These consume the iov in place. You could pass struct iovec **iov and int * if you wanted to be really efficient (otherwise you have zero-length iov entries at the front after you've pulled things off). This goes away: > + if (hdr->type == VIRTIO_BLK_T_IN || hdr->type == VIRTIO_BLK_T_GET_ID) > + iov_nr = in - 1; > + else > + iov_nr = out - 1; This becomes a simple assignment: > + /* The block data buffer follows block header buffer */ > + req->iov = &vq->iov[BLK_HDR + 1]; This one actually requires iteration, since you should handle the case where the last iov is zero length: > + /* The block status buffer follows block data buffer */ > + req->status = vq->iov[iov_nr + 1].iov_base; This becomes copy_to_iovec_user: > + case VIRTIO_BLK_T_GET_ID: { > + char id[VIRTIO_BLK_ID_BYTES]; > + int len; > + > + ret = snprintf(id, VIRTIO_BLK_ID_BYTES, > + "vhost-blk%d", blk->index); > + if (ret < 0) > + break; > + len = ret; > + ret = __copy_to_user(req->iov[0].iov_base, id, len); This becomes copy_from_iovec_user: > + if (unlikely(copy_from_user(&hdr, vq->iov[BLK_HDR].iov_base, > + sizeof(hdr)))) { > + vq_err(vq, "Failed to get block header!\n"); > + vhost_discard_vq_desc(vq, 1); > + break; > + } The rest looks OK, at a glance. Thanks, Rusty. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
