Re: [libvirt] [PATCH 2/2] qemu: Order PCI controllers on the command line

Unlike other controllers, PCI controller can plug into each other,

which might turn into a problem if they are not listed in the expected order on the QEMU command line, because the guest will then not be able to start with an error such as qemu-kvm: -device pci-bridge,chassis_nr=3,id=pci.3,bus=pci.2.0,addr=0x7: Bus 'pci.2.0' not found Add some logic to make sure PCI controller appear in the correct

order on the QEMU command line, regardless of the order they were added to the guest configuration.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1479674 Signed-off-by: Andrea Bolognani <abologna(a)redhat.com&gt; --- src/qemu/qemu_command.c | 72 ++++++++++++++++------ .../qemuxml2argv-pci-autoadd-idx.args | 2 +- .../qemuxml2argv-pseries-many-buses-2.args | 4 +- 3 files changed, 56 insertions(+), 22 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 83430b33f..c0e60a184 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3096,6 +3096,8 @@ qemuBuildControllerDevCommandLine(virCommandPtr cmd, virQEMUCapsPtr qemuCaps) { size_t i, j; + size_t *pciContIndex; + size_t npciContIndex; int usbcontroller = 0; bool usblegacy = false; int contOrder[] = { @@ -3107,14 +3109,10 @@ qemuBuildControllerDevCommandLine(virCommandPtr cmd, * machines. For newer Q35 machines it is added out of the * controllers loop, after the floppy drives. * - * We don't add PCI/PCIe root controller either, because it's - * implicit, but we do add PCI bridges and other PCI - * controllers, so we leave that in to check each - * one. Likewise, we don't do anything for the primary IDE + * Likewise, we don't do anything for the primary IDE * controller on an i440fx machine or primary SATA on q35, but * we do add those beyond these two exceptions. */ - VIR_DOMAIN_CONTROLLER_TYPE_PCI, VIR_DOMAIN_CONTROLLER_TYPE_USB, VIR_DOMAIN_CONTROLLER_TYPE_SCSI, VIR_DOMAIN_CONTROLLER_TYPE_IDE, @@ -3124,6 +3122,54 @@ qemuBuildControllerDevCommandLine(virCommandPtr cmd, }; int ret = -1; + /* PCI controllers require special handling because they plug into + * each other, which makes getting the ordering right on the QEMU + * command line crucial: if we don't, and eg. list a pci-bridge + * before the pci-root it plugs into, the guest will not start */ + + npciContIndex = 0; + if (VIR_ALLOC_N(pciContIndex, def->ncontrollers) < 0) + goto cleanup; + + for (i = 0; i < def->ncontrollers; i++) { + virDomainControllerDefPtr cont = def->controllers[i]; + + if (cont->type != VIR_DOMAIN_CONTROLLER_TYPE_PCI) + continue; + + pciContIndex[cont->idx] = i; + npciContIndex = MAX(npciContIndex, cont->idx + 1); + } + + for (i = 0; i < npciContIndex; i++) { + virDomainControllerDefPtr cont = def->controllers[pciContIndex[i]]; + char *devstr; + + if (cont->type != VIR_DOMAIN_CONTROLLER_TYPE_PCI) + continue; + + /* skip pcie-root */ + if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) + continue; + + /* Skip pci-root, except for pSeries guests (which actually + * support more than one PCI Host Bridge per guest) */ + if (!qemuDomainIsPSeries(def) && + cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) { + continue; + } + + if (qemuBuildControllerDevStr(def, cont, qemuCaps, + &devstr, &usbcontroller) < 0) + goto cleanup; + + if (devstr) { + virCommandAddArg(cmd, "-device"); + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); + } + } + for (j = 0; j < ARRAY_CARDINALITY(contOrder); j++) { for (i = 0; i < def->ncontrollers; i++) { virDomainControllerDefPtr cont = def->controllers[i]; @@ -3139,20 +3185,6 @@ qemuBuildControllerDevCommandLine(virCommandPtr cmd, continue; } - /* skip pcie-root */ - if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI && - cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) { - continue; - } - - /* Skip pci-root, except for pSeries guests (which actually - * support more than one PCI Host Bridge per guest) */ - if (!qemuDomainIsPSeries(def) && - cont->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI && - cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) { - continue; - } - /* first SATA controller on Q35 machines is implicit */ if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_SATA && cont->idx == 0 && qemuDomainIsQ35(def)) @@ -3215,6 +3247,8 @@ qemuBuildControllerDevCommandLine(virCommandPtr cmd, ret = 0; cleanup: + VIR_FREE(pciContIndex); + return ret; } diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pci-autoadd-idx.args b/tests/qemuxml2argvdata/qemuxml2argv-pci-autoadd-idx.args index 6b2f21bba..c03f72d96 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-pci-autoadd-idx.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-pci-autoadd-idx.args @@ -17,7 +17,6 @@ QEMU_AUDIO_DRV=none \ server,nowait \ -mon chardev=charmonitor,id=monitor,mode=readline \ -boot c \ --device pci-bridge,chassis_nr=8,id=pci.8,bus=pci.0,addr=0x3 \ -device pci-bridge,chassis_nr=1,id=pci.1,bus=pci.0,addr=0x4 \ -device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.0,addr=0x5 \ -device pci-bridge,chassis_nr=3,id=pci.3,bus=pci.0,addr=0x6 \ @@ -25,6 +24,7 @@ server,nowait \ -device pci-bridge,chassis_nr=5,id=pci.5,bus=pci.0,addr=0x8 \ -device pci-bridge,chassis_nr=6,id=pci.6,bus=pci.0,addr=0x9 \ -device pci-bridge,chassis_nr=7,id=pci.7,bus=pci.0,addr=0xa \ +-device pci-bridge,chassis_nr=8,id=pci.8,bus=pci.0,addr=0x3 \

-usb \ -drive file=/var/iso/f18kde.iso,format=raw,if=none,media=cdrom,\ id=drive-ide0-1-0,readonly=on \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-many-buses-2.args b/tests/qemuxml2argvdata/qemuxml2argv-pseries-many-buses-2.args index 13fed02f8..7b2171f59 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-pseries-many-buses-2.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-many-buses-2.args @@ -18,5 +18,5 @@ QEMU_AUDIO_DRV=none \ server,nowait \ -mon chardev=charmonitor,id=monitor,mode=readline \ -boot c \ --device spapr-pci-host-bridge,index=1,id=pci.2 \ --device spapr-pci-host-bridge,index=2,id=pci.1 +-device spapr-pci-host-bridge,index=2,id=pci.1 \ +-device spapr-pci-host-bridge,index=1,id=pci.2