Re: [RFC v2 1/8] blkio: add io_uring block driver using libblkio

[Stefan Hajnoczi]

On Thu, Apr 07, 2022 at 10:34:07AM +0200, Kevin Wolf wrote:
> Am 07.04.2022 um 10:25 hat Kevin Wolf geschrieben:
> > Am 07.04.2022 um 09:22 hat Stefan Hajnoczi geschrieben:
> > > On Wed, Apr 06, 2022 at 07:32:04PM +0200, Kevin Wolf wrote:
> > > > Am 05.04.2022 um 17:33 hat Stefan Hajnoczi geschrieben:
> > > > > libblkio (https://gitlab.com/libblkio/libblkio/) is a library for
> > > > > high-performance disk I/O. It currently supports io_uring with
> > > > > additional drivers planned.
> > > > > 
> > > > > One of the reasons for developing libblkio is that other applications
> > > > > besides QEMU can use it. This will be particularly useful for
> > > > > vhost-user-blk which applications may wish to use for connecting to
> > > > > qemu-storage-daemon.
> > > > > 
> > > > > libblkio also gives us an opportunity to develop in Rust behind a C 
> > > > > API
> > > > > that is easy to consume from QEMU.
> > > > > 
> > > > > This commit adds an io_uring BlockDriver to QEMU using libblkio. For 
> > > > > now
> > > > > I/O buffers are copied through bounce buffers if the libblkio driver
> > > > > requires it. Later commits add an optimization for pre-registering 
> > > > > guest
> > > > > RAM to avoid bounce buffers. It will be easy to add other libblkio
> > > > > drivers since they will share the majority of code.
> > > > > 
> > > > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > > > 
> > > > > +static BlockDriver bdrv_io_uring = {
> > > > > +    .format_name                = "io_uring",
> > > > > +    .protocol_name              = "io_uring",
> > > > > +    .instance_size              = sizeof(BDRVBlkioState),
> > > > > +    .bdrv_needs_filename        = true,
> > > > > +    .bdrv_parse_filename        = blkio_parse_filename_io_uring,
> > > > > +    .bdrv_file_open             = blkio_file_open,
> > > > > +    .bdrv_close                 = blkio_close,
> > > > > +    .bdrv_getlength             = blkio_getlength,
> > > > > +    .has_variable_length        = true,
> > > > 
> > > > This one is a bad idea. It means that every request will call
> > > > blkio_getlength() first, which looks up the "capacity" property in
> > > > libblkio and then calls lseek() for the io_uring backend.
> > > 
> > > Thanks for pointing this out. I didn't think this through. More below on
> > > what I was trying to do.
> > > 
> > > > For other backends like the vhost_user one (where I just copied your
> > > > definition and then noticed this behaviour), it involve a message over
> > > > the vhost socket, which is even worse.
> > > 
> > > (A vhost-user-blk driver could cache the capacity field and update it
> > > when a Configuration Change Notification is received. There is no need
> > > to send a vhost-user protocol message every time.)
> > 
> > In theory we could cache in libblkio, but then we would need a mechanism
> > to invalidate the cache so we can support resizing an image (similar to
> > what block_resize does in QEMU, except that it wouldn't set the new
> > size from a parameter, but just get the new value from the backend).
> 
> Oh, sorry, I misread. VHOST_USER_SLAVE_CONFIG_CHANGE_MSG is probably
> what you mean, so that the backend triggers the update. It exists in the
> spec, but neither libvhost-user nor rust-vmm seem to support it
> currently. We also don't set up the backchannel yet where this message
> could even be passed.
> 
> So it's an option, but probably only for later because it involves
> extending several places.

Agreed.

Stefan

signature.asc
Description: PGP signature