[GEDI] [PATCH v5 02/12] blkio: add libblkio block driver

Alberto Faria afaria at redhat.com
Thu Oct 6 16:41:39 UTC 2022


On Tue, Sep 27, 2022 at 8:34 PM Stefan Hajnoczi <stefanha at redhat.com> wrote:
> +static int blkio_virtio_blk_vhost_user_open(BlockDriverState *bs,
> +        QDict *options, int flags, Error **errp)
> +{
> +    const char *path = qdict_get_try_str(options, "path");
> +    BDRVBlkioState *s = bs->opaque;
> +    int ret;
> +
> +    if (!path) {
> +        error_setg(errp, "missing 'path' option");
> +        return -EINVAL;
> +    }
> +
> +    ret = blkio_set_str(s->blkio, "path", path);
> +    qdict_del(options, "path");
> +    if (ret < 0) {
> +        error_setg_errno(errp, -ret, "failed to set path: %s",
> +                         blkio_get_error_msg());
> +        return ret;
> +    }
> +
> +    if (!(flags & BDRV_O_NOCACHE)) {
> +        error_setg(errp, "cache.direct=off is not supported");
> +        return -EINVAL;
> +    }
> +    return 0;
> +}
> +
> +static int blkio_virtio_blk_vhost_vdpa_open(BlockDriverState *bs,
> +        QDict *options, int flags, Error **errp)
> +{
> +    const char *path = qdict_get_try_str(options, "path");
> +    BDRVBlkioState *s = bs->opaque;
> +    int ret;
> +
> +    if (!path) {
> +        error_setg(errp, "missing 'path' option");
> +        return -EINVAL;
> +    }
> +
> +    ret = blkio_set_str(s->blkio, "path", path);
> +    qdict_del(options, "path");
> +    if (ret < 0) {
> +        error_setg_errno(errp, -ret, "failed to set path: %s",
> +                         blkio_get_error_msg());
> +        return ret;
> +    }
> +
> +    if (!(flags & BDRV_O_NOCACHE)) {
> +        error_setg(errp, "cache.direct=off is not supported");
> +        return -EINVAL;
> +    }
> +    return 0;
> +}

These two functions are (currently) exact duplicates. Should we just
have a single blkio_virtio_blk_open() function?

> +static BlockDriver bdrv_io_uring = {
> +    .format_name                = DRIVER_IO_URING,
> +    .protocol_name              = DRIVER_IO_URING,
> +    .instance_size              = sizeof(BDRVBlkioState),
> +    .bdrv_needs_filename        = true,
> +    .bdrv_file_open             = blkio_file_open,
> +    .bdrv_close                 = blkio_close,
> +    .bdrv_getlength             = blkio_getlength,
> +    .bdrv_get_info              = blkio_get_info,
> +    .bdrv_attach_aio_context    = blkio_attach_aio_context,
> +    .bdrv_detach_aio_context    = blkio_detach_aio_context,
> +    .bdrv_co_pdiscard           = blkio_co_pdiscard,
> +    .bdrv_co_preadv             = blkio_co_preadv,
> +    .bdrv_co_pwritev            = blkio_co_pwritev,
> +    .bdrv_co_flush_to_disk      = blkio_co_flush,
> +    .bdrv_co_pwrite_zeroes      = blkio_co_pwrite_zeroes,
> +    .bdrv_io_unplug             = blkio_io_unplug,
> +    .bdrv_refresh_limits        = blkio_refresh_limits,
> +};
> +
> +static BlockDriver bdrv_virtio_blk_vhost_user = {
> +    .format_name                = DRIVER_VIRTIO_BLK_VHOST_USER,
> +    .protocol_name              = DRIVER_VIRTIO_BLK_VHOST_USER,
> +    .instance_size              = sizeof(BDRVBlkioState),
> +    .bdrv_file_open             = blkio_file_open,
> +    .bdrv_close                 = blkio_close,
> +    .bdrv_getlength             = blkio_getlength,
> +    .bdrv_get_info              = blkio_get_info,
> +    .bdrv_attach_aio_context    = blkio_attach_aio_context,
> +    .bdrv_detach_aio_context    = blkio_detach_aio_context,
> +    .bdrv_co_pdiscard           = blkio_co_pdiscard,
> +    .bdrv_co_preadv             = blkio_co_preadv,
> +    .bdrv_co_pwritev            = blkio_co_pwritev,
> +    .bdrv_co_flush_to_disk      = blkio_co_flush,
> +    .bdrv_co_pwrite_zeroes      = blkio_co_pwrite_zeroes,
> +    .bdrv_io_unplug             = blkio_io_unplug,
> +    .bdrv_refresh_limits        = blkio_refresh_limits,
> +};
> +
> +static BlockDriver bdrv_virtio_blk_vhost_vdpa = {
> +    .format_name                = DRIVER_VIRTIO_BLK_VHOST_VDPA,
> +    .protocol_name              = DRIVER_VIRTIO_BLK_VHOST_VDPA,
> +    .instance_size              = sizeof(BDRVBlkioState),
> +    .bdrv_file_open             = blkio_file_open,
> +    .bdrv_close                 = blkio_close,
> +    .bdrv_getlength             = blkio_getlength,
> +    .bdrv_get_info              = blkio_get_info,
> +    .bdrv_attach_aio_context    = blkio_attach_aio_context,
> +    .bdrv_detach_aio_context    = blkio_detach_aio_context,
> +    .bdrv_co_pdiscard           = blkio_co_pdiscard,
> +    .bdrv_co_preadv             = blkio_co_preadv,
> +    .bdrv_co_pwritev            = blkio_co_pwritev,
> +    .bdrv_co_flush_to_disk      = blkio_co_flush,
> +    .bdrv_co_pwrite_zeroes      = blkio_co_pwrite_zeroes,
> +    .bdrv_io_unplug             = blkio_io_unplug,
> +    .bdrv_refresh_limits        = blkio_refresh_limits,
> +};

It's difficult to identify the fields that differ between
BlockDrivers. Maybe we could do something like:

    #define DRIVER(name, ...) \
        { \
            .format_name             = name, \
            .protocol_name           = name, \
            .instance_size           = sizeof(BDRVBlkioState), \
            .bdrv_file_open          = blkio_file_open, \
            .bdrv_close              = blkio_close, \
            .bdrv_getlength          = blkio_getlength, \
            .bdrv_get_info           = blkio_get_info, \
            .bdrv_attach_aio_context = blkio_attach_aio_context, \
            .bdrv_detach_aio_context = blkio_detach_aio_context, \
            .bdrv_co_pdiscard        = blkio_co_pdiscard, \
            .bdrv_co_preadv          = blkio_co_preadv, \
            .bdrv_co_pwritev         = blkio_co_pwritev, \
            .bdrv_co_flush_to_disk   = blkio_co_flush, \
            .bdrv_co_pwrite_zeroes   = blkio_co_pwrite_zeroes, \
            .bdrv_io_unplug          = blkio_io_unplug, \
            .bdrv_refresh_limits     = blkio_refresh_limits, \
            __VA_ARGS__
        } \

    static BlockDriver bdrv_io_uring = DRIVER(
        DRIVER_IO_URING,
        .bdrv_needs_filename = true,
    );

    static BlockDriver bdrv_virtio_blk_vhost_user = DRIVER(
        DRIVER_VIRTIO_BLK_VHOST_USER
    );

    static BlockDriver bdrv_virtio_blk_vhost_vdpa = DRIVER(
        DRIVER_VIRTIO_BLK_VHOST_VDPA
    );

Alberto



More information about the integration mailing list