Re: [libvirt] [PATCH 3/7] qemu: eliminate almost-duplicate code in qemu_command.c

On Fri, Aug 2, 2013 at 11:55 AM, Laine Stump <laine(a)laine.org&gt; wrote: > - * Check that the PCI address is valid for use > - * with the specified PCI address set. > +static bool > +qemuDomainPCIAddressFlagsCompatible(virDevicePCIAddressPtr addr, > + qemuDomainPCIConnectFlags busFlags, > + qemuDomainPCIConnectFlags devFlags, > + bool reportError) > +{ > + /* 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. > + */ > + 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); So even though this patch has been ACK'd and committed. I really would have liked to see some info about the device printed in the error message because when you're defining a domain its really not clear which device is the problem.

> + } 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")); > + } > + } > + return false; > + } > + 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); Same as above, when you're defining the whole domain its really not clear which device is the problem so provide some details on which one it is.