On Tue, Mar 15, 2022 at 7:08 PM Stefan Hajnoczi <stefa...@redhat.com> wrote: > > 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. >
Yes. > > + 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. > Got it. > 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) | > ...; It's OK to define the VIRTIO_F_* feature bits in libvduse since daemon might not want to disable it. But maybe not including VIRTIO_RING_F_* feature bits since daemon might want to disable them in some cases. Thanks, Yongji