On Fri, 2016-08-05 at 23:01 -0400, Laine Stump wrote: > The virDomainPCIAddressFlagsCompatible() error logs report that a > device required a controller that accepted standard PCI endpoint > devices, or PCI Express endpoint devices, and if hotplug was required > by the configuration but not provided by the selected controller. But > the wording of the error messages was apparently confusing (according > to the bugzilla report referenced below). On top of that, if the > device was something other than an endpoint device (e.g. a > pcie-switch-downstream-port) the error message was a complete punt - > it would just say that the flags were incorrect. > > This patch makes the messages for PCI/PCIe endpoint and hotplug > requirements more clear, and also specifically indicates what was the > device type when it is other than an endpoint device. > > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1363627 > --- > src/conf/domain_addr.c | 58 ++++++++++++++++++++++++++++---------------------- > 1 file changed, 33 insertions(+), 25 deletions(-) > > diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c > index a0c2f88..98176e2 100644 > --- a/src/conf/domain_addr.c > +++ b/src/conf/domain_addr.c > @@ -118,38 +118,46 @@ virDomainPCIAddressFlagsCompatible(virPCIDeviceAddressPtr addr, > * hot-plug and this bus doesn't have it, return false. > */ > if (!(devFlags & busFlags & VIR_PCI_CONNECT_TYPES_MASK)) { > - if (reportError) { > - if (devFlags & VIR_PCI_CONNECT_TYPE_PCI_DEVICE) { > - 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 & VIR_PCI_CONNECT_TYPE_PCIE_DEVICE) { > - 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(errType, > - _("The device information for %s has no PCI " > - "connection types listed"), addrStr); > - } > + const char *connectStr; > + > + if (!reportError) > + return false; > + > + if (devFlags & VIR_PCI_CONNECT_TYPE_PCI_DEVICE) { > + connectStr = "standard PCI device"; > + } else if (devFlags & VIR_PCI_CONNECT_TYPE_PCIE_DEVICE) { > + connectStr = "PCI Express device"; > + } else if (devFlags & VIR_PCI_CONNECT_TYPE_PCIE_ROOT_PORT) { > + connectStr = "pcie-root-port"; > + } else if (devFlags & VIR_PCI_CONNECT_TYPE_PCIE_SWITCH_UPSTREAM_PORT) { > + connectStr = "pci-switch-upstream-port"; > + } else if (devFlags & VIR_PCI_CONNECT_TYPE_PCIE_SWITCH_DOWNSTREAM_PORT) { > + connectStr = "pci-switch-downstream-port"; > + } else { > + /* this should never happen. If it does, there is a > + * bug in the code that sets the flag bits for devices. > + */ > + virReportError(errType, > + _("The device at PCI address %s has " > + "unrecognized connection type flags 0x%.2x"), > + addrStr, devFlags & VIR_PCI_CONNECT_TYPES_MASK); > + return false; > } > + virReportError(errType, > + _("The device at PCI address %s cannot be " > + "plugged into the PCI controller with index 0x%.2x. " > + "It requires a controller that accepts a %s."), > + addrStr, addr->bus, connectStr); > return false; > } > if ((devFlags & VIR_PCI_CONNECT_HOTPLUGGABLE) && > !(busFlags & VIR_PCI_CONNECT_HOTPLUGGABLE)) { > if (reportError) { > 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); > + _("The device at PCI address %s requires " > + "hot-plug capability, but the PCI controller " > + "at index %.2x doesn't support hot-plug"), > + addrStr, addr->bus); s/hot-plug/hotplug/g ? > } > return false; > } ACK -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list