bhyve: soften requirements for slot 1

Currently, slot 1 is only allowed to be used by the LPC device.
Relax this requirement and allow to use slot 1 if it was explicitly
specified by the user for any other device type. In this case the LPC
device will have the next available address.

If slot 1 was not used by the user, it'll be reserved for the LPC
device, even if it is not configured to make address assignment
consistent in case the LPC device becomes necessary (e.g. the user
adds a console or a video device which require LPC).

Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

diff --git a/po/POTFILES.in b/po/POTFILES.in
index 54f78e7861cb5b83eb56926b48553808dc140011..9782b2beb402ee57fe3bdb537371acaec87fb5d2 100644 (file)
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -12,7 +12,6 @@
 @SRCDIR@src/admin/libvirt-admin.c
 @SRCDIR@src/bhyve/bhyve_capabilities.c
 @SRCDIR@src/bhyve/bhyve_command.c
-@SRCDIR@src/bhyve/bhyve_device.c
 @SRCDIR@src/bhyve/bhyve_domain.c
 @SRCDIR@src/bhyve/bhyve_driver.c
 @SRCDIR@src/bhyve/bhyve_monitor.c

diff --git a/src/bhyve/bhyve_device.c b/src/bhyve/bhyve_device.c
index 2295acf5528fe01534b51ef5c85a7f0bff7e6f39..e2e1efd97ebf2b9484644a64270922a90428ca37 100644 (file)
--- a/src/bhyve/bhyve_device.c
+++ b/src/bhyve/bhyve_device.c
@@ -42,21 +42,8 @@ bhyveCollectPCIAddress(virDomainDefPtr def G_GNUC_UNUSED,
     virDomainPCIAddressSetPtr addrs = opaque;
     virPCIDeviceAddressPtr addr = &info->addr.pci;
 
-    if (addr->domain == 0 && addr->bus == 0) {
-        if (addr->slot == 0) {
+    if (addr->domain == 0 && addr->bus == 0 && addr->slot == 0) {
             return 0;
-        } else if (addr->slot == 1) {
-            if (!(device->type == VIR_DOMAIN_DEVICE_CONTROLLER &&
-                  device->data.controller->type == VIR_DOMAIN_CONTROLLER_TYPE_ISA)) {
-                 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                                _("PCI bus 0 slot 1 is reserved for the implicit "
-                                  "LPC PCI-ISA bridge"));
-                 return -1;
-            } else {
-                /* We reserve slot 1 for LPC in bhyveAssignDevicePCISlots(), so exit early */
-                return 0;
-            }
-        }
     }
 
     if (virDomainPCIAddressReserveAddr(addrs, addr,
@@ -98,29 +85,38 @@ bhyveAssignDevicePCISlots(virDomainDefPtr def,
     size_t i;
     virPCIDeviceAddress lpc_addr;
 
-    /* explicitly reserve slot 1 for LPC-ISA bridge */
     memset(&lpc_addr, 0, sizeof(lpc_addr));
     lpc_addr.slot = 0x1;
 
-    if (virDomainPCIAddressReserveAddr(addrs, &lpc_addr,
-                                       VIR_PCI_CONNECT_TYPE_PCI_DEVICE, 0) < 0) {
-        return -1;
-    }
+    /* If the user didn't explicitly specify slot 1 for some of the devices,
+       reserve it for LPC, even if there's no LPC device configured.
+       If the slot 1 is used by some other device, LPC will have an address
+       auto-assigned.
 
-    for (i = 0; i < def->ncontrollers; i++) {
-         if ((def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_ISA) &&
-              virDeviceInfoPCIAddressIsWanted(&def->controllers[i]->info)) {
-             def->controllers[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
-             def->controllers[i]->info.addr.pci = lpc_addr;
-             break;
-         }
+       The idea behind that is to try to use slot 1 for the LPC device unless
+       user specifically configured otherwise.*/
+    if (!virDomainPCIAddressSlotInUse(addrs, &lpc_addr)) {
+        if (virDomainPCIAddressReserveAddr(addrs, &lpc_addr,
+                                           VIR_PCI_CONNECT_TYPE_PCI_DEVICE, 0) < 0) {
+            return -1;
+        }
+
+        for (i = 0; i < def->ncontrollers; i++) {
+             if ((def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_ISA) &&
+                  virDeviceInfoPCIAddressIsWanted(&def->controllers[i]->info)) {
+                 def->controllers[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
+                 def->controllers[i]->info.addr.pci = lpc_addr;
+                 break;
+             }
+        }
     }
 
     for (i = 0; i < def->ncontrollers; i++) {
         if ((def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) ||
             (def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_SATA) ||
             ((def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_USB) &&
-             (def->controllers[i]->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI))) {
+             (def->controllers[i]->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI)) ||
+            def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_ISA) {
             if (def->controllers[i]->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT ||
                 !virDeviceInfoPCIAddressIsWanted(&def->controllers[i]->info))
                 continue;

diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.xml
index 88ad9aebb72aa98d5b3ad4d481a362f469e7af23..222a11b7d0bc2ec357aa0a0dcb872a0dbcd78ce4 100644 (file)
--- a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.xml
+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.xml
@@ -5,6 +5,7 @@
   <vcpu>1</vcpu>
   <os>
     <type>hvm</type>
+    <loader readonly="yes" type="pflash">/path/to/test.fd</loader>
   </os>
   <devices>
     <disk type='file'>

diff --git a/tests/bhyvexml2argvtest.c b/tests/bhyvexml2argvtest.c
index b86a4243c233b4a74ec6be5206ce909ee2cf9f14..2167cd6310270d3335663e910c89ee3ff2fbe368 100644 (file)
--- a/tests/bhyvexml2argvtest.c
+++ b/tests/bhyvexml2argvtest.c
@@ -213,7 +213,7 @@ mymain(void)
     DO_TEST("addr-multiple-virtio-disks");
     DO_TEST("addr-isa-controller-on-slot-1");
     DO_TEST("addr-isa-controller-on-slot-31");
-    DO_TEST_FAILURE("addr-non-isa-controller-on-slot-1");
+    DO_TEST("addr-non-isa-controller-on-slot-1");
 
     /* The same without 32 devs per controller support */
     driver.bhyvecaps ^= BHYVE_CAP_AHCI32SLOT;