On Wed, Feb 03, 2021 at 11:04:04AM +0100, Peter Krempa wrote:
On Tue, Feb 02, 2021 at 16:04:12 +0100, Pavel Hrdina wrote:
Implements QEMU support for vhost-user-blk together with live hotplug/unplug.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_block.c | 16 ++++ src/qemu/qemu_block.h | 5 + src/qemu/qemu_command.c | 91 +++++++++++++++++-- src/qemu/qemu_command.h | 8 ++ src/qemu/qemu_domain.c | 5 + src/qemu/qemu_hotplug.c | 5 +- src/qemu/qemu_validate.c | 13 +++ .../disk-vhostuser.x86_64-latest.args | 41 +++++++++ tests/qemuxml2argvtest.c | 1 + 9 files changed, 176 insertions(+), 9 deletions(-) create mode 100644 tests/qemuxml2argvdata/disk-vhostuser.x86_64-latest.args
[...]
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 3e652e18b7..059126cfeb 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1712,9 +1712,16 @@ qemuBuildDiskDeviceStr(const virDomainDef *def, break;
case VIR_DOMAIN_DISK_BUS_VIRTIO: - if (qemuBuildVirtioDevStr(&opt, "virtio-blk", qemuCaps, - VIR_DOMAIN_DEVICE_DISK, disk) < 0) { - return NULL; + if (virStorageSourceGetActualType(disk->src) == VIR_STORAGE_TYPE_VHOST_USER) { + if (qemuBuildVirtioDevStr(&opt, "vhost-user-blk", qemuCaps, + VIR_DOMAIN_DEVICE_DISK, disk) < 0) { + return NULL; + } + } else { + if (qemuBuildVirtioDevStr(&opt, "virtio-blk", qemuCaps, + VIR_DOMAIN_DEVICE_DISK, disk) < 0) { + return NULL; + }
Preferably, add a temporary string holding either "vhost-user-blk" or "virtio-blk" and call qemuBuildVirtioDevStr just once, since all other arguments are the same.
This would require adding one level of nesting because of the temporary variable or I would have to define the variable at the top of the function. Not sure if it makes it better.
}
if (disk->iothread)
[...]
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 0c078a9388..aa6d539610 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -10581,6 +10581,11 @@ qemuDomainPrepareDiskSource(virDomainDiskDefPtr disk, disk->src->format = VIR_STORAGE_FILE_RAW; }
+ /* Nothing else to prepare as it will use -chardev instead + * of -blockdev/-drive option. */ + if (disk->src->type == VIR_STORAGE_TYPE_VHOST_USER) + return 0;
The code above sets 'format' for storage pool backed disks, but since format isn't something we'd be setting for vhost-user this exemption should be at the beginning of the function.
Fixed.
+ if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV) && !qemuDiskBusIsSD(disk->bus)) { if (qemuDomainPrepareDiskSourceBlockdev(disk, priv, cfg) < 0)
Missing stuff from this commit:
- hot-unplug support - see qemuDomainRemoveDiskDevice where the chardev isn't deleted - blockjob lockout - qemuDomainDiskBlockJobIsSupported - block iotune lockout - possible better lockout for qemuDomainBlockPeek - it will barf either that the file is not 'raw' or that it can't be initialized - possible better lockout for qemuDomainSetBlockThreshold - "threshold currently can't be set for block device '%s'" <- it will never be possible - lockout for qemuDomainGetBlockInfo - lockout for qemuDomainBlockResize - lockout for qemuDomainBlockStats(Flags) - handling in bulk stats (qemuDomainGetStatsBlockExportDisk ?)
Well, that's a lot of missing things, thanks for the pointers, I'll look into it.