[12.0-stable] Revert "domainmgr: do pci-reserve on start of edge app"

pkg/pillar/cmd/domainmgr/domainmgr.go

@@ -1344,9 +1344,61 @@ func handleCreate(ctx *domainContext, key string, config *types.DomainConfig) {
 		config.UUIDandVersion, config.DisplayName)
 }
 
+// returns a map of PciLong to *types.IoBundle
+func usbControllersWithoutPCIReserve(ioBundles []types.IoBundle) map[string][]*types.IoBundle {
+	ret := make(map[string][]*types.IoBundle, 0)
+
+	usbControllerGroups := make(map[string][]*types.IoBundle) // assigngrp -> iobundle
+
+	for i, ioBundle := range ioBundles {
+		if ioBundle.Type != types.IoUSBController {
+			continue
+		}
+
+		if usbControllerGroups[ioBundle.AssignmentGroup] == nil {
+			usbControllerGroups[ioBundle.AssignmentGroup] = make([]*types.IoBundle, 0)
+		}
+
+		usbControllerGroups[ioBundle.AssignmentGroup] = append(usbControllerGroups[ioBundle.AssignmentGroup], &ioBundles[i])
+	}
+
+	for _, ioBundle := range ioBundles {
+		if ioBundle.UsbAddr == "" && ioBundle.UsbProduct == "" && ioBundle.Type != types.IoUSBDevice {
+			continue
+		}
+
+		if ioBundle.ParentAssignmentGroup == "" {
+			ret = make(map[string][]*types.IoBundle, 0)
+
+			for i, usbControllers := range usbControllerGroups {
+				for j, usbController := range usbControllers {
+					if ret[usbController.PciLong] == nil {
+						ret[usbController.PciLong] = make([]*types.IoBundle, 0)
+					}
+
+					ret[usbController.PciLong] = append(ret[usbController.PciLong], usbControllerGroups[i][j])
+				}
+			}
+
+			return ret
+		}
+
+		if usbControllerGroups[ioBundle.ParentAssignmentGroup] != nil {
+			for i, usbController := range usbControllerGroups[ioBundle.ParentAssignmentGroup] {
+				if ret[usbController.PciLong] == nil {
+					ret[usbController.PciLong] = make([]*types.IoBundle, 0)
+				}
+
+				ret[usbController.PciLong] = append(ret[usbController.PciLong], usbControllerGroups[ioBundle.ParentAssignmentGroup][i])
+			}
+		}
+	}
+
+	return ret
+}
+
 // doAssignAdaptersToDomain assigns IO adapters to the newly created domain.
-// The adapters are reserved here for the domain
-// UsedByUUID is already set in reserveAdapters
+// Note that the adapters are already reserved for the domain using reserveAdapters (UsedByUUID is set).
 func doAssignIoAdaptersToDomain(ctx *domainContext, config types.DomainConfig,
 	status *types.DomainStatus) error {
 
@@ -1396,8 +1448,8 @@ func doAssignIoAdaptersToDomain(ctx *domainContext, config types.DomainConfig,
 			}
 		}
 		publishAssignableAdapters = len(assignmentsUsb) > 0 || len(assignmentsPci) > 0
-
 	}
+
 	for i, long := range assignmentsPci {
 		err := hyper.PCIReserve(long)
 		if err != nil {
@@ -1865,6 +1917,7 @@ func unmountContainers(ctx *domainContext, diskStatusList []types.DiskStatus, fo
 
 // releaseAdapters is called when the domain is done with the device and we
 // clear UsedByUUID
+// In addition, if KeepInHost is set, we move it back to the host.
 // If status is set, any errors are recorded in status
 func releaseAdapters(ctx *domainContext, ioAdapterList []types.IoAdapter,
 	myUUID uuid.UUID, status *types.DomainStatus) {
@@ -1895,7 +1948,7 @@ func releaseAdapters(ctx *domainContext, ioAdapterList []types.IoAdapter,
 					myUUID)
 				continue
 			}
-			if ib.PciLong != "" && ib.IsPCIBack {
+			if ib.PciLong != "" && ib.KeepInHost && ib.IsPCIBack {
 				log.Functionf("releaseAdapters removing %s (%s) from %s",
 					ib.Phylabel, ib.PciLong, myUUID)
 				assignments = addNoDuplicate(assignments, ib.PciLong)
@@ -2974,6 +3027,13 @@ func updatePortAndPciBackIoBundleAll(ctx *domainContext) {
 	if anyChanged {
 		ctx.publishAssignableAdapters()
 	}
+
+	keepInHostUsbControllers := usbControllersWithoutPCIReserve(ctx.assignableAdapters.IoBundleList)
+	for i := range keepInHostUsbControllers {
+		for j := range keepInHostUsbControllers[i] {
+			keepInHostUsbControllers[i][j].KeepInHost = true
+		}
+	}
 }
 
 // updatePortAndPciBackIoBundle determines whether IsPort should be set for
@@ -2992,6 +3052,9 @@ func updatePortAndPciBackIoBundle(ctx *domainContext, ib *types.IoBundle) (chang
 	} else {
 		list = append(list, ib)
 	}
+
+	keepInHostUsbControllers := usbControllersWithoutPCIReserve(ctx.assignableAdapters.IoBundleList)
+
 	// Is any member a network port?
 	// We look across all members in the assignment group (expanded below
 	// for safety when the model is incorrect) and if any member is a port
@@ -3040,6 +3103,10 @@ func updatePortAndPciBackIoBundle(ctx *domainContext, ib *types.IoBundle) (chang
 		if ib.Type == types.IoCAN || ib.Type == types.IoVCAN {
 			keepInHost = true
 		}
+		_, found := keepInHostUsbControllers[ib.PciLong]
+		if found {
+			keepInHost = true
+		}
 	}
 
 	log.Functionf("updatePortAndPciBackIoBundle(%d %s %s) isPort %t keepInHost %t members %d",
@@ -3136,9 +3203,13 @@ func updatePortAndPciBackIoMember(ctx *domainContext, ib *types.IoBundle, isPort
 			log.Noticef("Not assigning %s (%s) to pciback due to Testing",
 				ib.Phylabel, ib.PciLong)
 		} else if ib.PciLong != "" && ib.UsbAddr == "" {
-			log.Noticef("Assigning %s (%s) later to pciback",
+			log.Noticef("Assigning %s (%s) to pciback",
 				ib.Phylabel, ib.PciLong)
-
+			err := hyper.PCIReserve(ib.PciLong)
+			if err != nil {
+				return changed, err
+			}
+			ib.IsPCIBack = true
 			changed = true
 		}
 	}

pkg/pillar/cmd/domainmgr/domainmgr_test.go

@@ -483,6 +483,113 @@ func TestHandleMimeMultipart(t *testing.T) {
 	os.RemoveAll(dir)
 }
 
+func TestUsbControllersNoImmediatePCIReserve(t *testing.T) {
+	var pciLongs map[string]struct{}
+
+	noPCIReserve := []types.IoBundle{
+		{
+			Type:                  types.IoUSBController,
+			AssignmentGroup:       "",
+			ParentAssignmentGroup: "",
+			PciLong:               "00:01",
+		},
+	}
+
+	setPciLongs := func(ioBundles []types.IoBundle) {
+		pciLongs = make(map[string]struct{})
+		for p := range usbControllersWithoutPCIReserve(ioBundles) {
+			pciLongs[p] = struct{}{}
+		}
+	}
+	setPciLongs(noPCIReserve)
+	if len(pciLongs) > 0 {
+		t.Fatalf("expected no controllers that should be reserved, but got %+v", pciLongs)
+	}
+
+	noPCIReserve = append(noPCIReserve, types.IoBundle{
+		Type: types.IoUSBDevice,
+	})
+
+	setPciLongs(noPCIReserve)
+	if len(pciLongs) != 1 {
+		t.Fatalf("expected controller to be reserved for usb device, but got %+v", pciLongs)
+	}
+
+	noPCIReserveBecauseOfDifferentAssigngrp := []types.IoBundle{
+		{
+			Type:                  types.IoUSBController,
+			AssignmentGroup:       "one",
+			ParentAssignmentGroup: "",
+			PciLong:               "00:01",
+		},
+		{
+			Type:                  types.IoUSBDevice,
+			AssignmentGroup:       "",
+			ParentAssignmentGroup: "two",
+		},
+	}
+
+	setPciLongs(noPCIReserveBecauseOfDifferentAssigngrp)
+	if len(pciLongs) > 0 {
+		t.Fatalf("expected no controllers that should be reserved, but got %+v", pciLongs)
+	}
+
+	noPCIReserveBecauseOfDifferentAssigngrp = append(noPCIReserveBecauseOfDifferentAssigngrp, types.IoBundle{
+		Type: types.IoUSBDevice,
+	})
+	setPciLongs(noPCIReserveBecauseOfDifferentAssigngrp)
+	if len(pciLongs) != 1 {
+		t.Fatalf("expected controller to be reserved for usb device, but got %+v", pciLongs)
+	}
+
+	pciReserveBecauseOfMatchingAssigngrp := []types.IoBundle{
+		{
+			Type:                  types.IoUSBController,
+			AssignmentGroup:       "one",
+			ParentAssignmentGroup: "",
+			PciLong:               "00:01",
+		},
+		{
+			Type:                  types.IoUSBDevice,
+			AssignmentGroup:       "",
+			ParentAssignmentGroup: "one",
+		},
+	}
+	setPciLongs(pciReserveBecauseOfMatchingAssigngrp)
+	if len(pciLongs) != 1 {
+		t.Fatalf("expected controller to be reserved for usb device, but got %+v", pciLongs)
+	}
+
+	pciReserveBecauseOfMatchingAssigngrp = append(pciReserveBecauseOfMatchingAssigngrp, types.IoBundle{
+		Type:            types.IoUSBController,
+		AssignmentGroup: "one",
+		PciLong:         "00:02",
+	})
+	setPciLongs(pciReserveBecauseOfMatchingAssigngrp)
+	if len(pciLongs) != 2 {
+		t.Fatalf("expected both controllers to be reserved for usb device, but got %+v", pciLongs)
+	}
+
+	pciReserveBecauseOfMatchingAssigngrp = append(pciReserveBecauseOfMatchingAssigngrp, types.IoBundle{
+		Type:            types.IoUSBController,
+		AssignmentGroup: "two", // no usb device depends on it, therefore pci reservation can be done
+		PciLong:         "00:03",
+	})
+	setPciLongs(pciReserveBecauseOfMatchingAssigngrp)
+	if len(pciLongs) != 2 {
+		t.Fatalf("expected both controllers to be reserved for usb device, but got %+v", pciLongs)
+	}
+
+	pciReserveBecauseOfMatchingAssigngrp = append(pciReserveBecauseOfMatchingAssigngrp, types.IoBundle{
+		Type:                  types.IoUSBDevice,
+		ParentAssignmentGroup: "",
+	})
+	setPciLongs(pciReserveBecauseOfMatchingAssigngrp)
+	if len(pciLongs) != 3 {
+		t.Fatalf("expected all three controllers to be reserved for usb device, but got %+v", pciLongs)
+	}
+}
+
 func TestConfigEnableUsbUpdatePortAndPciBackIoBundle(t *testing.T) {
 	assignableAdapters := types.AssignableAdapters{
 		IoBundleList: []types.IoBundle{

pkg/pillar/hypervisor/kvm.go

@@ -393,7 +393,6 @@ const qemuCANBusTemplate = `
 `
 
 const kvmStateDir = "/run/hypervisor/kvm/"
-const sysfsVfioPciBind = "/sys/bus/pci/drivers/vfio-pci/bind"
 const sysfsPciDriversProbe = "/sys/bus/pci/drivers_probe"
 const vfioDriverPath = "/sys/bus/pci/drivers/vfio-pci"