> +static int vhost_blk_req_submit(struct vhost_blk_req *req, struct file *file)
> +{
> +
> +     struct inode *inode = file->f_mapping->host;
> +     struct block_device *bdev = inode->i_bdev;
> +     int ret;

Please just pass the block_device directly instead of a file struct.

> +
> +     ret = vhost_blk_bio_make(req, bdev);
> +     if (ret < 0)
> +             return ret;
> +
> +     vhost_blk_bio_send(req);
> +
> +     return ret;
> +}

Then again how simple the this function is it probably should just go
away entirely.

> +     set_fs(USER_DS);

What do we actually need the set_fs for here?

> +
> +static inline void vhost_blk_stop(struct vhost_blk *blk, struct file **file)
> +{
> +
> +     *file = vhost_blk_stop_vq(blk, &blk->vq);
> +}

What is the point of this helper?  Also I can't see anyone actually
using the returned struct file.

> +     case VIRTIO_BLK_T_FLUSH:
> +             ret = vfs_fsync(file, 1);
> +             status = ret < 0 ? VIRTIO_BLK_S_IOERR : VIRTIO_BLK_S_OK;
> +             if (!vhost_blk_set_status(req, status))
> +                     vhost_add_used_and_signal(&blk->dev, vq, head, ret);
> +             break;

Sending a fsync here is actually wrong in two different ways:

 a) it operates at the filesystem level instead of bio level
 b) it's a blocking operation

It should instead send a REQ_FLUSH bio using the same submission scheme
as the read/write requests.
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Reply via email to