On Mon, Aug 5, 2013 at 9:15 PM, Doug Goldstein <cardoe@xxxxxxxxxx> wrote:
> On Mon, Aug 5, 2013 at 8:13 PM, Laine Stump <laine@xxxxxxxxx> wrote:
>> This patch addresses two concerns with the error reporting when an
>> incompatible PCI address is specified for a device:
>>
>> 1) It wasn't always apparent which device had the problem. With this
>> patch applied, any error about an incompatible address will always
>> contain the full address as given in the config, so it will be easier
>> to determine which device's config aused the problem.
>>
>> 2) In some cases when the problem came from bad config, the error
>> message was erroneously classified as VIR_ERR_INTERNAL_ERROR. With
>> this patch applied, the same error message will be changed to indicate
>> either "internal" or "xml" error depending on whether the address came
>> from the config, or was automatically generated by libvirt.
>>
>> Note that in the case of "internal" (due to bad auto-generation)
>> errors, the PCI address won't be of much use in finding the location
>> in config to change (because it was automatically generated). Of
>> course that makes perfect sense, but still the address could provide a
>> clue about a bug in libvirt attempting to use a type of pci bus that
>> doesn't have its flags set correctly (or something similar). In other
>> words, it's not perfect, but it is definitely better.
>> ---
>> src/qemu/qemu_command.c | 224 +++++++++++++++++++++++++++++-------------------
>> 1 file changed, 138 insertions(+), 86 deletions(-)
>>
>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>> index 3d670f9..6060bb0 100644
>> --- a/src/qemu/qemu_command.c
>> +++ b/src/qemu/qemu_command.c
>> @@ -1445,10 +1445,15 @@ struct _qemuDomainPCIAddressSet {
>>
>> static bool
>> qemuDomainPCIAddressFlagsCompatible(virDevicePCIAddressPtr addr,
>> + char *addrStr,
>
> Slight nitpick. const char maybe?
>
>> qemuDomainPCIConnectFlags busFlags,
>> qemuDomainPCIConnectFlags devFlags,
>> - bool reportError)
>> + bool reportError,
>> + bool fromConfig)
>> {
>> + virErrorNumber errType = (fromConfig
>> + ? VIR_ERR_XML_ERROR : VIR_ERR_INTERNAL_ERROR);
>> +
>> /* If this bus doesn't allow the type of connection (PCI
>> * vs. PCIe) required by the device, or if the device requires
>> * hot-plug and this bus doesn't have it, return false.
>> @@ -1456,24 +1461,24 @@ qemuDomainPCIAddressFlagsCompatible(virDevicePCIAddressPtr addr,
>> if (!(devFlags & busFlags & QEMU_PCI_CONNECT_TYPES_MASK)) {
>> if (reportError) {
>> if (devFlags & QEMU_PCI_CONNECT_TYPE_PCI) {
>> - virReportError(VIR_ERR_INTERNAL_ERROR,
>> - _("PCI bus %.4x:%.2x is not compatible with the "
>> - "device. Device requires a standard PCI slot, "
>> - "which is not provided by this bus"),
>> - addr->domain, addr->bus);
>> + virReportError(errType,
>> + _("PCI bus is not compatible with the device "
>> + "at %s. Device requires a standard PCI slot, "
>> + "which is not provided by bus %.4x:%.2x"),
>> + addrStr, addr->domain, addr->bus);
>> } else if (devFlags & QEMU_PCI_CONNECT_TYPE_PCIE) {
>> - virReportError(VIR_ERR_INTERNAL_ERROR,
>> - _("PCI bus %.4x:%.2x is not compatible with the "
>> - "device. Device requires a PCI Express slot, "
>> - "which is not provided by this bus"),
>> - addr->domain, addr->bus);
>> + virReportError(errType,
>> + _("PCI bus is not compatible with the device "
>> + "at %s. Device requires a PCI Express slot, "
>> + "which is not provided by bus %.4x:%.2x"),
>> + addrStr, addr->domain, addr->bus);
>> } else {
>> /* this should never happen. If it does, there is a
>> * bug in the code that sets the flag bits for devices.
>> */
>> - virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> - _("The device information has no PCI "
>> - "connection types listed"));
>> + virReportError(errType,
>> + _("The device information for %s has no PCI "
>> + "connection types listed"), addrStr);
>> }
>> }
>> return false;
>> @@ -1481,11 +1486,11 @@ qemuDomainPCIAddressFlagsCompatible(virDevicePCIAddressPtr addr,
>> if ((devFlags & QEMU_PCI_CONNECT_HOTPLUGGABLE) &&
>> !(busFlags & QEMU_PCI_CONNECT_HOTPLUGGABLE)) {
>> if (reportError) {
>> - virReportError(VIR_ERR_INTERNAL_ERROR,
>> - _("PCI bus %.4x:%.2x is not compatible with the "
>> - "device. Device requires hot-plug capability, "
>> - "which is not provided by the bus"),
>> - addr->domain, addr->bus);
>> + virReportError(errType,
>> + _("PCI bus is not compatible with the device "
>> + "at %s. Device requires hot-plug capability, "
>> + "which is not provided by bus %.4x:%.2x"),
>> + addrStr, addr->domain, addr->bus);
>> }
>> return false;
>> }
>> @@ -1500,23 +1505,30 @@ qemuDomainPCIAddressFlagsCompatible(virDevicePCIAddressPtr addr,
>> static bool
>> qemuDomainPCIAddressValidate(qemuDomainPCIAddressSetPtr addrs,
>> virDevicePCIAddressPtr addr,
>> - qemuDomainPCIConnectFlags flags)
>> + char *addrStr,
>
> Same here? const char?
>
>> + qemuDomainPCIConnectFlags flags,
>> + bool fromConfig)
>> {
>> qemuDomainPCIAddressBusPtr bus;
>> + virErrorNumber errType = (fromConfig
>> + ? VIR_ERR_XML_ERROR : VIR_ERR_INTERNAL_ERROR);
>>
>> if (addrs->nbuses == 0) {
>> - virReportError(VIR_ERR_XML_ERROR, "%s", _("No PCI buses available"));
>> + virReportError(errType, "%s", _("No PCI buses available"));
>> return false;
>> }
>> if (addr->domain != 0) {
>> - virReportError(VIR_ERR_XML_ERROR, "%s",
>> - _("Only PCI domain 0 is available"));
>> + virReportError(errType,
>> + _("Invalid PCI address %s. "
>> + "Only PCI domain 0 is available"),
>> + addrStr);
>> return false;
>> }
>> if (addr->bus >= addrs->nbuses) {
>> - virReportError(VIR_ERR_XML_ERROR,
>> - _("Only PCI buses up to %zu are available"),
>> - addrs->nbuses - 1);
>> + virReportError(errType,
>> + _("Invalid PCI address %s. "
>> + "Only PCI buses up to %zu are available"),
>> + addrStr, addrs->nbuses - 1);
>> return false;
>> }
>>
>> @@ -1525,26 +1537,27 @@ qemuDomainPCIAddressValidate(qemuDomainPCIAddressSetPtr addrs,
>> /* assure that at least one of the requested connection types is
>> * provided by this bus
>> */
>> - if (!qemuDomainPCIAddressFlagsCompatible(addr, bus->flags, flags, true))
>> + if (!qemuDomainPCIAddressFlagsCompatible(addr, addrStr, bus->flags,
>> + flags, true, fromConfig))
>> return false;
>>
>> /* some "buses" are really just a single port */
>> if (bus->minSlot && addr->slot < bus->minSlot) {
>> - virReportError(VIR_ERR_XML_ERROR,
>> - _("Invalid PCI address: slot must be >= %zu"),
>> - bus->minSlot);
>> + virReportError(errType,
>> + _("Invalid PCI address %s. slot must be >= %zu"),
>> + addrStr, bus->minSlot);
>> return false;
>> }
>> if (addr->slot > bus->maxSlot) {
>> - virReportError(VIR_ERR_XML_ERROR,
>> - _("Invalid PCI address: slot must be <= %zu"),
>> - bus->maxSlot);
>> + virReportError(errType,
>> + _("Invalid PCI address %s. slot must be <= %zu"),
>> + addrStr, bus->maxSlot);
>> return false;
>> }
>> if (addr->function > QEMU_PCI_ADDRESS_FUNCTION_LAST) {
>> - virReportError(VIR_ERR_XML_ERROR,
>> - _("Invalid PCI address: function must be <= %u"),
>> - QEMU_PCI_ADDRESS_FUNCTION_LAST);
>> + virReportError(errType,
>> + _("Invalid PCI address %s. function must be <= %u"),
>> + addrStr, QEMU_PCI_ADDRESS_FUNCTION_LAST);
>> return false;
>> }
>> return true;
>> @@ -1953,12 +1966,12 @@ qemuDomainPCIAddressReserveAddr(qemuDomainPCIAddressSetPtr addrs,
>> bool fromConfig)
>> {
>> int ret = -1;
>> - char *str = NULL;
>> + char *addrStr = NULL;
>> qemuDomainPCIAddressBusPtr bus;
>> virErrorNumber errType = (fromConfig
>> ? VIR_ERR_XML_ERROR : VIR_ERR_INTERNAL_ERROR);
>>
>> - if (!(str = qemuDomainPCIAddressAsString(addr)))
>> + if (!(addrStr = qemuDomainPCIAddressAsString(addr)))
>> goto cleanup;
>>
>> /* Add an extra bus if necessary */
>> @@ -1967,7 +1980,7 @@ qemuDomainPCIAddressReserveAddr(qemuDomainPCIAddressSetPtr addrs,
>> /* Check that the requested bus exists, is the correct type, and we
>> * are asking for a valid slot
>> */
>> - if (!qemuDomainPCIAddressValidate(addrs, addr, flags))
>> + if (!qemuDomainPCIAddressValidate(addrs, addr, addrStr, flags, fromConfig))
>> goto cleanup;
>>
>> bus = &addrs->buses[addr->bus];
>> @@ -1977,31 +1990,32 @@ qemuDomainPCIAddressReserveAddr(qemuDomainPCIAddressSetPtr addrs,
>> virReportError(errType,
>> _("Attempted double use of PCI slot %s "
>> "(may need \"multifunction='on'\" for "
>> - "device on function 0)"), str);
>> + "device on function 0)"), addrStr);
>> goto cleanup;
>> }
>> bus->slots[addr->slot] = 0xFF; /* reserve all functions of slot */
>> - VIR_DEBUG("Reserving PCI slot %s (multifunction='off')", str);
>> + VIR_DEBUG("Reserving PCI slot %s (multifunction='off')", addrStr);
>> } else {
>> if (bus->slots[addr->slot] & (1 << addr->function)) {
>> if (addr->function == 0) {
>> virReportError(errType,
>> - _("Attempted double use of PCI Address %s"), str);
>> + _("Attempted double use of PCI Address %s"),
>> + addrStr);
>> } else {
>> virReportError(errType,
>> _("Attempted double use of PCI Address %s "
>> "(may need \"multifunction='on'\" "
>> - "for device on function 0)"), str);
>> + "for device on function 0)"), addrStr);
>> }
>> goto cleanup;
>> }
>> bus->slots[addr->slot] |= (1 << addr->function);
>> - VIR_DEBUG("Reserving PCI address %s", str);
>> + VIR_DEBUG("Reserving PCI address %s", addrStr);
>> }
>>
>> ret = 0;
>> cleanup:
>> - VIR_FREE(str);
>> + VIR_FREE(addrStr);
>> return ret;
>> }
>>
>> @@ -2017,7 +2031,8 @@ qemuDomainPCIAddressReserveSlot(qemuDomainPCIAddressSetPtr addrs,
>> int qemuDomainPCIAddressEnsureAddr(qemuDomainPCIAddressSetPtr addrs,
>> virDomainDeviceInfoPtr dev)
>> {
>> - int ret = 0;
>> + int ret = -1;
>> + char *addrStr = NULL;
>> /* Flags should be set according to the particular device,
>> * but only the caller knows the type of device. Currently this
>> * function is only used for hot-plug, though, and hot-plug is
>> @@ -2026,6 +2041,9 @@ int qemuDomainPCIAddressEnsureAddr(qemuDomainPCIAddressSetPtr addrs,
>> qemuDomainPCIConnectFlags flags = (QEMU_PCI_CONNECT_HOTPLUGGABLE |
>> QEMU_PCI_CONNECT_TYPE_PCI);
>>
>> + if (!(addrStr = qemuDomainPCIAddressAsString(&dev->addr.pci)))
>> + goto cleanup;
>> +
>> if (dev->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
>> /* We do not support hotplug multi-function PCI device now, so we should
>> * reserve the whole slot. The function of the PCI device must be 0.
>> @@ -2034,16 +2052,20 @@ int qemuDomainPCIAddressEnsureAddr(qemuDomainPCIAddressSetPtr addrs,
>> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> _("Only PCI device addresses with function=0"
>> " are supported"));
>> - return -1;
>> + goto cleanup;
>> }
>>
>> - if (!qemuDomainPCIAddressValidate(addrs, &dev->addr.pci, flags))
>> - return -1;
>> + if (!qemuDomainPCIAddressValidate(addrs, &dev->addr.pci,
>> + addrStr, flags, true))
>> + goto cleanup;
>>
>> ret = qemuDomainPCIAddressReserveSlot(addrs, &dev->addr.pci, flags);
>> } else {
>> ret = qemuDomainPCIAddressReserveNextSlot(addrs, dev, flags);
>> }
>> +
>> +cleanup:
>> + VIR_FREE(addrStr);
>> return ret;
>> }
>>
>> @@ -2063,12 +2085,20 @@ qemuDomainPCIAddressReleaseSlot(qemuDomainPCIAddressSetPtr addrs,
>> * already had it, and are giving it back.
>> */
>> qemuDomainPCIConnectFlags flags = QEMU_PCI_CONNECT_TYPES_MASK;
>> + int ret = -1;
>> + char *addrStr = NULL;
>>
>> - if (!qemuDomainPCIAddressValidate(addrs, addr, flags))
>> - return -1;
>> + if (!(addrStr = qemuDomainPCIAddressAsString(addr)))
>> + goto cleanup;
>> +
>> + if (!qemuDomainPCIAddressValidate(addrs, addr, addrStr, flags, false))
>> + goto cleanup;
>>
>> addrs->buses[addr->bus].slots[addr->slot] = 0;
>> - return 0;
>> + ret = 0;
>> +cleanup:
>> + VIR_FREE(addrStr);
>> + return ret;
>> }
>>
>> void qemuDomainPCIAddressSetFree(qemuDomainPCIAddressSetPtr addrs)
>> @@ -2090,6 +2120,7 @@ qemuDomainPCIAddressGetNextSlot(qemuDomainPCIAddressSetPtr addrs,
>> * 0000:00:00.0
>> */
>> virDevicePCIAddress a = { 0, 0, 0, 0, false };
>> + char *addrStr = NULL;
>>
>> /* except if this search is for the exact same type of device as
>> * last time, continue the search from the previous match
>> @@ -2099,13 +2130,17 @@ qemuDomainPCIAddressGetNextSlot(qemuDomainPCIAddressSetPtr addrs,
>>
>> if (addrs->nbuses == 0) {
>> virReportError(VIR_ERR_XML_ERROR, "%s", _("No PCI buses available"));
>> - return -1;
>> + goto error;
>> }
>>
>> /* Start the search at the last used bus and slot */
>> for (a.slot++; a.bus < addrs->nbuses; a.bus++) {
>> - if (!qemuDomainPCIAddressFlagsCompatible(&a, addrs->buses[a.bus].flags,
>> - flags, false)) {
>> + addrStr = NULL;
>> + if (!(addrStr = qemuDomainPCIAddressAsString(&a)))
>> + goto error;
>> + if (!qemuDomainPCIAddressFlagsCompatible(&a, addrStr,
>> + addrs->buses[a.bus].flags,
>> + flags, false, false)) {
>> VIR_DEBUG("PCI bus %.4x:%.2x is not compatible with the device",
>> a.domain, a.bus);
>> continue;
>> @@ -2124,13 +2159,17 @@ qemuDomainPCIAddressGetNextSlot(qemuDomainPCIAddressSetPtr addrs,
>> if (addrs->dryRun) {
>> /* a is already set to the first new bus and slot 1 */
>> if (qemuDomainPCIAddressSetGrow(addrs, &a, flags) < 0)
>> - return -1;
>> + goto error;
>> goto success;
>> } else if (flags == addrs->lastFlags) {
>> /* Check the buses from 0 up to the last used one */
>> for (a.bus = 0; a.bus <= addrs->lastaddr.bus; a.bus++) {
>> - if (!qemuDomainPCIAddressFlagsCompatible(&a, addrs->buses[a.bus].flags,
>> - flags, false)) {
>> + addrStr = NULL;
>> + if (!(addrStr = qemuDomainPCIAddressAsString(&a)))
>> + goto error;
>> + if (!qemuDomainPCIAddressFlagsCompatible(&a, addrStr,
>> + addrs->buses[a.bus].flags,
>> + flags, false, false)) {
>> VIR_DEBUG("PCI bus %.4x:%.2x is not compatible with the device",
>> a.domain, a.bus);
>> continue;
>> @@ -2147,12 +2186,15 @@ qemuDomainPCIAddressGetNextSlot(qemuDomainPCIAddressSetPtr addrs,
>>
>> virReportError(VIR_ERR_INTERNAL_ERROR,
>> "%s", _("No more available PCI slots"));
>> +error:
>> + VIR_FREE(addrStr);
>> return -1;
>>
>> success:
>> VIR_DEBUG("Found free PCI slot %.4x:%.2x:%.2x",
>> a.domain, a.bus, a.slot);
>> *next_addr = a;
>> + VIR_FREE(addrStr);
>> return 0;
>> }
>>
>> @@ -2217,10 +2259,12 @@ qemuValidateDevicePCISlotsPIIX3(virDomainDefPtr def,
>> virQEMUCapsPtr qemuCaps,
>> qemuDomainPCIAddressSetPtr addrs)
>> {
>> + int ret = -1;
>> size_t i;
>> virDevicePCIAddress tmp_addr;
>> bool qemuDeviceVideoUsable = virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIDEO_PRIMARY);
>> virDevicePCIAddressPtr addrptr;
>> + char *addrStr = NULL;
>> qemuDomainPCIConnectFlags flags = QEMU_PCI_CONNECT_HOTPLUGGABLE | QEMU_PCI_CONNECT_TYPE_PCI;
>>
>> /* Verify that first IDE and USB controllers (if any) is on the PIIX3, fn 1 */
>> @@ -2235,7 +2279,7 @@ qemuValidateDevicePCISlotsPIIX3(virDomainDefPtr def,
>> def->controllers[i]->info.addr.pci.function != 1) {
>> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> _("Primary IDE controller must have PCI address 0:0:1.1"));
>> - goto error;
>> + goto cleanup;
>> }
>> } else {
>> def->controllers[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
>> @@ -2255,7 +2299,7 @@ qemuValidateDevicePCISlotsPIIX3(virDomainDefPtr def,
>> def->controllers[i]->info.addr.pci.function != 2) {
>> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> _("PIIX3 USB controller must have PCI address 0:0:1.2"));
>> - goto error;
>> + goto cleanup;
>> }
>> } else {
>> def->controllers[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
>> @@ -2274,7 +2318,7 @@ qemuValidateDevicePCISlotsPIIX3(virDomainDefPtr def,
>> memset(&tmp_addr, 0, sizeof(tmp_addr));
>> tmp_addr.slot = 1;
>> if (qemuDomainPCIAddressReserveSlot(addrs, &tmp_addr, flags) < 0)
>> - goto error;
>> + goto cleanup;
>> }
>>
>> if (def->nvideos > 0) {
>> @@ -2293,8 +2337,11 @@ qemuValidateDevicePCISlotsPIIX3(virDomainDefPtr def,
>> primaryVideo->info.addr.pci.function = 0;
>> addrptr = &primaryVideo->info.addr.pci;
>>
>> - if (!qemuDomainPCIAddressValidate(addrs, addrptr, flags))
>> - goto error;
>> + if (!(addrStr = qemuDomainPCIAddressAsString(addrptr)))
>> + goto cleanup;
>> + if (!qemuDomainPCIAddressValidate(addrs, addrptr,
>> + addrStr, flags, false))
>> + goto cleanup;
>>
>> if (qemuDomainPCIAddressSlotInUse(addrs, addrptr)) {
>> if (qemuDeviceVideoUsable) {
>> @@ -2302,15 +2349,15 @@ qemuValidateDevicePCISlotsPIIX3(virDomainDefPtr def,
>> if (qemuDomainPCIAddressReserveNextSlot(addrs,
>> &primaryVideo->info,
>> flags) < 0)
>> - goto error;
>> + goto cleanup;
>> } else {
>> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> _("PCI address 0:0:2.0 is in use, "
>> "QEMU needs it for primary video"));
>> - goto error;
>> + goto cleanup;
>> }
>> } else if (qemuDomainPCIAddressReserveSlot(addrs, addrptr, flags) < 0) {
>> - goto error;
>> + goto cleanup;
>> }
>> } else if (!qemuDeviceVideoUsable) {
>> if (primaryVideo->info.addr.pci.domain != 0 ||
>> @@ -2319,7 +2366,7 @@ qemuValidateDevicePCISlotsPIIX3(virDomainDefPtr def,
>> primaryVideo->info.addr.pci.function != 0) {
>> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> _("Primary video card must have PCI address 0:0:2.0"));
>> - goto error;
>> + goto cleanup;
>> }
>> /* If TYPE==PCI, then qemuCollectPCIAddress() function
>> * has already reserved the address, so we must skip */
>> @@ -2334,13 +2381,13 @@ qemuValidateDevicePCISlotsPIIX3(virDomainDefPtr def,
>> " intervention");
>> virResetLastError();
>> } else if (qemuDomainPCIAddressReserveSlot(addrs, &tmp_addr, flags) < 0) {
>> - goto error;
>> + goto cleanup;
>> }
>> }
>> - return 0;
>> -
>> -error:
>> - return -1;
>> + ret = 0;
>> +cleanup:
>> + VIR_FREE(addrStr);
>> + return ret;
>> }
>>
>>
>> @@ -2357,10 +2404,12 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def,
>> virQEMUCapsPtr qemuCaps,
>> qemuDomainPCIAddressSetPtr addrs)
>> {
>> + int ret = -1;
>> size_t i;
>> virDevicePCIAddress tmp_addr;
>> bool qemuDeviceVideoUsable = virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIDEO_PRIMARY);
>> virDevicePCIAddressPtr addrptr;
>> + char *addrStr = NULL;
>> qemuDomainPCIConnectFlags flags = QEMU_PCI_CONNECT_TYPE_PCIE;
>>
>> /* Verify that the first SATA controller is at 00:1F.2 */
>> @@ -2375,7 +2424,7 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def,
>> def->controllers[i]->info.addr.pci.function != 2) {
>> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> _("Primary SATA controller must have PCI address 0:0:1f.2"));
>> - goto error;
>> + goto cleanup;
>> }
>> } else {
>> def->controllers[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
>> @@ -2399,12 +2448,12 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def,
>> tmp_addr.multi = 1;
>> if (qemuDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags,
>> false, false) < 0)
>> - goto error;
>> + goto cleanup;
>> tmp_addr.function = 3;
>> tmp_addr.multi = 0;
>> if (qemuDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags,
>> false, false) < 0)
>> - goto error;
>> + goto cleanup;
>> }
>>
>> if (def->nvideos > 0) {
>> @@ -2423,8 +2472,11 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def,
>> primaryVideo->info.addr.pci.function = 0;
>> addrptr = &primaryVideo->info.addr.pci;
>>
>> - if (!qemuDomainPCIAddressValidate(addrs, addrptr, flags))
>> - goto error;
>> + if (!(addrStr = qemuDomainPCIAddressAsString(addrptr)))
>> + goto cleanup;
>> + if (!qemuDomainPCIAddressValidate(addrs, addrptr,
>> + addrStr, flags, false))
>> + goto cleanup;
>>
>> if (qemuDomainPCIAddressSlotInUse(addrs, addrptr)) {
>> if (qemuDeviceVideoUsable) {
>> @@ -2432,15 +2484,15 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def,
>> if (qemuDomainPCIAddressReserveNextSlot(addrs,
>> &primaryVideo->info,
>> flags) < 0)
>> - goto error;
>> + goto cleanup;
>> } else {
>> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> _("PCI address 0:0:1.0 is in use, "
>> "QEMU needs it for primary video"));
>> - goto error;
>> + goto cleanup;
>> }
>> } else if (qemuDomainPCIAddressReserveSlot(addrs, addrptr, flags) < 0) {
>> - goto error;
>> + goto cleanup;
>> }
>> } else if (!qemuDeviceVideoUsable) {
>> if (primaryVideo->info.addr.pci.domain != 0 ||
>> @@ -2449,7 +2501,7 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def,
>> primaryVideo->info.addr.pci.function != 0) {
>> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> _("Primary video card must have PCI address 0:0:1.0"));
>> - goto error;
>> + goto cleanup;
>> }
>> /* If TYPE==PCI, then qemuCollectPCIAddress() function
>> * has already reserved the address, so we must skip */
>> @@ -2464,13 +2516,13 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def,
>> " intervention");
>> virResetLastError();
>> } else if (qemuDomainPCIAddressReserveSlot(addrs, &tmp_addr, flags) < 0) {
>> - goto error;
>> + goto cleanup;
>> }
>> }
>> - return 0;
>> -
>> -error:
>> - return -1;
>> + ret = 0;
>> +cleanup:
>> + VIR_FREE(addrStr);
>> + return ret;
>> }
>>
>>
>> --
>> 1.7.11.7
>>
>
> Other than my slight nitpick, visually this looks good. I'll give it a
> whirl with some of my bad configs tonight.
>
> --
> Doug Goldstein
Works as expected.
error: Failed to define domain from error.xml
error: XML error: PCI bus is not compatible with the device at
0000:00:04.0. Device requires a standard PCI slot, which is not
provided by bus 0000:00
--
Doug Goldstein
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list