conf/qemu: change the way VIR_PCI_CONNECT_TYPE_* flags work

The flags used to determine which devices could be plugged into which
controllers were quite confusing, as they tried to create classes of
connections, then put particular devices into possibly multiple
classes, while sometimes setting multiple flags for the controllers
themselves. The attempt to have a single flag indicate, e.g. that a
root-port or a switch-downstream-port could connect was not only
confusing, it was leading to a situation where it would be impossible
to specify exactly the right combinations for a new controller.

The solution is for the VIR_PCI_CONNECT_TYPE_* flags to have a 1:1
correspondence with each type of PCI controller, plus a flag for a PCI
endpoint device and another for a PCIe endpoint device (the only
exception to this is that pci-bridge and pcie-expander-bus controllers
have their upstream connection classified as
VIR_PCI_CONNECT_TYPE_PCI_DEVICE since they can be plugged into
*exactly* the same ports as any endpoint device).  Each device then
has a single flag for connect type (plus the HOTPLUG flag if that
device can e hotplugged), and each controller sets the CONNECT bits
for all controllers that can be plugged into it, as well as for either
type of endpoint device that can be plugged in (and the HOTPLUG flag
if it can accept hotplugged devices).

With this change, it is *slightly* easier to understand the matching
of connections (as long as you remember that the flag for a
device/upstream-facing connection of a controller is the same as that
device's type, while the flags for a controller's downstream
connections is the OR of all device types that can be plugged into
that controller). More importantly, it will be possible to correctly
specify what can be plugged into a pcie-switch-expander-bus, when
support for it is added.

src/bhyve/bhyve_device.c

@@ -53,7 +53,7 @@ bhyveCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED,
         }
     }
 
-    if (virDomainPCIAddressReserveSlot(addrs, addr, VIR_PCI_CONNECT_TYPE_PCI) < 0)
+    if (virDomainPCIAddressReserveSlot(addrs, addr, VIR_PCI_CONNECT_TYPE_PCI_DEVICE) < 0)
         goto cleanup;
 
     ret = 0;
@@ -94,7 +94,7 @@ bhyveAssignDevicePCISlots(virDomainDefPtr def,
     memset(&lpc_addr, 0, sizeof(lpc_addr));
     lpc_addr.slot = 0x1;
 
-    if (virDomainPCIAddressReserveSlot(addrs, &lpc_addr, VIR_PCI_CONNECT_TYPE_PCI) < 0)
+    if (virDomainPCIAddressReserveSlot(addrs, &lpc_addr, VIR_PCI_CONNECT_TYPE_PCI_DEVICE) < 0)
         goto error;
 
     for (i = 0; i < def->nnets; i++) {
@@ -102,7 +102,7 @@ bhyveAssignDevicePCISlots(virDomainDefPtr def,
             continue;
         if (virDomainPCIAddressReserveNextSlot(addrs,
                                                &def->nets[i]->info,
-                                               VIR_PCI_CONNECT_TYPE_PCI) < 0)
+                                               VIR_PCI_CONNECT_TYPE_PCI_DEVICE) < 0)
             goto error;
     }
 
@@ -112,7 +112,7 @@ bhyveAssignDevicePCISlots(virDomainDefPtr def,
             continue;
         if (virDomainPCIAddressReserveNextSlot(addrs,
                                                &def->disks[i]->info,
-                                               VIR_PCI_CONNECT_TYPE_PCI) < 0)
+                                               VIR_PCI_CONNECT_TYPE_PCI_DEVICE) < 0)
             goto error;
     }
 
@@ -125,7 +125,7 @@ bhyveAssignDevicePCISlots(virDomainDefPtr def,
 
             if (virDomainPCIAddressReserveNextSlot(addrs,
                                                    &def->controllers[i]->info,
-                                                   VIR_PCI_CONNECT_TYPE_PCI) < 0)
+                                                   VIR_PCI_CONNECT_TYPE_PCI_DEVICE) < 0)
                 goto error;
         }
 

src/conf/domain_addr.c

 /*
  * domain_addr.c: helper APIs for managing domain device addresses
  *
- * Copyright (C) 2006-2015 Red Hat, Inc.
+ * Copyright (C) 2006-2016 Red Hat, Inc.
  * Copyright (C) 2006 Daniel P. Berrange
  *
  * This library is free software; you can redistribute it and/or
@@ -64,13 +64,13 @@ virDomainPCIAddressFlagsCompatible(virDevicePCIAddressPtr addr,
      */
     if (!(devFlags & busFlags & VIR_PCI_CONNECT_TYPES_MASK)) {
         if (reportError) {
-            if (devFlags & VIR_PCI_CONNECT_TYPE_PCI) {
+            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) {
+            } 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, "
@@ -172,45 +172,51 @@ int
 virDomainPCIAddressBusSetModel(virDomainPCIAddressBusPtr bus,
                                virDomainControllerModelPCI model)
 {
+    /* set flags for what can be connected *downstream* from each
+     * bus.
+     */
     switch (model) {
     case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE:
     case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
         bus->flags = (VIR_PCI_CONNECT_HOTPLUGGABLE |
-                      VIR_PCI_CONNECT_TYPE_PCI);
+                      VIR_PCI_CONNECT_TYPE_PCI_DEVICE);
         bus->minSlot = 1;
         bus->maxSlot = VIR_PCI_ADDRESS_SLOT_LAST;
         break;
     case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
-        /* slots 1 - 31, no hotplug, PCIe only unless the address was
-         * specified in user config *and* the particular device being
-         * attached also allows it.
+        /* slots 1 - 31, no hotplug, PCIe endpoint device or
+         * pcie-root-port only, unless the address was specified in
+         * user config *and* the particular device being attached also
+         * allows it.
          */
-        bus->flags = VIR_PCI_CONNECT_TYPE_PCIE | VIR_PCI_CONNECT_TYPE_PCIE_ROOT;
+        bus->flags = (VIR_PCI_CONNECT_TYPE_PCIE_DEVICE
+                      | VIR_PCI_CONNECT_TYPE_PCIE_ROOT_PORT);
         bus->minSlot = 1;
         bus->maxSlot = VIR_PCI_ADDRESS_SLOT_LAST;
         break;
     case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE:
         /* slots 0 - 31, standard PCI slots,
          * but *not* hot-pluggable */
-        bus->flags = VIR_PCI_CONNECT_TYPE_PCI;
+        bus->flags = VIR_PCI_CONNECT_TYPE_PCI_DEVICE;
         bus->minSlot = 0;
         bus->maxSlot = VIR_PCI_ADDRESS_SLOT_LAST;
         break;
     case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT:
     case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT:
-        /* provides one slot which is pcie, can be used by devices
-         * that must connect to some type of "pcie-*-port", and
-         * is hotpluggable
+        /* provides one slot which is pcie, can be used by endpoint
+         * devices and pcie-switch-upstream-ports, and is hotpluggable
          */
-        bus->flags = VIR_PCI_CONNECT_TYPE_PCIE
-           | VIR_PCI_CONNECT_TYPE_PCIE_PORT
+        bus->flags = VIR_PCI_CONNECT_TYPE_PCIE_DEVICE
+           | VIR_PCI_CONNECT_TYPE_PCIE_SWITCH_UPSTREAM_PORT
            | VIR_PCI_CONNECT_HOTPLUGGABLE;
         bus->minSlot = 0;
         bus->maxSlot = 0;
         break;
     case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT:
-        /* 31 slots, can only accept pcie-switch-port, no hotplug */
-        bus->flags = VIR_PCI_CONNECT_TYPE_PCIE_SWITCH;
+        /* 32 slots, can only accept pcie-switch-downstrean-ports,
+         * no hotplug
+         */
+        bus->flags = VIR_PCI_CONNECT_TYPE_PCIE_SWITCH_DOWNSTREAM_PORT;
         bus->minSlot = 0;
         bus->maxSlot = VIR_PCI_ADDRESS_SLOT_LAST;
         break;
@@ -249,7 +255,7 @@ virDomainPCIAddressSetGrow(virDomainPCIAddressSetPtr addrs,
         return 0;
 
     /* auto-grow only works when we're adding plain PCI devices */
-    if (!(flags & VIR_PCI_CONNECT_TYPE_PCI)) {
+    if (!(flags & VIR_PCI_CONNECT_TYPE_PCI_DEVICE)) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                        _("Cannot automatically add a new PCI bus for a "
                          "device requiring a slot other than standard PCI."));
@@ -388,7 +394,7 @@ virDomainPCIAddressEnsureAddr(virDomainPCIAddressSetPtr addrs,
      * only supported for standard PCI devices, so we can safely use
      * the setting below */
     virDomainPCIConnectFlags flags = (VIR_PCI_CONNECT_HOTPLUGGABLE |
-                                      VIR_PCI_CONNECT_TYPE_PCI);
+                                      VIR_PCI_CONNECT_TYPE_PCI_DEVICE);
 
     if (!(addrStr = virDomainPCIAddressAsString(&dev->addr.pci)))
         goto cleanup;

src/conf/domain_addr.h

 /*
  * domain_addr.h: helper APIs for managing domain device addresses
  *
- * Copyright (C) 2006-2015 Red Hat, Inc.
+ * Copyright (C) 2006-2016 Red Hat, Inc.
  * Copyright (C) 2006 Daniel P. Berrange
  *
  * This library is free software; you can redistribute it and/or
@@ -30,32 +30,42 @@
 # define VIR_PCI_ADDRESS_FUNCTION_LAST 7
 
 typedef enum {
-   VIR_PCI_CONNECT_HOTPLUGGABLE = 1 << 0,
-   /* This bus supports hot-plug */
-   VIR_PCI_CONNECT_SINGLESLOT   = 1 << 1,
-   /* This "bus" has only a single downstream slot/port */
-
-   VIR_PCI_CONNECT_TYPE_PCI     = 1 << 2,
-   /* PCI devices can connect to this bus */
-   VIR_PCI_CONNECT_TYPE_PCIE    = 1 << 3,
-   /* PCI Express devices can connect to this bus */
-   VIR_PCI_CONNECT_TYPE_PCIE_ROOT = 1 << 4,
-   /* for devices that can only connect to pcie-root (i.e. root-port) */
-   VIR_PCI_CONNECT_TYPE_PCIE_PORT = 1 << 5,
-   /* devices that can only connect to a pcie-root-port
-    * or pcie-downstream-switch-port
+   VIR_PCI_CONNECT_HOTPLUGGABLE = 1 << 0, /* is hotplug needed/supported */
+
+   /* kinds of devices as a bitmap so they can be combined (some PCI
+    * controllers permit connecting multiple types of devices)
     */
-   VIR_PCI_CONNECT_TYPE_PCIE_SWITCH = 1 << 6,
-   /* devices that can only connect to a pcie-switch */
+   VIR_PCI_CONNECT_TYPE_PCI_DEVICE = 1 << 1,
+   VIR_PCI_CONNECT_TYPE_PCIE_DEVICE = 1 << 2,
+   VIR_PCI_CONNECT_TYPE_PCIE_ROOT_PORT = 1 << 3,
+   VIR_PCI_CONNECT_TYPE_PCIE_SWITCH_UPSTREAM_PORT = 1 << 4,
+   VIR_PCI_CONNECT_TYPE_PCIE_SWITCH_DOWNSTREAM_PORT = 1 << 5,
 } virDomainPCIConnectFlags;
 
+/* a combination of all bits that describe the type of connections
+ * allowed, e.g. PCI, PCIe, switch
+ */
+# define VIR_PCI_CONNECT_TYPES_MASK \
+   (VIR_PCI_CONNECT_TYPE_PCI_DEVICE | VIR_PCI_CONNECT_TYPE_PCIE_DEVICE | \
+    VIR_PCI_CONNECT_TYPE_PCIE_SWITCH_UPSTREAM_PORT | \
+    VIR_PCI_CONNECT_TYPE_PCIE_SWITCH_DOWNSTREAM_PORT | \
+    VIR_PCI_CONNECT_TYPE_PCIE_ROOT_PORT)
+
+/* combination of all bits that could be used to connect a normal
+ * endpoint device (i.e. excluding the connection possible between an
+ * upstream and downstream switch port, or a PCIe root port and a PCIe
+ * port)
+ */
+# define VIR_PCI_CONNECT_TYPES_ENDPOINT \
+   (VIR_PCI_CONNECT_TYPE_PCI_DEVICE | VIR_PCI_CONNECT_TYPE_PCIE_DEVICE)
+
 typedef struct {
     virDomainControllerModelPCI model;
-    /* flags an min/max can be computed from model, but
+    /* flags and min/max can be computed from model, but
      * having them ready makes life easier.
      */
     virDomainPCIConnectFlags flags;
-    size_t minSlot, maxSlot; /* usually 0,0 or 1,31 */
+    size_t minSlot, maxSlot; /* usually 0,0 or 0,31, or 1,31 */
     /* Each bit in a slot represents one function on that slot. If the
      * bit is set, that function is in use by a device.
      */
@@ -74,22 +84,6 @@ struct _virDomainPCIAddressSet {
 typedef struct _virDomainPCIAddressSet virDomainPCIAddressSet;
 typedef virDomainPCIAddressSet *virDomainPCIAddressSetPtr;
 
-/* a combination of all bits that describe the type of connections
- * allowed, e.g. PCI, PCIe, switch
- */
-# define VIR_PCI_CONNECT_TYPES_MASK \
-   (VIR_PCI_CONNECT_TYPE_PCI | VIR_PCI_CONNECT_TYPE_PCIE | \
-    VIR_PCI_CONNECT_TYPE_PCIE_ROOT | VIR_PCI_CONNECT_TYPE_PCIE_PORT | \
-    VIR_PCI_CONNECT_TYPE_PCIE_SWITCH)
-
-/* combination of all bits that could be used to connect a normal
- * endpoint device (i.e. excluding the connection possible between an
- * upstream and downstream switch port, or a PCIe root port and a PCIe
- * port)
- */
-# define VIR_PCI_CONNECT_TYPES_ENDPOINT \
-   (VIR_PCI_CONNECT_TYPE_PCI | VIR_PCI_CONNECT_TYPE_PCIE)
-
 char *virDomainPCIAddressAsString(virDevicePCIAddressPtr addr)
       ATTRIBUTE_NONNULL(1);
 

src/qemu/qemu_domain_address.c

@@ -427,7 +427,7 @@ qemuDomainCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED,
     bool entireSlot;
     /* flags may be changed from default below */
     virDomainPCIConnectFlags flags = (VIR_PCI_CONNECT_HOTPLUGGABLE |
-                                      VIR_PCI_CONNECT_TYPE_PCI);
+                                      VIR_PCI_CONNECT_TYPE_PCI_DEVICE);
 
     if ((info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)
         || ((device->type == VIR_DOMAIN_DEVICE_HOSTDEV) &&
@@ -451,31 +451,30 @@ qemuDomainCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED,
                 /* pci-bridge needs a PCI slot, but it isn't
                  * hot-pluggable, so it doesn't need a hot-pluggable slot.
                  */
-                flags = VIR_PCI_CONNECT_TYPE_PCI;
+                flags = VIR_PCI_CONNECT_TYPE_PCI_DEVICE;
                 break;
             case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE:
                 /* pci-bridge needs a PCIe slot, but it isn't
                  * hot-pluggable, so it doesn't need a hot-pluggable slot.
                  */
-                flags = VIR_PCI_CONNECT_TYPE_PCIE;
+                flags = VIR_PCI_CONNECT_TYPE_PCIE_DEVICE;
                 break;
             case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT:
-                /* pcie-root-port can only connect to pcie-root, isn't
-                 * hot-pluggable
+                /* pcie-root-port isn't hot-pluggable, and
+                 * is unique in what it can connect to, so
+                 * it has its own flag.
                  */
-                flags = VIR_PCI_CONNECT_TYPE_PCIE_ROOT;
+                flags = VIR_PCI_CONNECT_TYPE_PCIE_ROOT_PORT;
                 break;
             case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT:
-                /* pcie-switch can only connect to a true
-                 * pcie bus, and can't be hot-plugged.
+                /* pcie-switch-upstream-port is also unique, and
+                 * not hot-pluggable...
                  */
-                flags = VIR_PCI_CONNECT_TYPE_PCIE_PORT;
+                flags = VIR_PCI_CONNECT_TYPE_PCIE_SWITCH_UPSTREAM_PORT;
                 break;
             case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT:
-                /* pcie-switch-downstream-port can only connect to a
-                 * pcie-switch-upstream-port, and can't be hot-plugged.
-                 */
-                flags = VIR_PCI_CONNECT_TYPE_PCIE_SWITCH;
+                /* ... same for pcie-switch-downstream-port */
+                flags = VIR_PCI_CONNECT_TYPE_PCIE_SWITCH_DOWNSTREAM_PORT;
                 break;
             default:
                 break;
@@ -486,7 +485,8 @@ qemuDomainCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED,
             /* SATA controllers aren't hot-plugged, and can be put in
              * either a PCI or PCIe slot
              */
-            flags = VIR_PCI_CONNECT_TYPE_PCI | VIR_PCI_CONNECT_TYPE_PCIE;
+            flags = (VIR_PCI_CONNECT_TYPE_PCI_DEVICE
+                     | VIR_PCI_CONNECT_TYPE_PCIE_DEVICE);
             break;
 
         case VIR_DOMAIN_CONTROLLER_TYPE_USB:
@@ -500,13 +500,14 @@ qemuDomainCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED,
             case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI2:
             case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI3:
             case VIR_DOMAIN_CONTROLLER_MODEL_USB_VT82C686B_UHCI:
-              flags = VIR_PCI_CONNECT_TYPE_PCI;
+                flags = VIR_PCI_CONNECT_TYPE_PCI_DEVICE;
                 break;
             case VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI:
                 /* should this be PCIE-only? Or do we need to allow PCI
                  * for backward compatibility?
                  */
-              flags = VIR_PCI_CONNECT_TYPE_PCI | VIR_PCI_CONNECT_TYPE_PCIE;
+                flags = (VIR_PCI_CONNECT_TYPE_PCI_DEVICE
+                         | VIR_PCI_CONNECT_TYPE_PCIE_DEVICE);
                 break;
             case VIR_DOMAIN_CONTROLLER_MODEL_USB_PCI_OHCI:
             case VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI:
@@ -521,7 +522,7 @@ qemuDomainCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED,
         switch (device->data.sound->model) {
         case VIR_DOMAIN_SOUND_MODEL_ICH6:
         case VIR_DOMAIN_SOUND_MODEL_ICH9:
-            flags = VIR_PCI_CONNECT_TYPE_PCI;
+            flags = VIR_PCI_CONNECT_TYPE_PCI_DEVICE;
             break;
         }
         break;
@@ -530,7 +531,8 @@ qemuDomainCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED,
         /* video cards aren't hot-plugged, and can be put in either a
          * PCI or PCIe slot
          */
-        flags = VIR_PCI_CONNECT_TYPE_PCI | VIR_PCI_CONNECT_TYPE_PCIE;
+       flags = (VIR_PCI_CONNECT_TYPE_PCI_DEVICE
+                | VIR_PCI_CONNECT_TYPE_PCIE_DEVICE);
         break;
     }
 
@@ -562,7 +564,7 @@ qemuDomainCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED,
              * commandline, but that don't really care if a PCI bus
              * actually exists. */
             if (addrs->nbuses > 0 &&
-                !(addrs->buses[0].flags & VIR_PCI_CONNECT_TYPE_PCI)) {
+                !(addrs->buses[0].flags & VIR_PCI_CONNECT_TYPE_PCI_DEVICE)) {
                 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                                _("Bus 0 must be PCI for integrated PIIX3 "
                                  "USB or IDE controllers"));
@@ -653,7 +655,8 @@ qemuDomainValidateDevicePCISlotsPIIX3(virDomainDefPtr def,
     virDevicePCIAddress tmp_addr;
     bool qemuDeviceVideoUsable = virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIDEO_PRIMARY);
     char *addrStr = NULL;
-    virDomainPCIConnectFlags flags = VIR_PCI_CONNECT_HOTPLUGGABLE | VIR_PCI_CONNECT_TYPE_PCI;
+    virDomainPCIConnectFlags flags = (VIR_PCI_CONNECT_HOTPLUGGABLE
+                                      | VIR_PCI_CONNECT_TYPE_PCI_DEVICE);
 
     /* Verify that first IDE and USB controllers (if any) is on the PIIX3, fn 1 */
     for (i = 0; i < def->ncontrollers; i++) {
@@ -786,7 +789,7 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def,
     virDevicePCIAddress tmp_addr;
     bool qemuDeviceVideoUsable = virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIDEO_PRIMARY);
     char *addrStr = NULL;
-    virDomainPCIConnectFlags flags = VIR_PCI_CONNECT_TYPE_PCIE;
+    virDomainPCIConnectFlags flags = VIR_PCI_CONNECT_TYPE_PCIE_DEVICE;
 
     for (i = 0; i < def->ncontrollers; i++) {
         switch (def->controllers[i]->type) {
@@ -1049,7 +1052,7 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def,
                                virDomainPCIAddressSetPtr addrs)
 {
     size_t i, j;
-    virDomainPCIConnectFlags flags;
+    virDomainPCIConnectFlags flags = 0; /* initialize to quiet gcc warning */
     virDevicePCIAddress tmp_addr;
 
     /* PCI controllers */
@@ -1057,41 +1060,48 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def,
         if (def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) {
             if (def->controllers[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
                 continue;
-            switch (def->controllers[i]->model) {
+
+            /* convert the type of controller into a "CONNECT_TYPE"
+             * flag to use when searching for the proper
+             * controller/bus to connect it to on the upstream side.
+             */
+            switch ((virDomainControllerModelPCI)def->controllers[i]->model) {
             case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
             case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
                 /* pci-root and pcie-root are implicit in the machine,
-                 * and needs no address */
+                 * and need no address */
                 continue;
             case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE:
                 /* pci-bridge doesn't require hot-plug
                  * (although it does provide hot-plug in its slots)
                  */
-                flags = VIR_PCI_CONNECT_TYPE_PCI;
+                flags = VIR_PCI_CONNECT_TYPE_PCI_DEVICE;
                 break;
             case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE:
-                /* dmi-to-pci-bridge requires a non-hotplug PCIe
-                 * slot
+                /* dmi-to-pci-bridge is treated like a
+                 * non-hotplug-capable PCIe device (e.g. it can be
+                 * plugged directly into pcie-root)
                  */
-                flags = VIR_PCI_CONNECT_TYPE_PCIE;
+                flags = VIR_PCI_CONNECT_TYPE_PCIE_DEVICE;
                 break;
             case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT:
-                /* pcie-root-port can only plug into pcie-root */
-                flags = VIR_PCI_CONNECT_TYPE_PCIE_ROOT;
+                flags = VIR_PCI_CONNECT_TYPE_PCIE_ROOT_PORT;
                 break;
             case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT:
-                /* pcie-switch really does need a real PCIe
-                 * port, but it doesn't need to be pcie-root
-                 */
-                flags = VIR_PCI_CONNECT_TYPE_PCIE_PORT;
+                flags = VIR_PCI_CONNECT_TYPE_PCIE_SWITCH_UPSTREAM_PORT;
                 break;
             case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT:
-                /* pcie-switch-port can only plug into pcie-switch */
-                flags = VIR_PCI_CONNECT_TYPE_PCIE_SWITCH;
-                break;
-            default:
-                flags = VIR_PCI_CONNECT_HOTPLUGGABLE | VIR_PCI_CONNECT_TYPE_PCI;
+                flags = VIR_PCI_CONNECT_TYPE_PCIE_SWITCH_DOWNSTREAM_PORT;
                 break;
+
+            case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST:
+                /* if this happens, there is an error in the code. A
+                 * PCI controller should always have a proper model
+                 * set
+                 */
+                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                               _("PCI controller model icorrectly set to 'last'"));
+                goto error;
             }
             if (virDomainPCIAddressReserveNextSlot(addrs,
                                                    &def->controllers[i]->info,
@@ -1100,7 +1110,12 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def,
         }
     }
 
-    flags = VIR_PCI_CONNECT_HOTPLUGGABLE | VIR_PCI_CONNECT_TYPE_PCI;
+    /* all other devices that plug into a PCI slot are treated as a
+     * PCI endpoint devices that require a hotplug-capable slot
+     * (except for some special cases which have specific handling
+     * below)
+     */
+    flags = VIR_PCI_CONNECT_HOTPLUGGABLE | VIR_PCI_CONNECT_TYPE_PCI_DEVICE;
 
     for (i = 0; i < def->nfss; i++) {
         if (def->fss[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
@@ -1414,7 +1429,7 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def,
         int rv;
         bool buses_reserved = true;
 
-        virDomainPCIConnectFlags flags = VIR_PCI_CONNECT_TYPE_PCI;
+        virDomainPCIConnectFlags flags = VIR_PCI_CONNECT_TYPE_PCI_DEVICE;
 
         for (i = 0; i < def->ncontrollers; i++) {
             if (def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) {