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@xxxxxxxxxx> 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. -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
