On 12/18/2017 10:35 AM, Laine Stump wrote:
Commit 10c73bf1 fixed a bug that I had introduced back in commit 70249927 - if a vhost-scsi device had no manually assigned PCI address, one wouldn't be assigned automatically. There was a slight problem with the logic of the fix though - in the case of domains with pcie-root (e.g. those with a q35 machinetype), qemuDomainDeviceCalculatePCIConnectFlags() will attempt to determine if the host-side PCI device is Express or legacy by examining sysfs based on the host-side PCI address stored in hostdev->source.subsys.u.pci.addr, but that part of the union is only valid for PCI hostdevs, *not* for SCSI hostdevs. So we end up trying to read sysfs for some probably-non-existent device, which fails, and the function virPCIDeviceIsPCIExpress() returns failure (-1). By coincidence, the return value is being examined as a boolean, and since -1 is true, we still end up assigning the vhost-scsi device to an Express slot, but that is just be chance (and could fail in the case that the gibberish in the "hostside PCI address" was the address of a real device that happened to be legacy PCI).
^^ Probably something that should be fixed as a separate patch? Hazards of the undocumented virPCIDeviceIsPCIExpress return values.
Since (according to Paolo Bonzini at least) vhost-scsi devices appear just like virtio-scsi devices in the guest, they should follow the same rules as virtio devices when deciding whether they should be placed in an Express or a legacy slot. That's accomplished in this patch by returning early with virtioFlags, rather than erroneously using hostdev->source.subsys.u.pci.addr. It also adds a test case for PCIe to assure it doesn't get broken in the future. --- src/qemu/qemu_domain_address.c | 7 ++++ .../hostdev-scsi-vhost-scsi-pcie.args | 25 ++++++++++++++ .../hostdev-scsi-vhost-scsi-pcie.xml | 23 +++++++++++++ tests/qemuxml2argvtest.c | 7 ++++ .../hostdev-scsi-vhost-scsi-pcie.xml | 40 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 7 ++++ 6 files changed, 109 insertions(+) create mode 100644 tests/qemuxml2argvdata/hostdev-scsi-vhost-scsi-pcie.args create mode 100644 tests/qemuxml2argvdata/hostdev-scsi-vhost-scsi-pcie.xml create mode 100644 tests/qemuxml2xmloutdata/hostdev-scsi-vhost-scsi-pcie.xml
Reviewed-by: John Ferlan <jferlan@redhat.com> John Thanks - I was wondering about PCIE, but figured/assumed that since it's an HBA sitting on a bus as opposed to a LUN that we're making it look like a disk on the guest, that the code would magically work. Just seemed different enough from a MDEV which is to me more like how a vHBA LUN would be handled as a "child" (so to speak) of some device that's sitting on a bus.