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

[Kevin Wolf <kwolf@redhat.com>]

Am 06.04.2022 um 09:59 hat Xie Yongji geschrieben:
> 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
> 
> Also the device must be removed via the "vdpa" command
> before we stop the qemu-storage-daemon.
> 
> Signed-off-by: Xie Yongji <xieyongji@bytedance.com>

The request handling code is almos the same as for the vhost-user-blk
export. I wonder if we could share this code instead of copying.

The main difference seems to be that you chose not to support discard
and write_zeroes yet. I'm curious if there is a reason why the
vhost-user-blk code wouldn't work for vdpa there?

> +    features = vduse_get_virtio_features() |
> +               (1ULL << VIRTIO_BLK_F_SIZE_MAX) |
> +               (1ULL << VIRTIO_BLK_F_SEG_MAX) |
> +               (1ULL << VIRTIO_BLK_F_TOPOLOGY) |
> +               (1ULL << VIRTIO_BLK_F_BLK_SIZE);
> +
> +    if (num_queues > 1) {
> +        features |= 1ULL << VIRTIO_BLK_F_MQ;
> +    }
> +    if (!vblk_exp->writable) {
> +        features |= 1ULL << VIRTIO_BLK_F_RO;
> +    }

VIRTIO_BLK_F_FLUSH seems to be missing even though the flush command is
implemented.

(This is not a full review yet, just two or three things I noticed while
having a quick look.)

Kevin