Re: [Qemu-devel] [PATCH v6 04/14] block/io_uring: implements interfaces for io_uring

[Stefan Hajnoczi]

On Fri, Jul 19, 2019 at 07:05:20PM +0530, Aarushi Mehta wrote:
> diff --git a/block.c b/block.c
> index 29e931e217..4aa3500ad8 100644
> --- a/block.c
> +++ b/block.c
> @@ -844,6 +844,28 @@ static BlockdevDetectZeroesOptions 
> bdrv_parse_detect_zeroes(QemuOpts *opts,
>      return detect_zeroes;
>  }
>  
> +/**
> + * Set flags for aio engine

Other parse_*() functions are more specific about which type of flags
are produced:

s/flags/open flags/

> + *
> + * Return 0 on success, -1 if the engine specifies is invalid

s/specifies/specified/

> + */
> +int bdrv_parse_aio(const char *mode, int *flags)
> +{
> +    if (!strcmp(mode, "threads")) {
> +        /* do nothing, default */
> +    } else if (!strcmp(mode, "native")) {
> +        *flags |= BDRV_O_NATIVE_AIO;
> +#ifdef CONFIG_LINUX_IO_URING
> +    } else if (!strcmp(mode, "io_uring")) {
> +        *flags |= BDRV_O_IO_URING;
> +#endif
> +    } else {
> +        return -1;
> +    }
> +
> +    return 0;
> +}

Can you move this change to the patch that uses bdrv_parse_aio()?  I
don't see any callers in this patch.

> +        if (ret < 0) {
> +            if (ret == -EINTR) {
> +                luring_resubmit(s, luringcb);
> +                continue;
> +            }
> +        } else if (!luringcb->qiov) {
> +            goto end;
> +        } else if (total_bytes == luringcb->qiov->size) {
> +                ret = 0;

Indentation is off here.

> +static int luring_do_submit(int fd, LuringAIOCB *luringcb, LuringState *s,
> +                            uint64_t offset, int type)
> +{
> +    struct io_uring_sqe *sqes = &luringcb->sqeq;
> +
> +    switch (type) {
> +    case QEMU_AIO_WRITE:
> +        io_uring_prep_writev(sqes, fd, luringcb->qiov->iov,
> +                             luringcb->qiov->niov, offset);
> +        break;
> +    case QEMU_AIO_READ:
> +        io_uring_prep_readv(sqes, fd, luringcb->qiov->iov,
> +                            luringcb->qiov->niov, offset);
> +        break;
> +    case QEMU_AIO_FLUSH:
> +        io_uring_prep_fsync(sqes, fd, 0);

In QEMU QEMU_AIO_FLUSH is fdatasync so the IORING_FSYNC_DATASYNC flag
can be used for a slightly cheaper fsync operation.

> +        break;
> +    default:
> +        fprintf(stderr, "%s: invalid AIO request type, aborting 0x%x.\n",
> +                        __func__, type);
> +        abort();
> +    }
> +    io_uring_sqe_set_data(sqes, luringcb);
> +
> +    QSIMPLEQ_INSERT_TAIL(&s->io_q.sq_overflow, luringcb, next);

Now that we use sq_overflow unconditionally it can be renamed to
submit_queue, making the code easier to understand.

signature.asc
Description: PGP signature