Re: [PATCH 1/3] conf: improve error log when PCI devices don't match requested controller

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]