
On 01/18/2019 10:54 AM, Andrea Bolognani wrote:
On Thu, 2019-01-17 at 12:52 -0500, Cole Robinson wrote: [...]
@@ -1118,6 +1120,8 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { {"virtio-blk-pci-non-transitional", QEMU_CAPS_DEVICE_VIRTIO_BLK_NON_TRANSITIONAL}, {"virtio-net-pci-transitional", QEMU_CAPS_DEVICE_VIRTIO_NET_TRANSITIONAL}, {"virtio-net-pci-non-transitional", QEMU_CAPS_DEVICE_VIRTIO_NET_NON_TRANSITIONAL}, + {"vhost-scsi-pci-transitional", QEMU_CAPS_DEVICE_VHOST_SCSI_TRANSITIONAL}, + {"vhost-scsi-pci-non-transitional", QEMU_CAPS_DEVICE_VHOST_SCSI_NON_TRANSITIONAL}, };
Usual comment about capabilities.
[...]
@@ -5038,10 +5043,11 @@ qemuBuildSCSIVHostHostdevDevStr(const virDomainDef *def, goto cleanup; }
- if (ARCH_IS_S390(def->os.arch)) - virBufferAddLit(&buf, "vhost-scsi-ccw"); - else - virBufferAddLit(&buf, "vhost-scsi-pci"); + if (qemuBuildVirtioTransitional(&buf, "vhost-scsi", qemuCaps, + dev->info->type, + hostsrc->model, NULL, + VIR_DOMAIN_DEVICE_HOSTDEV) < 0) + goto cleanup;
This changes quite a bit: instead of basing the choice of device upon the architecture and supporting -ccw and -pci only, now we base it upon address type and potentially support -device as well as-s390.
I'm assuming the latter is not going to be a problem because there are probably checks along the way preventing us to get there in the first place, but we should definitely make sure we address the former correctly, most likely by setting the address type based on the architecture in postParse().
Either way, once you've made sure the change is actually safe or added code that makes it so, you should switch from the open-coded version to qemuBuildVirtioDevStr(), and only later in a separate commit switch from that to qemuBuildVirtioTransitional().
If I rework BuildVirtioTransitional like I mentioned in a previous response, we don't need to touch this code. Then it can be cleaned up in a separate patch after this series Thanks, Cole