On Tue, Sep 27, 2022 at 8:34 PM Stefan Hajnoczi <stefa...@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
