[libvirt] [PATCH 1/3] qemu: Need to assign PCI address to vhost-scsi

[Laine Stump]

On 12/12/2017 09:22 AM, John Ferlan wrote:
> Commit id '70249927b' neglected to cover this case because the test
> had taken the "shortcut" to already add the <address>, so I altered
> that too just to make sure it doesn't happen again.
>
> Previously the qemuxml2xmloutdata was a softlink to the source
> qemuxml2argvdata, so I unlinked and recreated the output file to
> force generation of the adddress. Without the test changes, an
> address generation returns:
>
>     libvirt: Domain Config error : internal error: Cannot automatically
>     add a new PCI bus for a device with connect flags 00
>
> if an address was supplied in the test, a restart of libvirtd or
> edit of a guest would display the following opaque message:
>
>     warning : qemuDomainCollectPCIAddress:1237 :
>     qemuDomainDeviceCalculatePCIConnectFlags() thinks that the device
>      with PCI address 0000:00:09.0 should not have a PCI address
>
> where the address is related to the guest PCI address provided.
>
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
>  src/qemu/qemu_domain_address.c                     |  3 +-
>  .../hostdev-scsi-vhost-scsi-ccw.xml                |  1 -
>  .../hostdev-scsi-vhost-scsi-pci.xml                |  1 -
>  .../hostdev-scsi-vhost-scsi-ccw.xml                | 35 +++++++++++++++++-
>  .../hostdev-scsi-vhost-scsi-pci.xml                | 43 +++++++++++++++++++++-
>  5 files changed, 78 insertions(+), 5 deletions(-)
>  mode change 120000 => 100644 tests/qemuxml2xmloutdata/hostdev-scsi-vhost-scsi-ccw.xml
>  mode change 120000 => 100644 tests/qemuxml2xmloutdata/hostdev-scsi-vhost-scsi-pci.xml
>
> diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
> index a40cdb399..6e7561d39 100644
> --- a/src/qemu/qemu_domain_address.c
> +++ b/src/qemu/qemu_domain_address.c
> @@ -623,7 +623,8 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev,
>  
>          if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS ||
>              (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI &&
> -             hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV)) {
> +             hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV &&
> +             hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST)) {
>              return 0;
>          }

Sorry I missed this before. The fix above doesn't work for domains with
PCIe root (i.e. Q35) (well okay, it *does* work, but only by accident -
see below) because it will end up creating a virPCIDevice from
"hostAddr", which was initialized to point to
hostdev->source.subsys.u.pci.addr, which is only valid for PCI hostdevs,
not for SCSI_HOST or MDEV - this is why there is an early return for
MDEV devices (right before the calle to virPCIDeviceNew(). In the case
of mdev, we make the assumption that pcieFlags should be returned (i.e.
the device should be on a PCIe controller in the guest) because all
currently existing MDEV devices are in fact Express devices.

Since the code as it is now will create a nonsense PCI address and try
to read data from the host sysfs based on that gibberish, it will end up
returning -1 for virPCIDeviceIsPCIExpress.

Hmm, so it *does* end up working (if vhost-scsi devices are Express
devices), but only by chance, since -1 == true, so pcieFlags will be
returned, which is what you want *iff vhost-scsi devices are Express
devices*.

But I don't know if vhost-scsi devices are Express or legacy PCI - if
you don't have any concrete idea about this, you could create a Q35
guest, add a vhost-scsi device with PCI address that's on a
pcie-root-port (it will be, based on my reading of the code), then look
at "lspci -v" for the SCSI device in the guest. I can help with looking
at the device if you can create a Q35 guest with a vhost-scsi device (I
don't have anything to connect such a device to).