From mboxrd@z Thu Jan  1 00:00:00 1970
From: Badari Pulavarty <pbadari@us.ibm.com>
Subject: Re: [RFC] vhost-blk implementation
Date: Mon, 05 Apr 2010 19:27:27 -0700
Message-ID: <1270520847.28348.17.camel@badari-desktop>
References: <1269306023.7931.72.camel@badari-desktop>
	 <20100324200402.GA22272@infradead.org>
	 <1269877312.7931.93.camel@badari-desktop>
	 <r2wfbd9d3991004050722lfbe6b0faoeb3090185ccedaa4@mail.gmail.com>
Mime-Version: 1.0
Content-Type: text/plain
Content-Transfer-Encoding: 7bit
Cc: kvm@vger.kernel.org
To: Stefan Hajnoczi <stefanha@gmail.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from e38.co.us.ibm.com ([32.97.110.159]:43577 "EHLO
	e38.co.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1757052Ab0DFC10 (ORCPT <rfc822;kvm@vger.kernel.org>);
	Mon, 5 Apr 2010 22:27:26 -0400
Received: from d03relay03.boulder.ibm.com (d03relay03.boulder.ibm.com [9.17.195.228])
	by e38.co.us.ibm.com (8.14.3/8.13.1) with ESMTP id o362LOgr005169
	for <kvm@vger.kernel.org>; Mon, 5 Apr 2010 20:21:24 -0600
Received: from d03av03.boulder.ibm.com (d03av03.boulder.ibm.com [9.17.195.169])
	by d03relay03.boulder.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id o362RFxt132944
	for <kvm@vger.kernel.org>; Mon, 5 Apr 2010 20:27:17 -0600
Received: from d03av03.boulder.ibm.com (loopback [127.0.0.1])
	by d03av03.boulder.ibm.com (8.14.3/8.13.1/NCO v10.0 AVout) with ESMTP id o35JRFnq015687
	for <kvm@vger.kernel.org>; Mon, 5 Apr 2010 13:27:15 -0600
In-Reply-To: <r2wfbd9d3991004050722lfbe6b0faoeb3090185ccedaa4@mail.gmail.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

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