[libvirt] [PATCH 15/26] qemu: Automatically pick index and model for pci-root controllers
Laine Stump
laine at laine.org
Wed Jun 14 01:52:02 UTC 2017
On 06/02/2017 12:07 PM, Andrea Bolognani wrote:
> pSeries guests will soon need the new information;
> luckily, we can figure it out automatically most of
> the time, so users won't have to bother with it.
> ---
> src/qemu/qemu_domain_address.c | 57 +++++++++++++++++++++-
> .../qemuargv2xmldata/qemuargv2xml-pseries-disk.xml | 5 +-
> .../qemuargv2xml-pseries-nvram.xml | 5 +-
> .../qemuxml2xmlout-panic-pseries.xml | 5 +-
> .../qemuxml2xmlout-ppc64-usb-controller-legacy.xml | 5 +-
> .../qemuxml2xmlout-ppc64-usb-controller.xml | 5 +-
> .../qemuxml2xmlout-pseries-nvram.xml | 5 +-
> .../qemuxml2xmlout-pseries-panic-missing.xml | 5 +-
> .../qemuxml2xmlout-pseries-panic-no-address.xml | 5 +-
> 9 files changed, 87 insertions(+), 10 deletions(-)
>
> diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
> index 2106b34..d46f8f7 100644
> --- a/src/qemu/qemu_domain_address.c
> +++ b/src/qemu/qemu_domain_address.c
> @@ -1856,6 +1856,7 @@ qemuDomainSupportsPCI(virDomainDefPtr def,
>
> static void
> qemuDomainPCIControllerSetDefaultModelName(virDomainControllerDefPtr cont,
> + virDomainDefPtr def,
> virQEMUCapsPtr qemuCaps)
> {
> int *modelName = &cont->opts.pciopts.modelName;
> @@ -1892,6 +1893,9 @@ qemuDomainPCIControllerSetDefaultModelName(virDomainControllerDefPtr cont,
> *modelName = VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PXB_PCIE;
> break;
> case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
> + if (qemuDomainIsPSeries(def))
> + *modelName = VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_SPAPR_PCI_HOST_BRIDGE;
> + break;
Just to verify - *all* the pci-roots on pSeries will be
spapr-pci-host-bridge, even the first one?
> case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
> case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST:
> break;
> @@ -1900,6 +1904,30 @@ qemuDomainPCIControllerSetDefaultModelName(virDomainControllerDefPtr cont,
>
>
> static int
> +qemuDomainAddressFindNewIndex(virDomainDefPtr def)
To help avoid confusion between the target index and the index at the
toplevel, can we call this qemuDomainAddressFindNewTargetIndex()? (yeah,
I know that's really long)
> +{
> + int ret = 1;
> + size_t i;
> +
> + for (i = 0; i < def->ncontrollers; i++) {
> + virDomainControllerDefPtr cont = def->controllers[i];
> +
> + if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) {
> + if (cont->opts.pciopts.idx >= ret)
> + ret = cont->opts.pciopts.idx + 1;
> + }
I know it would be idiotic to do so, but what if someone removed one of
the pci-root controllers, and then later added a new one? You'd end up
with an unused index where the earlier one was removed (and it would
never be filled in). Maybe you should instead start at 0, and scan all
the existing controllers for 0, then for 1, then for 2, etc, until you
find the lowest target index value that isn't used.
> + }
> +
> + /* 0 is reserved for the implicit PHB, whereas anything lower
> + * than 0 or higher than 31 is invalid */
> + if (ret > 31)
> + ret = -1;
> +
> + return ret;
> +}
> +
> +
> +static int
> qemuDomainAddressFindNewBusNr(virDomainDefPtr def)
> {
> /* Try to find a nice default for busNr for a new pci-expander-bus.
> @@ -2159,7 +2187,7 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def,
> * device in qemu) for any controller that doesn't yet
> * have it set.
> */
> - qemuDomainPCIControllerSetDefaultModelName(cont, qemuCaps);
> + qemuDomainPCIControllerSetDefaultModelName(cont, def, qemuCaps);
>
> /* set defaults for any other auto-generated config
> * options for this controller that haven't been
> @@ -2196,9 +2224,34 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def,
> goto cleanup;
> }
> break;
> + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
> + if (!qemuDomainIsPSeries(def))
> + break;
> + if (options->idx == -1) {
> + if (cont->idx == 0) {
> + /* The pcie-root controller with controller index 0
*really* unimportant, but I think the above should say "pci-root" rather
than "pcie-root" :-)
> + * must always be assigned target index 0, because
> + * it represents the implicit PHB which is treated
> + * differently than all other PHBs */
> + options->idx = 0;
> + } else {
> + /* For all other PHBs the target index doesn't need
> + * to match the controller index or have any
> + * particular value, really */
How is it used then? Just directly put on qemu commandline? And it's
allowed to have gaps in the index numbers of the PHBs?
> + options->idx = qemuDomainAddressFindNewIndex(def);
> + }
> + }
> + if (options->idx == -1) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("No valid index is available to "
> + "auto-assign to bus %d. Must be "
> + "manually assigned"),
I guess if you checked for unused indexes inside the limits of currently
used indexes (as I suggested above) then the part about "Must be
manually assigned" wouldn't be true - if we couldn't find an unused
index, then that's the end of it; there's no possible value to manually
assign either.
> + addr->bus);
> + goto cleanup;
> + }
> + break;
> case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE:
> case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT:
> - case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
> case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
> case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST:
> break;
> diff --git a/tests/qemuargv2xmldata/qemuargv2xml-pseries-disk.xml b/tests/qemuargv2xmldata/qemuargv2xml-pseries-disk.xml
> index 1bad8ee..601d0f7 100644
> --- a/tests/qemuargv2xmldata/qemuargv2xml-pseries-disk.xml
> +++ b/tests/qemuargv2xmldata/qemuargv2xml-pseries-disk.xml
> @@ -30,7 +30,10 @@
> <controller type='usb' index='0'>
> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
> </controller>
> - <controller type='pci' index='0' model='pci-root'/>
> + <controller type='pci' index='0' model='pci-root'>
> + <model name='spapr-pci-host-bridge'/>
> + <target index='0'/>
> + </controller>
> <controller type='scsi' index='0'>
> <address type='spapr-vio' reg='0x2000'/>
> </controller>
> diff --git a/tests/qemuargv2xmldata/qemuargv2xml-pseries-nvram.xml b/tests/qemuargv2xmldata/qemuargv2xml-pseries-nvram.xml
> index 7e9f864..7787847 100644
> --- a/tests/qemuargv2xmldata/qemuargv2xml-pseries-nvram.xml
> +++ b/tests/qemuargv2xmldata/qemuargv2xml-pseries-nvram.xml
> @@ -17,7 +17,10 @@
> <controller type='usb' index='0'>
> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
> </controller>
> - <controller type='pci' index='0' model='pci-root'/>
> + <controller type='pci' index='0' model='pci-root'>
> + <model name='spapr-pci-host-bridge'/>
> + <target index='0'/>
> + </controller>
> <memballoon model='none'/>
> <nvram>
> <address type='spapr-vio' reg='0x4000'/>
> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-panic-pseries.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-panic-pseries.xml
> index 1ed11ce..7fb49fe 100644
> --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-panic-pseries.xml
> +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-panic-pseries.xml
> @@ -17,7 +17,10 @@
> <controller type='usb' index='0'>
> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
> </controller>
> - <controller type='pci' index='0' model='pci-root'/>
> + <controller type='pci' index='0' model='pci-root'>
> + <model name='spapr-pci-host-bridge'/>
> + <target index='0'/>
> + </controller>
> <serial type='pty'>
> <target port='0'/>
> <address type='spapr-vio' reg='0x30000000'/>
> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-ppc64-usb-controller-legacy.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-ppc64-usb-controller-legacy.xml
> index b7bde24..673b81d 100644
> --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-ppc64-usb-controller-legacy.xml
> +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-ppc64-usb-controller-legacy.xml
> @@ -22,7 +22,10 @@
> <controller type='usb' index='0'>
> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
> </controller>
> - <controller type='pci' index='0' model='pci-root'/>
> + <controller type='pci' index='0' model='pci-root'>
> + <model name='spapr-pci-host-bridge'/>
> + <target index='0'/>
> + </controller>
> <memballoon model='virtio'>
> <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/>
> </memballoon>
> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-ppc64-usb-controller.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-ppc64-usb-controller.xml
> index 82aaaca..68995a9 100644
> --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-ppc64-usb-controller.xml
> +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-ppc64-usb-controller.xml
> @@ -22,7 +22,10 @@
> <controller type='usb' index='0' model='pci-ohci'>
> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
> </controller>
> - <controller type='pci' index='0' model='pci-root'/>
> + <controller type='pci' index='0' model='pci-root'>
> + <model name='spapr-pci-host-bridge'/>
> + <target index='0'/>
> + </controller>
> <memballoon model='virtio'>
> <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/>
> </memballoon>
> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-nvram.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-nvram.xml
> index 713f31c..f89b23b 100644
> --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-nvram.xml
> +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-nvram.xml
> @@ -17,7 +17,10 @@
> <controller type='usb' index='0'>
> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
> </controller>
> - <controller type='pci' index='0' model='pci-root'/>
> + <controller type='pci' index='0' model='pci-root'>
> + <model name='spapr-pci-host-bridge'/>
> + <target index='0'/>
> + </controller>
> <memballoon model='none'/>
> <nvram>
> <address type='spapr-vio' reg='0x4000'/>
> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-missing.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-missing.xml
> index 1ed11ce..7fb49fe 100644
> --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-missing.xml
> +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-missing.xml
> @@ -17,7 +17,10 @@
> <controller type='usb' index='0'>
> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
> </controller>
> - <controller type='pci' index='0' model='pci-root'/>
> + <controller type='pci' index='0' model='pci-root'>
> + <model name='spapr-pci-host-bridge'/>
> + <target index='0'/>
> + </controller>
> <serial type='pty'>
> <target port='0'/>
> <address type='spapr-vio' reg='0x30000000'/>
> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-no-address.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-no-address.xml
> index 1ed11ce..7fb49fe 100644
> --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-no-address.xml
> +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-no-address.xml
> @@ -17,7 +17,10 @@
> <controller type='usb' index='0'>
> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
> </controller>
> - <controller type='pci' index='0' model='pci-root'/>
> + <controller type='pci' index='0' model='pci-root'>
> + <model name='spapr-pci-host-bridge'/>
> + <target index='0'/>
> + </controller>
> <serial type='pty'>
> <target port='0'/>
> <address type='spapr-vio' reg='0x30000000'/>
>
More information about the libvir-list
mailing list