On 08/04/2013 05:01 PM, Doug Goldstein wrote:
> On Sat, Aug 3, 2013 at 9:01 PM, Laine Stump <laine@xxxxxxxxx> wrote:
>> + if ((cont->type == VIR_DOMAIN_CONTROLLER_TYPE_IDE && cont->idx == 0 &&
>> + addr->function == 1) ||
>> + (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_USB && cont->idx == 0 &&
>> + (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI ||
>> + cont->model == -1) && addr->function == 2)) {
>> + /* Note the check for nbuses > 0 - if there are no PCI
>> + * buses, we skip this check. This is a quirk required for
>> + * some machinetypes such as s390, which pretend to have a
>> + * PCI bus for long enough to generate the "-usb" on the
>> + * commandline, but that don't really care if a PCI bus
>> + * actually exists. */
>> + if (addrs->nbuses > 0 &&
>> + !(addrs->buses[0].flags & QEMU_PCI_CONNECT_TYPE_PCI)) {
>> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> + _("Bus 0 must be PCI for integrated PIIX3 "
>> + "USB or IDE controllers"));
>> + return -1;
>> + } else {
>> + return 0;
>> + }
>> + }
> Still very hacky but at least you improved it by providing a code
> comment as to why and a sensible error message if the user gets in
> this path. I'd still love to see us improve the situation so we didn't
> have to play games like this to get -usb to be emitted for different
> machine types.
Yes. Totally agree.
>> }
>>
>> entireSlot = (addr->function == 0 &&
>> @@ -1695,8 +1728,7 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def,
>> int nbuses = 0;
>> size_t i;
>> int rv;
>> - qemuDomainPCIConnectFlags flags = (QEMU_PCI_CONNECT_HOTPLUGGABLE |
>> - QEMU_PCI_CONNECT_TYPE_PCI);
>> + qemuDomainPCIConnectFlags flags = QEMU_PCI_CONNECT_TYPE_PCI;
> So just for my edification, we previously were saying that pci-bridge
> devices were hotpluggable when in fact they are not? You could
> probably add a code comment above the default flags to that effect,
> but not strictly necessary.
You can hot-plug a device into a pci-bridge, but a pci-bridge itself
cannot be hot-plugged into the system. (Maybe a better way to describe
it is that pci-bridge slots can accept hot-plugs, but the pci-bridge
device cannot be hot-plugged. No, that's not any better. Let's see...)
>
>> for (i = 0; i < def->ncontrollers; i++) {
>> if (def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) {
>> @@ -1941,7 +1973,11 @@ int qemuDomainPCIAddressEnsureAddr(qemuDomainPCIAddressSetPtr addrs,
>> virDomainDeviceInfoPtr dev)
>> {
>> int ret = 0;
>> - /* FIXME: flags should be set according to the particular device */
>> + /* 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
>> + * only supported for standard PCI devices, so we can safely use
>> + * the setting below */
>> qemuDomainPCIConnectFlags flags = (QEMU_PCI_CONNECT_HOTPLUGGABLE |
>> QEMU_PCI_CONNECT_TYPE_PCI);
>>
>> @@ -2005,7 +2041,13 @@ qemuDomainPCIAddressGetNextSlot(qemuDomainPCIAddressSetPtr addrs,
>> virDevicePCIAddressPtr next_addr,
>> qemuDomainPCIConnectFlags flags)
>> {
>> - virDevicePCIAddress a = addrs->lastaddr;
>> + virDevicePCIAddress a = { 0, 0, 0, 0, false };
> Again, me being nitpicky but maybe a comment that says start the
> search from the top of the PCI addresses by settings this to all 0s
> and then below the comment says that this is a fast out when the flags
> match.
Is this better?
/* default to starting the search for a free slot from
* 0000:00:00.0
*/
virDevicePCIAddress a = { 0, 0, 0, 0, false };
/* except if this search is for the exact same type of device as
* last time, continue the search from the previous match
*/
if (flags == addrs->lastFlags)
a = addrs->lastaddr;
I squashed that in.
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list