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;
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)
+{
+ 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;
+ }
+ }
+
+ /* 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
+ * 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 */
+ 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"),
+ 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'/>
--
2.7.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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'/>
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, 2017-06-13 at 21:52 -0400, Laine Stump wrote:
> > @@ -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?
Yup.
> > 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)
Sure.
> > +{
> > + 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.
Zero is going to be used for the default PHB, so I'd have to
start from one instead. But I like the idea :)
> > @@ -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" :-)
It's not unimportant! Comments that disagree with the code
can cause a lot of confusion. Good catch :)
> > + * 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?
Yes to both. I can't think of a single sensible reason why
you'd want to have gaps, but it's a valid configuration.
> > + 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.
Indeed.
I just realized I'm not making sure user-assigned target
indexes are contained in the allowed range either, so I'll
add that as well.
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2025 Red Hat, Inc.