Re: [PATCH v3 08/15] block/file-posix.c: extend to use io_uring

[Stefan Hajnoczi]

On Mon, Jan 13, 2020 at 12:49:27PM +0100, Stefano Garzarella wrote:
> On Wed, Dec 18, 2019 at 04:32:21PM +0000, Stefan Hajnoczi wrote:
> > @@ -503,9 +504,11 @@ static int raw_open_common(BlockDriverState *bs, QDict 
> > *options,
> >          goto fail;
> >      }
> >  
> > -    aio_default = (bdrv_flags & BDRV_O_NATIVE_AIO)
> > -                  ? BLOCKDEV_AIO_OPTIONS_NATIVE
> > -                  : BLOCKDEV_AIO_OPTIONS_THREADS;
> > +    if (bdrv_flags & BDRV_O_NATIVE_AIO) {
> > +        aio_default = BLOCKDEV_AIO_OPTIONS_NATIVE;
> > +    } else {
> > +        aio_default = BLOCKDEV_AIO_OPTIONS_THREADS;
> > +    }
> 
> This is only a cosmetic change?
...
> > @@ -578,7 +585,7 @@ static int raw_open_common(BlockDriverState *bs, QDict 
> > *options,
> >      s->shared_perm = BLK_PERM_ALL;
> >  
> >  #ifdef CONFIG_LINUX_AIO
> > -     /* Currently Linux does AIO only for files opened with O_DIRECT */
> > +    /* Currently Linux does AIO only for files opened with O_DIRECT */
> 
> Also this is a not related fix, if you respin maybe we should split in a
> new patch or say something in the commit message.

Thanks, I'll remove whitespace changes and unrelated reformatting from
this patch.

> > @@ -1877,21 +1900,25 @@ static int coroutine_fn raw_co_prw(BlockDriverState 
> > *bs, uint64_t offset,
> >          return -EIO;
> >  
> >      /*
> > -     * Check if the underlying device requires requests to be aligned,
> > -     * and if the request we are trying to submit is aligned or not.
> > -     * If this is the case tell the low-level driver that it needs
> > -     * to copy the buffer.
> > +     * When using O_DIRECT, the request must be aligned to be able to use
> > +     * either libaio or io_uring interface. If not fail back to regular 
> > thread
> > +     * pool read/write code which emulates this for us if we
> > +     * set QEMU_AIO_MISALIGNED.
> >       */
> > -    if (s->needs_alignment) {
> > -        if (!bdrv_qiov_is_aligned(bs, qiov)) {
> > -            type |= QEMU_AIO_MISALIGNED;
> > +    if (s->needs_alignment && !bdrv_qiov_is_aligned(bs, qiov)) {
> > +        type |= QEMU_AIO_MISALIGNED;
> > +#ifdef CONFIG_LINUX_IO_URING
> > +    } else if (s->use_linux_io_uring) {
> > +        LuringState *aio = 
> > aio_get_linux_io_uring(bdrv_get_aio_context(bs));
> > +        assert(qiov->size == bytes);
> > +        return luring_co_submit(bs, aio, s->fd, offset, qiov, type);
> > +#endif
> >  #ifdef CONFIG_LINUX_AIO
> > -        } else if (s->use_linux_aio) {
> > -            LinuxAioState *aio = 
> > aio_get_linux_aio(bdrv_get_aio_context(bs));
> > -            assert(qiov->size == bytes);
> > -            return laio_co_submit(bs, aio, s->fd, offset, qiov, type);
> > +    } else if (s->use_linux_aio) {
> 
> This code block was executed if "s->needs_alignment" was true, now we don't
> check it. Could this be a problem?

From raw_open_common():

  /* Currently Linux does AIO only for files opened with O_DIRECT */
  if (s->use_linux_aio) {
      if (!(s->open_flags & O_DIRECT)) {
          error_setg(errp, "aio=native was specified, but it requires "
                           "cache.direct=on, which was not specified.");
          ret = -EINVAL;
          goto fail;

There is no change in behavior since use_linux_aio is only true when
needs_alignment is set.

signature.asc
Description: PGP signature