Re: [RFC] vhost-blk implementation

[Badari Pulavarty <pbadari@us.ibm.com>]

On Mon, 2010-04-05 at 15:22 +0100, Stefan Hajnoczi wrote:
> On Mon, Mar 29, 2010 at 4:41 PM, Badari Pulavarty <pbadari@us.ibm.com> wrote:
> > +static void handle_io_work(struct work_struct *work)
> > +{
> > +       struct vhost_blk_io *vbio;
> > +       struct vhost_virtqueue *vq;
> > +       struct vhost_blk *blk;
> > +       int i, ret = 0;
> > +       loff_t pos;
> > +       uint8_t status = 0;
> > +
> > +       vbio = container_of(work, struct vhost_blk_io, work);
> > +       blk = vbio->blk;
> > +       vq = &blk->dev.vqs[0];
> > +       pos = vbio->sector << 8;
> > +
> > +       use_mm(blk->dev.mm);
> > +
> > +       if (vbio->type & VIRTIO_BLK_T_FLUSH)  {
> > +               ret = vfs_fsync(vbio->file, vbio->file->f_path.dentry, 1);
> > +       } else if (vbio->type & VIRTIO_BLK_T_OUT) {
> > +               ret = vfs_writev(vbio->file, vbio->iov, vbio->nvecs, &pos);
> > +       } else {
> > +               ret = vfs_readv(vbio->file, vbio->iov, vbio->nvecs, &pos);
> > +       }
> > +
> > +       status = (ret < 0) ? VIRTIO_BLK_S_IOERR : VIRTIO_BLK_S_OK;
> > +       if (copy_to_user(vbio->iov[vbio->nvecs].iov_base, &status, sizeof status) < 0) {
> > +               printk("copy to user failed\n");
> > +               vhost_discard_vq_desc(vq);
> > +               unuse_mm(blk->dev.mm);
> > +               return;
> 
> Do you need to kfree(vbio) here?

Yes. I do. As mentioned earlier, I haven't fixed error handling yet :(
> 
> > +static long vhost_blk_set_backend(struct vhost_blk *n, unsigned index, int fd)
> > +{
> > +       struct file *file;
> > +       struct vhost_virtqueue *vq;
> > +
> > +       file = fget(fd);
> > +       if (!file)
> > +               return -EBADF;
> > +
> > +       vq = n->vqs + index;
> > +       mutex_lock(&vq->mutex);
> > +       rcu_assign_pointer(vq->private_data, file);
> > +       mutex_unlock(&vq->mutex);
> > +       return 0;
> > +}
> > +
> > +
> > +static long vhost_blk_ioctl(struct file *f, unsigned int ioctl,
> > +                            unsigned long arg)
> > +{
> > +       struct vhost_blk *n = f->private_data;
> > +       void __user *argp = (void __user *)arg;
> > +       struct vhost_vring_file backend;
> > +       int r;
> > +
> > +       switch (ioctl) {
> > +        case VHOST_NET_SET_BACKEND:
> > +               r = copy_from_user(&backend, argp, sizeof backend);
> > +               if (r < 0)
> > +                       return r;
> > +               return vhost_blk_set_backend(n, backend.index, backend.fd);
> 
> I don't see backend.index being checked against VHOST_BLK_VQ_MAX.

Yep. You are right. I will add these checks for my next revision.

Thanks,
Badari