Re: [libvirt] [PATCH 1/2] qemu: assign correct type of PCI address for vhost-scsi when using pcie-root
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(a)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.