Re: [PATCH v2 4/6] vduse-blk: implements vduse-blk export

[Stefan Hajnoczi]

On Tue, Feb 15, 2022 at 06:59:41PM +0800, Xie Yongji wrote:
> This implements a VDUSE block backends based on
> the libvduse library. We can use it to export the BDSs
> for both VM and container (host) usage.
> 
> The new command-line syntax is:
> 
> $ qemu-storage-daemon \
>     --blockdev file,node-name=drive0,filename=test.img \
>     --export vduse-blk,node-name=drive0,id=vduse-export0,writable=on
> 
> After the qemu-storage-daemon started, we need to use
> the "vdpa" command to attach the device to vDPA bus:
> 
> $ vdpa dev add name vduse-export0 mgmtdev vduse

The per-QEMU export id is used as the global vdpa device name. If this
becomes a problem in the future then a new --export
vduse-blk,vdpa-dev-name= option can be added.

> +    case VIRTIO_BLK_T_GET_ID: {
> +        size_t size = MIN(iov_size(&elem->in_sg[0], in_num),
> +                          VIRTIO_BLK_ID_BYTES);
> +        snprintf(elem->in_sg[0].iov_base, size, "%s", vblk_exp->export.id);

Please use iov_from_buf(). The driver is allowed to submit as many
in_sg[] elements as it wants and a compliant virtio-blk device
implementation must support that.

Here is the VIRTIO specification section that covers message framing:
https://docs.oasis-open.org/virtio/virtio/v1.1/cs01/virtio-v1.1-cs01.html#x1-280004

> +    features = (1ULL << VIRTIO_F_IOMMU_PLATFORM) |
> +               (1ULL << VIRTIO_F_VERSION_1) |
> +               (1ULL << VIRTIO_RING_F_EVENT_IDX) |
> +               (1ULL << VIRTIO_F_NOTIFY_ON_EMPTY) |
> +               (1ULL << VIRTIO_RING_F_INDIRECT_DESC) |
> +               (1ULL << VIRTIO_BLK_F_SIZE_MAX) |
> +               (1ULL << VIRTIO_BLK_F_SEG_MAX) |
> +               (1ULL << VIRTIO_BLK_F_TOPOLOGY) |
> +               (1ULL << VIRTIO_BLK_F_BLK_SIZE);

The VIRTIO_F_ and VIRTIO_RING_F_ feature bits report the capabilities of
libvduse. They should probably be defined in libvduse. That way no
changes to vduse-blk.c are required when libvduse changes:

  features = LIBVDUSE_VIRTIO_FEATURES |
             (1ULL << VIRTIO_BLK_F_SIZE_MAX) |
             ...;

signature.asc
Description: PGP signature