[libvirt] [PATCHv3 2/2] qemu: improve error reporting during PCI address validation
Doug Goldstein
cardoe at gentoo.org
Tue Aug 6 02:15:09 UTC 2013
On Mon, Aug 5, 2013 at 8:13 PM, Laine Stump <laine at laine.org> 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
More information about the libvir-list
mailing list