[libvirt] [PATCH v6 06/17] qemu: assign virtio devices to PCIe slot when appropriate

[Andrea Bolognani]

On Mon, 2016-11-07 at 14:50 -0500, Laine Stump wrote:
> libvirt previously assigned nearly all devices to a "hotpluggable"
> legacy PCI slot even on machines with a PCIe root bus (and even though
> most such machines don't even support hotplug on legacy PCI slots!)
> Forcing all devices onto legacy PCI slots means that the domain will
> need a dmi-to-pci-bridge (to convert from PCIe to legacy PCI) and a
> pci-bridge (to provide hotpluggable legacy PCI slots which, again,
> usually aren't hotpluggable anyway).
> 
> To help reduce the need for these legacy controllers, this patch tries
> to assign virtio-1.0-capable devices to PCIe slots whenever possible,
> by setting appropriate connectFlags in
> virDomainCalculateDevicePCIConnectFlags(). Happily, when that function
> was written (just a few commits ago) it was created with a
> "virtioFlags" argument, set by both of its callers, which is the
> proper connectFlags to set for any virtio-*-pci device - depending on
> the arch/machinetype of the domain, and whether or not the qemu binary
> supports virtio-1.0, that flag will have either been set to PCI or
> PCIE. This patch merely enables the functionality by setting the flags

s/PCIE/PCIe/

> for the device to whatever is in virtioFlags if the device is a
> virtio-*-pci device.
> 
> NB: the first virtio video device will be placed directly on bus 0
> slot 1 rather than on a pcie-root-port due to the override for primary
> video devices in qemuDomainValidateDevicePCISlotsQ35(). Whether or not
> to change that is a topic of discussion, but this patch doesn't change
> that particular behavior.
> 
> NB2: since the slot must be hotpluggable, and pcie-root (the PCIe root
> complex) does *not* support hotplug, this means that suitable
> controllers must also be in the config (i.e. either pcie-root-port, or
> pcie-downstream-port). For now, libvirt doesn't add those
> automatically, so if you put virtio devices in a config for a qemu
> that has PCIe-capable virtio devices, you'll need to add extra
> pcie-root-ports yourself. That requirement will be eliminated in a
> future patch, but for now, it's simple to do this:
> 
>    <controller type='pci' model='pcie-root-port'/>
>    <controller type='pci' model='pcie-root-port'/>
>    <controller type='pci' model='pcie-root-port'/>
>    ...
> 
> Partially Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1330024

[...]
> @@ -471,9 +470,28 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev,
>              }
>  
>          case VIR_DOMAIN_CONTROLLER_TYPE_IDE:
> +            return pciFlags;
> +
>          case VIR_DOMAIN_CONTROLLER_TYPE_SCSI:
> +            switch ((virDomainControllerModelSCSI)cont->model) {

Add a space before 'cont->model' for consistency.

> @@ -584,14 +608,13 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev,
>              return 0;
>          }
>  
> -

Spurious change, but it's better to fix it here than to leave
it as it is :)

[...]
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-q35-pcie.xml
> @@ -0,0 +1,61 @@
> +<domain type='qemu'>
> +  <name>q35-test</name>
> +  <uuid>11dbdcdd-4c3b-482b-8903-9bdb8c0a2774</uuid>
> +  <memory unit='KiB'>2097152</memory>
> +  <currentMemory unit='KiB'>2097152</currentMemory>
> +  <vcpu placement='static' cpuset='0-1'>2</vcpu>
> +  <os>
> +    <type arch='x86_64' machine='q35'>hvm</type>
> +    <boot dev='hd'/>
> +  </os>
> +  <clock offset='utc'/>
> +  <on_poweroff>destroy</on_poweroff>
> +  <on_reboot>restart</on_reboot>
> +  <on_crash>destroy</on_crash>
> +  <devices>
> +    <emulator>/usr/libexec/qemu-kvm</emulator>
> +    <controller type='pci' model='pcie-root'/>
> +    <controller type='pci' model='dmi-to-pci-bridge'/>
> +    <controller type='pci' model='pci-bridge'/>

These controllers are still needed at this point, but we
should drop them from the input XML once that's no longer
the case, eg. definitely after patch 12/17.

Unless of course you've already thought of that and added
more minimal use cases down the line, which I've simply
not yet gotten to...

> +    <controller type='pci' model='pcie-root-port'/>
> +    <controller type='pci' model='pcie-root-port'/>
> +    <controller type='pci' model='pcie-root-port'/>
> +    <controller type='pci' model='pcie-root-port'/>
> +    <controller type='pci' model='pcie-root-port'/>
> +    <controller type='pci' model='pcie-root-port'/>
> +    <controller type='pci' model='pcie-root-port'/>
> +    <controller type='pci' model='pcie-root-port'/>
> +    <controller type='pci' model='pcie-root-port'/>
> +    <controller type='pci' model='pcie-root-port'/>
> +    <controller type='pci' model='pcie-root-port'/>
> +    <controller type='pci' model='pcie-root-port'/>

... which you probably have, because we definitely need
to make sure libvirt can add at least these for us :)


ACK with the minor stuff mentioned above taken care of.

-- 
Andrea Bolognani / Red Hat / Virtualization