Re: [libvirt] [PATCH v3 5/5] qemu: auto-add bridges and allow using them

Add a "dry run" address allocation to figure out how many bridges will be needed for all the devices without explicit addresses. Auto-add just enough bridges to put all the devices on, or up to the bridge with the largest specified index.

--- src/conf/domain_conf.c | 26 ++- src/qemu/qemu_command.c | 211 +++++++++++++++++---- src/qemu/qemu_command.h | 4 +- tests/qemuxml2argvdata/qemuxml2argv-pci-bridge.xml | 210 ++++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 5 files changed, 411 insertions(+), 41 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pci-bridge.xml diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 5740009..800c0a7 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2578,6 +2578,9 @@ virDomainDefPostParseInternal(virDomainDefPtr def, virCapsPtr caps ATTRIBUTE_UNUSED) { int i; + bool b; + int ret = -1; + virBitmapPtr bitmap = NULL; /* verify init path for container based domains */ if (STREQ(def->os.type, "exe") && !def->os.init) { @@ -2653,11 +2656,30 @@ virDomainDefPostParseInternal(virDomainDefPtr def, } } - return 0; + /* Verify that PCI controller indexes are unique */ + bitmap = virBitmapNew(def->ncontrollers); + for (i = 0; i < def->ncontrollers; i++) { + if (def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) { + ignore_value(virBitmapGetBit(bitmap, def->controllers[i]->idx, &b)); + if (b) { + virReportError(VIR_ERR_XML_ERROR, + _("Multiple PCI controllers with index %d"), + def->controllers[i]->idx); + goto cleanup; + } + ignore_value(virBitmapSetBit(bitmap, def->controllers[i]->idx)); + } + } + ret = 0; + +cleanup: + virBitmapFree(bitmap); + + return ret;

no_memory: virReportOOMError(); - return -1; + goto cleanup; } diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 3787ff1..ec7d0e6 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1197,6 +1197,8 @@ struct _qemuDomainPCIAddressSet { qemuDomainPCIAddressBus *used; virDevicePCIAddress lastaddr; size_t nbuses; /* allocation of 'used' */ + bool dryRun; /* on a dry run, new buses are auto-added + and addresses aren't saved in device infos */ }; @@ -1216,9 +1218,10 @@ static bool qemuPCIAddressValidate(qemuDomainPCIAddressSetPtr addrs ATTRIBUTE_UN _("Only PCI domain 0 is available")); return false; } - if (addr->bus != 0) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("Only PCI bus 0 is available")); + if (addr->bus >= addrs->nbuses) { + virReportError(VIR_ERR_XML_ERROR, + _("Only PCI buses up to %zu are available"), + addrs->nbuses - 1); return false; } if (addr->function >= QEMU_PCI_ADDRESS_FUNCTION_LAST) { @@ -1233,9 +1236,46 @@ static bool qemuPCIAddressValidate(qemuDomainPCIAddressSetPtr addrs ATTRIBUTE_UN QEMU_PCI_ADDRESS_SLOT_LAST); return false; } + if (addr->slot == 0) { + if (addr->bus) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Slot 0 is unusable on PCI bridges")); + return false; + } else { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Slot 0 on bus 0 is reserved for the host bridge")); + return false; + }

+ } return true; } +/* Ensure addr fits in the address set, by expanding it if needed. + * Return value: + * -1 = OOM + * 0 = no action performed + * >0 = number of buses added + */ +static int +qemuDomainPCIAddressSetGrow(qemuDomainPCIAddressSetPtr addrs, + virDevicePCIAddressPtr addr) +{ + int add, i; + + add = addr->bus - addrs->nbuses + 1; + i = addrs->nbuses; + if (add <= 0) + return 0; + if (VIR_EXPAND_N(addrs->used, addrs->nbuses, add) < 0) { + virReportOOMError(); + return -1; + } + /* reserve slot 0 on the new buses */ + for (; i < addrs->nbuses; i++) + addrs->used[i][0] = 0xFF; + return add; +} + static char *qemuPCIAddressAsString(virDevicePCIAddressPtr addr) { @@ -1273,6 +1313,9 @@ static int qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, return 0; } + if (addrs->dryRun && qemuDomainPCIAddressSetGrow(addrs, addr) < 0) + return -1; + if (!qemuPCIAddressValidate(addrs, addr)) return -1; @@ -1326,15 +1369,54 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, qemuDomainObjPrivatePtr priv = NULL; if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { + int max_idx = 0; int nbuses = 0; int i; + int rv; for (i = 0; i < def->ncontrollers; i++) { - if (def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) + if (def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) { + if (def->controllers[i]->idx > max_idx) + max_idx = def->controllers[i]->idx; nbuses++; + } + } + + if (nbuses > 0 && max_idx >= nbuses) + nbuses = max_idx + 1; + + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PCI_BRIDGE)) { + virDomainDeviceInfo info; + /* 1st pass to figure out how many PCI bridges we need */ + if (!(addrs = qemuDomainPCIAddressSetCreate(def, nbuses, true))) + goto cleanup; + if (qemuAssignDevicePCISlots(def, qemuCaps, addrs) < 0) + goto cleanup; + /* Reserve 1 extra slot for a (potential) bridge */ + if (qemuDomainPCIAddressSetNextAddr(addrs, &info) < 0) + goto cleanup; + + for (i = 1; i < addrs->nbuses; i++) { + if ((rv = virDomainDefMaybeAddController( + def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, + i, VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE)) < 0) + goto cleanup; + /* If we added a new bridge, we will need one more address */ + if (rv > 0 && qemuDomainPCIAddressSetNextAddr(addrs, &info) < 0) + goto cleanup; + } + nbuses = addrs->nbuses; + qemuDomainPCIAddressSetFree(addrs); + addrs = NULL; + + } else if (nbuses > 1) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("PCI bridges are not supported " + "by this QEMU binary")); + goto cleanup; } - if (!(addrs = qemuDomainPCIAddressSetCreate(def, nbuses))) + if (!(addrs = qemuDomainPCIAddressSetCreate(def, nbuses, false))) goto cleanup; if (qemuAssignDevicePCISlots(def, qemuCaps, addrs) < 0) @@ -1380,7 +1462,8 @@ int qemuDomainAssignAddresses(virDomainDefPtr def, } qemuDomainPCIAddressSetPtr qemuDomainPCIAddressSetCreate(virDomainDefPtr def, - unsigned int nbuses) + unsigned int nbuses, + bool dryRun) { qemuDomainPCIAddressSetPtr addrs; int i; @@ -1392,6 +1475,7 @@ qemuDomainPCIAddressSetPtr qemuDomainPCIAddressSetCreate(virDomainDefPtr def, goto no_memory; addrs->nbuses = nbuses; + addrs->dryRun = dryRun; /* reserve slot 0 in every bus - it's used by the host bridge on bus 0 * and unusable on PCI bridges */ @@ -1424,6 +1508,9 @@ int qemuDomainPCIAddressReserveAddr(qemuDomainPCIAddressSetPtr addrs, { char *str; + if (addrs->dryRun && qemuDomainPCIAddressSetGrow(addrs, addr) < 0) + return -1; + if (!(str = qemuPCIAddressAsString(addr))) return -1; @@ -1450,6 +1537,9 @@ int qemuDomainPCIAddressReserveSlot(qemuDomainPCIAddressSetPtr addrs, { char *str; + if (addrs->dryRun && qemuDomainPCIAddressSetGrow(addrs, addr) < 0) + return -1; + if (!(str = qemuPCIAddressAsString(addr))) return -1; @@ -1519,41 +1609,58 @@ void qemuDomainPCIAddressSetFree(qemuDomainPCIAddressSetPtr addrs) VIR_FREE(addrs); } - static int qemuDomainPCIAddressGetNextSlot(qemuDomainPCIAddressSetPtr addrs, virDevicePCIAddressPtr next_addr) { - virDevicePCIAddress tmp_addr = addrs->lastaddr; - int i; - char *addr; + virDevicePCIAddress a = addrs->lastaddr; - tmp_addr.slot++; - for (i = 0; i < QEMU_PCI_ADDRESS_SLOT_LAST; i++, tmp_addr.slot++) { - if (QEMU_PCI_ADDRESS_SLOT_LAST <= tmp_addr.slot) { - tmp_addr.slot = 0; - } + if (addrs->nbuses == 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", _("No PCI buses available")); + return -1; + } - if (!(addr = qemuPCIAddressAsString(&tmp_addr))) - return -1; + /* Start the search at the last used bus and slot */ + for (a.slot++; a.bus < addrs->nbuses; a.bus++) { + for ( ; a.slot < QEMU_PCI_ADDRESS_SLOT_LAST; a.slot++) { + if (!qemuDomainPCIAddressSlotInUse(addrs, &a)) + goto success; - if (qemuDomainPCIAddressSlotInUse(addrs, &tmp_addr)) { - VIR_DEBUG("PCI addr %s already in use", addr); - VIR_FREE(addr); - continue; + VIR_DEBUG("PCI slot %.4x:%.2x:%.2x already in use", + a.domain, a.bus, a.slot); } + a.slot = 1; + } - VIR_DEBUG("Found free PCI addr %s", addr); - VIR_FREE(addr); + /* There were no free slots after the last used one */ + if (addrs->dryRun) { + /* a is already set to the first new bus and slot 1 */ + if (qemuDomainPCIAddressSetGrow(addrs, &a) < 0) + return -1; + goto success; + } else { + /* Check the buses from 0 up to the last used one */ + for (a.bus = 0; a.bus <= addrs->lastaddr.bus; a.bus++) { + for (a.slot = 1; a.slot < QEMU_PCI_ADDRESS_SLOT_LAST; a.slot++) { + if (!qemuDomainPCIAddressSlotInUse(addrs, &a)) + goto success; - addrs->lastaddr = tmp_addr; - *next_addr = tmp_addr; - return 0; + VIR_DEBUG("PCI slot %.4x:%.2x:%.2x already in use", + a.domain, a.bus, a.slot); + } + + } } virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("No more available PCI addresses")); return -1; + +success: + VIR_DEBUG("Found free PCI slot %.4x:%.2x:%.2x", + a.domain, a.bus, a.slot); + *next_addr = a; + return 0; } int qemuDomainPCIAddressSetNextAddr(qemuDomainPCIAddressSetPtr addrs, @@ -1566,8 +1673,10 @@ int qemuDomainPCIAddressSetNextAddr(qemuDomainPCIAddressSetPtr addrs, if (qemuDomainPCIAddressReserveSlot(addrs, &addr) < 0) return -1; - dev->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; - dev->addr.pci = addr; + if (!addrs->dryRun) { + dev->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; + dev->addr.pci = addr; + } addrs->lastaddr = addr; return 0; @@ -1741,6 +1850,18 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, } } + /* PCI controllers */ + for (i = 0; i < def->ncontrollers; i++) { + if (def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) { + if (def->controllers[i]->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) + continue; + if (def->controllers[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) + continue; + if (qemuDomainPCIAddressSetNextAddr(addrs, &def->controllers[i]->info) < 0) + goto error; + } + } + for (i = 0; i < def->nfss ; i++) { if (def->fss[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) continue; @@ -1780,9 +1901,10 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, /* Device controllers (SCSI, USB, but not IDE, FDC or CCID) */ for (i = 0; i < def->ncontrollers ; i++) { - /* PCI root has no address */ + /* PCI controllers have been dealt with earlier */ if (def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) continue; + /* FDC lives behind the ISA bridge; CCID is a usb device */ if (def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_FDC || def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_CCID) @@ -1972,16 +2094,29 @@ qemuBuildDeviceAddressStr(virBufferPtr buf, } } - /* XXX - * When QEMU grows support for > 1 PCI bus, then pci.0 changes - * to pci.1, pci.2, etc - * When QEMU grows support for > 1 PCI domain, then pci.0 change - * to pciNN.0 where NN is the domain number + /* + * PCI bridge support is required for multiple buses + * 'pci.%u' is the ID of the bridge as specified in + * qemuBuildControllerDevStr + * + * PCI_MULTIBUS capability indicates that the implicit + * PCI bus is named 'pci.0' instead of 'pci'. */ - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_PCI_MULTIBUS)) - virBufferAsprintf(buf, ",bus=pci.0"); - else - virBufferAsprintf(buf, ",bus=pci"); + if (info->addr.pci.bus != 0) { + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PCI_BRIDGE)) { + virBufferAsprintf(buf, ",bus=pci.%u", info->addr.pci.bus); + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Multiple PCI buses are not supported " + "with this QEMU binary")); + return -1; + } + } else { + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_PCI_MULTIBUS)) + virBufferAsprintf(buf, ",bus=pci.0"); + else + virBufferAsprintf(buf, ",bus=pci"); + } if (info->addr.pci.multi == VIR_DEVICE_ADDRESS_PCI_MULTI_ON) virBufferAddLit(buf, ",multifunction=on"); else if (info->addr.pci.multi == VIR_DEVICE_ADDRESS_PCI_MULTI_OFF) diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index b05510b..5d9ae72 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -197,7 +197,9 @@ int qemuDomainAssignPCIAddresses(virDomainDefPtr def, virQEMUCapsPtr qemuCaps, virDomainObjPtr obj); qemuDomainPCIAddressSetPtr qemuDomainPCIAddressSetCreate(virDomainDefPtr def, - unsigned int nbuses); + unsigned int nbuses, + bool dryRun); +>>>>>>> a3dcfa9... qemu: auto-add bridges and allow using them

int qemuDomainPCIAddressReserveSlot(qemuDomainPCIAddressSetPtr addrs, virDevicePCIAddressPtr addr); int qemuDomainPCIAddressReserveAddr(qemuDomainPCIAddressSetPtr addrs, diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pci-bridge.xml b/tests/qemuxml2argvdata/qemuxml2argv-pci-bridge.xml new file mode 100644 index 0000000..eb20328 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pci-bridge.xml

@@ -0,0 +1,210 @@ +<domain type='qemu'> + <name>fdr-br</name> + <uuid>3ec6cbe1-b5a2-4515-b800-31a61855df41</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='pc-1.2'>hvm</type> + <boot dev='hd'/> + </os> + <features> + <acpi/> + <apic/> + <pae/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <devices> + <emulator>/usr/libexec/qemu-kvm</emulator> + <disk type='file' device='cdrom'> + <driver name='qemu' type='raw'/> + <source file='/var/iso/f18kde.iso'/> + <target dev='hdc' bus='ide'/> + <readonly/> + <address type='drive' controller='0' bus='1' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='ide' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <controller type='pci' index='1' model='pci-bridge'/> + <controller type='pci' index='2' model='pci-bridge'/> + <interface type='network'> + <mac address='52:54:00:f1:95:51'/> + <source network='default'/> + <model type='rtl8139'/> + </interface> + <interface type='network'> + <mac address='52:54:00:5c:c6:1a'/> + <source network='default'/> + <model type='e1000'/> + </interface> + <interface type='network'> + <mac address='52:54:00:39:97:ac'/> + <source network='default'/> + <model type='e1000'/> + </interface> + <interface type='network'> + <mac address='52:54:00:45:28:cb'/> + <source network='default'/> + <model type='e1000'/> + </interface> + <interface type='network'> + <mac address='52:54:00:ee:b9:a8'/> + <source network='default'/> + <model type='e1000'/> + </interface> + <interface type='network'> + <mac address='52:54:00:a9:f7:17'/> + <source network='default'/> + <model type='e1000'/> + </interface> + <interface type='network'> + <mac address='52:54:00:df:2b:f3'/> + <source network='default'/> + <model type='e1000'/> + </interface> + <interface type='network'> + <mac address='52:54:00:78:94:b4'/> + <source network='default'/> + <model type='e1000'/> + </interface> + <interface type='network'> + <mac address='52:54:00:6b:9b:06'/> + <source network='default'/> + <model type='e1000'/> + </interface> + <interface type='network'> + <mac address='52:54:00:17:df:bc'/> + <source network='default'/> + <model type='e1000'/> + </interface> + <interface type='network'> + <mac address='52:54:00:3b:d0:51'/> + <source network='default'/> + <model type='e1000'/> + </interface> + <interface type='network'> + <mac address='52:54:00:8d:2d:17'/> + <source network='default'/> + <model type='e1000'/> + </interface> + <interface type='network'> + <mac address='52:54:00:a7:66:af'/> + <source network='default'/> + <model type='e1000'/> + </interface> + <interface type='network'> + <mac address='52:54:00:54:ab:d7'/> + <source network='default'/> + <model type='e1000'/> + </interface> + <interface type='network'> + <mac address='52:54:00:1f:99:90'/> + <source network='default'/> + <model type='e1000'/> + </interface> + <interface type='network'> + <mac address='52:54:00:c8:43:87'/> + <source network='default'/> + <model type='e1000'/> + </interface> + <interface type='network'> + <mac address='52:54:00:df:22:b2'/> + <source network='default'/> + <model type='e1000'/> + </interface> + <interface type='network'> + <mac address='52:54:00:d2:9a:47'/> + <source network='default'/> + <model type='e1000'/> + </interface> + <interface type='network'> + <mac address='52:54:00:86:05:e2'/> + <source network='default'/> + <model type='e1000'/> + </interface> + <interface type='network'> + <mac address='52:54:00:8c:1c:c2'/> + <source network='default'/> + <model type='e1000'/> + </interface> + <interface type='network'> + <mac address='52:54:00:48:58:92'/> + <source network='default'/> + <model type='e1000'/> + </interface> + <interface type='network'> + <mac address='52:54:00:99:e5:bf'/> + <source network='default'/> + <model type='e1000'/> + </interface> + <interface type='network'> + <mac address='52:54:00:b1:8c:25'/> + <source network='default'/> + <model type='e1000'/> + </interface> + <interface type='network'> + <mac address='52:54:00:60:e0:d0'/> + <source network='default'/> + <model type='e1000'/> + </interface> + <interface type='network'> + <mac address='52:54:00:37:00:6a'/> + <source network='default'/> + <model type='e1000'/> + </interface> + <interface type='network'> + <mac address='52:54:00:c7:c8:ad'/> + <source network='default'/> + <model type='e1000'/> + </interface> + <interface type='network'> + <mac address='52:54:00:4e:a7:cf'/> + <source network='default'/> + <model type='e1000'/> + </interface> + <interface type='network'> + <mac address='52:54:00:00:79:69'/> + <source network='default'/> + <model type='e1000'/> + </interface> + <interface type='network'> + <mac address='52:54:00:47:00:6f'/> + <source network='default'/> + <model type='e1000'/> + </interface> + <interface type='network'> + <mac address='52:54:00:2a:8c:8b'/> + <source network='default'/> + <model type='e1000'/> + </interface> + <interface type='network'> + <mac address='52:54:00:ec:d5:e3'/> + <source network='default'/> + <model type='e1000'/> + </interface> + <interface type='network'> + <mac address='52:54:00:7e:6e:c8'/> + <source network='default'/> + <model type='e1000'/> + </interface> + <input type='mouse' bus='ps2'/> + <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' keymap='en-us'> + <listen type='address' address='127.0.0.1'/> + </graphics> + <video> + <model type='cirrus' vram='9216' heads='1'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </video> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 7434190..04b14df 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -276,6 +276,7 @@ mymain(void) DO_TEST_DIFFERENT("metadata"); DO_TEST("tpm-passthrough"); + DO_TEST("pci-bridge"); virObjectUnref(driver.caps); virObjectUnref(driver.xmlopt);