On Thu, Sep 26, 2019 at 5:11 PM Bart Van Assche <bvanassche@xxxxxxx> wrote: > >>> +struct ibnbd_dev *ibnbd_dev_open(const char *path, fmode_t flags, > >>> + enum ibnbd_io_mode mode, struct bio_set *bs, > >>> + ibnbd_dev_io_fn io_cb) > >>> +{ > >>> + struct ibnbd_dev *dev; > >>> + int ret; > >>> + > >>> + dev = kzalloc(sizeof(*dev), GFP_KERNEL); > >>> + if (!dev) > >>> + return ERR_PTR(-ENOMEM); > >>> + > >>> + if (mode == IBNBD_BLOCKIO) { > >>> + dev->blk_open_flags = flags; > >>> + ret = ibnbd_dev_blk_open(dev, path, dev->blk_open_flags); > >>> + if (ret) > >>> + goto err; > >>> + } else if (mode == IBNBD_FILEIO) { > >>> + dev->blk_open_flags = FMODE_READ; > >>> + ret = ibnbd_dev_blk_open(dev, path, dev->blk_open_flags); > >>> + if (ret) > >>> + goto err; > >>> + > >>> + ret = ibnbd_dev_vfs_open(dev, path, flags); > >>> + if (ret) > >>> + goto blk_put; > >> > >> This looks really weird. Why to call ibnbd_dev_blk_open() first for file > >> I/O mode? Why to set dev->blk_open_flags to FMODE_READ in file I/O mode? Bart, would it in your opinion be OK to drop the file_io support in IBNBD entirely? We implemented this feature in the beginning of the project to see whether it could be beneficial in some use cases, but never actually found any.