diff --git a/src/runtime/config/configuration-qemu.toml.in b/src/runtime/config/configuration-qemu.toml.in index f58f871418..594261c3df 100644 --- a/src/runtime/config/configuration-qemu.toml.in +++ b/src/runtime/config/configuration-qemu.toml.in @@ -352,8 +352,15 @@ pflashes = [] # Default false #hotplug_vfio_on_root_bus = true +# Enable hot-plugging of VFIO devices to a bridge-port, +# root-port or switch-port. +# The default setting is "no-port" +#hot_plug_vfio = "root-port" + # In a confidential compute environment hot-plugging can compromise -# security. Enable cold-plugging of VFIO devices to a root-port. +# security. +# Enable cold-plugging of VFIO devices to a bridge-port, +# root-port or switch-port. # The default setting is "no-port", which means disabled. #cold_plug_vfio = "root-port" diff --git a/src/runtime/pkg/device/drivers/utils.go b/src/runtime/pkg/device/drivers/utils.go index 19ac99c30b..f0c2109545 100644 --- a/src/runtime/pkg/device/drivers/utils.go +++ b/src/runtime/pkg/device/drivers/utils.go @@ -47,7 +47,7 @@ func deviceLogger() *logrus.Entry { return api.DeviceLogger() } -// IsPCIeDevice Identify PCIe device by reading the size of the PCI config space +// IsPCIeDevice Identifies PCIe device by reading the size of the PCI config space // Plain PCI device have 256 bytes of config space where PCIe devices have 4K func IsPCIeDevice(bdf string) bool { if len(strings.Split(bdf, ":")) == 2 { @@ -157,9 +157,7 @@ func checkIgnorePCIClass(pciClass string, deviceBDF string, bitmask uint64) (boo // GetAllVFIODevicesFromIOMMUGroup returns all the VFIO devices in the IOMMU group // We can reuse this function at various levels, sandbox, container. -// Only the VFIO module is allowed to do bus assignments, all other modules need to -// ignore it if used as helper function to get VFIO information. -func GetAllVFIODevicesFromIOMMUGroup(device config.DeviceInfo, ignoreBusAssignment bool) ([]*config.VFIODev, error) { +func GetAllVFIODevicesFromIOMMUGroup(device config.DeviceInfo) ([]*config.VFIODev, error) { vfioDevs := []*config.VFIODev{} @@ -207,8 +205,8 @@ func GetAllVFIODevicesFromIOMMUGroup(device config.DeviceInfo, ignoreBusAssignme Class: pciClass, Rank: -1, } - if isPCIe && !ignoreBusAssignment { - vfioPCI.Bus = fmt.Sprintf("%s%d", pcieRootPortPrefix, len(AllPCIeDevs)) + if isPCIe { + vfioPCI.Rank = len(AllPCIeDevs) AllPCIeDevs[deviceBDF] = true } vfio = vfioPCI diff --git a/src/runtime/pkg/device/drivers/vfio.go b/src/runtime/pkg/device/drivers/vfio.go index 4231f0d301..3604a355d0 100644 --- a/src/runtime/pkg/device/drivers/vfio.go +++ b/src/runtime/pkg/device/drivers/vfio.go @@ -73,7 +73,7 @@ func (device *VFIODevice) Attach(ctx context.Context, devReceiver api.DeviceRece } }() - device.VfioDevs, err = GetAllVFIODevicesFromIOMMUGroup(*device.DeviceInfo, false) + device.VfioDevs, err = GetAllVFIODevicesFromIOMMUGroup(*device.DeviceInfo) if err != nil { return err } diff --git a/src/runtime/pkg/katautils/config-settings.go.in b/src/runtime/pkg/katautils/config-settings.go.in index a6825a8a0d..521f7b60fa 100644 --- a/src/runtime/pkg/katautils/config-settings.go.in +++ b/src/runtime/pkg/katautils/config-settings.go.in @@ -109,7 +109,7 @@ const defaultVMCacheEndpoint string = "/var/run/kata-containers/cache.sock" // Default config file used by stateless systems. var defaultRuntimeConfiguration = "@CONFIG_PATH@" -const defaultHotPlugVFIO = hv.BridgePort +const defaultHotPlugVFIO = hv.NoPort const defaultColdPlugVFIO = hv.NoPort diff --git a/src/runtime/pkg/katautils/config.go b/src/runtime/pkg/katautils/config.go index c0a0372dfb..75a2469844 100644 --- a/src/runtime/pkg/katautils/config.go +++ b/src/runtime/pkg/katautils/config.go @@ -76,6 +76,7 @@ type factory struct { VMCacheNumber uint `toml:"vm_cache_number"` Template bool `toml:"enable_template"` } + type hypervisor struct { Path string `toml:"path"` JailerPath string `toml:"jailer_path"` @@ -886,6 +887,7 @@ func newQemuHypervisorConfig(h hypervisor) (vc.HypervisorConfig, error) { GuestMemoryDumpPath: h.GuestMemoryDumpPath, GuestMemoryDumpPaging: h.GuestMemoryDumpPaging, ConfidentialGuest: h.ConfidentialGuest, + SevSnpGuest: h.SevSnpGuest, GuestSwap: h.GuestSwap, Rootless: h.Rootless, LegacySerial: h.LegacySerial, @@ -1677,10 +1679,7 @@ func checkConfig(config oci.RuntimeConfig) error { hotPlugVFIO := config.HypervisorConfig.HotPlugVFIO coldPlugVFIO := config.HypervisorConfig.ColdPlugVFIO machineType := config.HypervisorConfig.HypervisorMachineType - if err := checkPCIeConfig(coldPlugVFIO, machineType); err != nil { - return err - } - if err := checkPCIeConfig(hotPlugVFIO, machineType); err != nil { + if err := checkPCIeConfig(coldPlugVFIO, hotPlugVFIO, machineType); err != nil { return err } @@ -1690,19 +1689,29 @@ func checkConfig(config oci.RuntimeConfig) error { // checkPCIeConfig ensures the PCIe configuration is valid. // Only allow one of the following settings for cold-plug: // no-port, root-port, switch-port -func checkPCIeConfig(vfioPort hv.PCIePort, machineType string) error { +func checkPCIeConfig(coldPlug hv.PCIePort, hotPlug hv.PCIePort, machineType string) error { // Currently only QEMU q35 supports advanced PCIe topologies // firecracker, dragonball do not have right now any PCIe support if machineType != "q35" { return nil } - if vfioPort == hv.NoPort || vfioPort == hv.BridgePort || - vfioPort == hv.RootPort || vfioPort == hv.SwitchPort { + + if coldPlug != hv.NoPort && hotPlug != hv.NoPort { + return fmt.Errorf("invalid hot-plug=%s and cold-plug=%s settings, only one of them can be set", coldPlug, hotPlug) + } + var port hv.PCIePort + if coldPlug != hv.NoPort { + port = coldPlug + } + if hotPlug != hv.NoPort { + port = hotPlug + } + if port == hv.NoPort || port == hv.BridgePort || port == hv.RootPort || port == hv.SwitchPort { return nil } return fmt.Errorf("invalid vfio_port=%s setting, allowed values %s, %s, %s, %s", - vfioPort, hv.NoPort, hv.BridgePort, hv.RootPort, hv.SwitchPort) + coldPlug, hv.NoPort, hv.BridgePort, hv.RootPort, hv.SwitchPort) } // checkNetNsConfig performs sanity checks on disable_new_netns config. diff --git a/src/runtime/pkg/oci/utils_test.go b/src/runtime/pkg/oci/utils_test.go index f3899f1b81..7b2c10f2f0 100644 --- a/src/runtime/pkg/oci/utils_test.go +++ b/src/runtime/pkg/oci/utils_test.go @@ -821,22 +821,22 @@ func TestRegexpContains(t *testing.T) { //nolint: govet type testData struct { - toMatch string regexps []string + toMatch string expected bool } data := []testData{ - {regexps: []string{}, toMatch: "", expected: false}, - {regexps: []string{}, toMatch: "nonempty", expected: false}, - {regexps: []string{"simple"}, toMatch: "simple", expected: true}, - {regexps: []string{"simple"}, toMatch: "some_simple_text", expected: true}, - {regexps: []string{"simple"}, toMatch: "simp", expected: false}, - {regexps: []string{"one", "two"}, toMatch: "one", expected: true}, - {regexps: []string{"one", "two"}, toMatch: "two", expected: true}, - {regexps: []string{"o*"}, toMatch: "oooo", expected: true}, - {regexps: []string{"o*"}, toMatch: "oooa", expected: true}, - {regexps: []string{"^o*$"}, toMatch: "oooa", expected: false}, + {[]string{}, "", false}, + {[]string{}, "nonempty", false}, + {[]string{"simple"}, "simple", true}, + {[]string{"simple"}, "some_simple_text", true}, + {[]string{"simple"}, "simp", false}, + {[]string{"one", "two"}, "one", true}, + {[]string{"one", "two"}, "two", true}, + {[]string{"o*"}, "oooo", true}, + {[]string{"o*"}, "oooa", true}, + {[]string{"^o*$"}, "oooa", false}, } for _, d := range data { @@ -850,25 +850,25 @@ func TestCheckPathIsInGlobs(t *testing.T) { //nolint: govet type testData struct { - toMatch string globs []string + toMatch string expected bool } data := []testData{ - {globs: []string{}, toMatch: "", expected: false}, - {globs: []string{}, toMatch: "nonempty", expected: false}, - {globs: []string{"simple"}, toMatch: "simple", expected: false}, - {globs: []string{"simple"}, toMatch: "some_simple_text", expected: false}, - {globs: []string{"/bin/ls"}, toMatch: "/bin/ls", expected: true}, - {globs: []string{"/bin/ls", "/bin/false"}, toMatch: "/bin/ls", expected: true}, - {globs: []string{"/bin/ls", "/bin/false"}, toMatch: "/bin/false", expected: true}, - {globs: []string{"/bin/ls", "/bin/false"}, toMatch: "/bin/bar", expected: false}, - {globs: []string{"/bin/*ls*"}, toMatch: "/bin/ls", expected: true}, - {globs: []string{"/bin/*ls*"}, toMatch: "/bin/false", expected: true}, - {globs: []string{"bin/ls"}, toMatch: "/bin/ls", expected: false}, - {globs: []string{"./bin/ls"}, toMatch: "/bin/ls", expected: false}, - {globs: []string{"*/bin/ls"}, toMatch: "/bin/ls", expected: false}, + {[]string{}, "", false}, + {[]string{}, "nonempty", false}, + {[]string{"simple"}, "simple", false}, + {[]string{"simple"}, "some_simple_text", false}, + {[]string{"/bin/ls"}, "/bin/ls", true}, + {[]string{"/bin/ls", "/bin/false"}, "/bin/ls", true}, + {[]string{"/bin/ls", "/bin/false"}, "/bin/false", true}, + {[]string{"/bin/ls", "/bin/false"}, "/bin/bar", false}, + {[]string{"/bin/*ls*"}, "/bin/ls", true}, + {[]string{"/bin/*ls*"}, "/bin/false", true}, + {[]string{"bin/ls"}, "/bin/ls", false}, + {[]string{"./bin/ls"}, "/bin/ls", false}, + {[]string{"*/bin/ls"}, "/bin/ls", false}, } for _, d := range data { @@ -923,10 +923,10 @@ func TestParseAnnotationUintConfiguration(t *testing.T) { // nolint: govet testCases := []struct { - err error annotations map[string]string - validFunc func(uint64) error expected uint64 + err error + validFunc func(uint64) error }{ { annotations: map[string]string{key: ""}, @@ -1007,10 +1007,10 @@ func TestParseAnnotationBoolConfiguration(t *testing.T) { // nolint: govet testCases := []struct { - err error annotationKey string annotationValueList []string expected bool + err error }{ { annotationKey: boolKey, @@ -1207,8 +1207,8 @@ func TestNewMount(t *testing.T) { assert := assert.New(t) testCases := []struct { - in specs.Mount out vc.Mount + in specs.Mount }{ { in: specs.Mount{ diff --git a/src/runtime/virtcontainers/acrn.go b/src/runtime/virtcontainers/acrn.go index 735b20019f..35c71a6d61 100644 --- a/src/runtime/virtcontainers/acrn.go +++ b/src/runtime/virtcontainers/acrn.go @@ -45,10 +45,10 @@ type AcrnState struct { // Acrn is an Hypervisor interface implementation for the Linux acrn hypervisor. type Acrn struct { + sandbox *Sandbox ctx context.Context arch acrnArch store persistapi.PersistDriver - sandbox *Sandbox id string acrnConfig Config config HypervisorConfig diff --git a/src/runtime/virtcontainers/clh.go b/src/runtime/virtcontainers/clh.go index cec2e1c634..05aa15b154 100644 --- a/src/runtime/virtcontainers/clh.go +++ b/src/runtime/virtcontainers/clh.go @@ -246,15 +246,15 @@ func (s *CloudHypervisorState) reset() { } type cloudHypervisor struct { - vmconfig chclient.VmConfig console console.Console virtiofsDaemon VirtiofsDaemon - ctx context.Context APIClient clhClient + ctx context.Context + id string netDevices *[]chclient.NetConfig devicesIds map[string]string netDevicesFiles map[string][]*os.File - id string + vmconfig chclient.VmConfig state CloudHypervisorState config HypervisorConfig stopped int32 diff --git a/src/runtime/virtcontainers/clh_test.go b/src/runtime/virtcontainers/clh_test.go index 0ab7ef5b96..8d47b731ef 100644 --- a/src/runtime/virtcontainers/clh_test.go +++ b/src/runtime/virtcontainers/clh_test.go @@ -188,13 +188,13 @@ func TestCloudHypervisorAddNetCheckEnpointTypes(t *testing.T) { } // nolint: govet tests := []struct { - args args name string + args args wantErr bool }{ - {name: "TapEndpoint", args: args{e: &TapEndpoint{}}, wantErr: true}, - {name: "Empty VethEndpoint", args: args{e: &VethEndpoint{}}, wantErr: true}, - {name: "Valid VethEndpoint", args: args{e: validVeth}, wantErr: false}, + {"TapEndpoint", args{e: &TapEndpoint{}}, true}, + {"Empty VethEndpoint", args{e: &VethEndpoint{}}, true}, + {"Valid VethEndpoint", args{e: validVeth}, false}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/src/runtime/virtcontainers/hypervisor.go b/src/runtime/virtcontainers/hypervisor.go index 7d14ab448b..43974adf68 100644 --- a/src/runtime/virtcontainers/hypervisor.go +++ b/src/runtime/virtcontainers/hypervisor.go @@ -507,7 +507,7 @@ type HypervisorConfig struct { // RawDevics are used to get PCIe device info early before the sandbox // is started to make better PCIe topology decisions - RawDevices []config.DeviceInfo + VFIODevices []config.DeviceInfo // HotplugVFIO is used to indicate if devices need to be hotplugged on the // root port or a switch diff --git a/src/runtime/virtcontainers/kata_agent.go b/src/runtime/virtcontainers/kata_agent.go index ed2be337c9..a350c1a486 100644 --- a/src/runtime/virtcontainers/kata_agent.go +++ b/src/runtime/virtcontainers/kata_agent.go @@ -284,16 +284,23 @@ type KataAgentState struct { // nolint: govet type kataAgent struct { - ctx context.Context - vmSocket interface{} - client *kataclient.AgentClient - reqHandlers map[string]reqFunc - state KataAgentState - kmodules []string + ctx context.Context + vmSocket interface{} + + client *kataclient.AgentClient + + // lock protects the client pointer sync.Mutex + + state KataAgentState + + reqHandlers map[string]reqFunc + kmodules []string + dialTimout uint32 - keepConn bool - dead bool + + keepConn bool + dead bool } func (k *kataAgent) Logger() *logrus.Entry { diff --git a/src/runtime/virtcontainers/kata_agent_test.go b/src/runtime/virtcontainers/kata_agent_test.go index 8fba818376..3653674ca9 100644 --- a/src/runtime/virtcontainers/kata_agent_test.go +++ b/src/runtime/virtcontainers/kata_agent_test.go @@ -235,10 +235,10 @@ func TestHandleDeviceBlockVolume(t *testing.T) { // nolint: govet tests := []struct { - inputDev *drivers.BlockDevice - resultVol *pb.Storage BlockDeviceDriver string inputMount Mount + inputDev *drivers.BlockDevice + resultVol *pb.Storage }{ { inputDev: &drivers.BlockDevice{ @@ -1024,10 +1024,10 @@ func TestKataAgentKernelParams(t *testing.T) { // nolint: govet type testData struct { - expectedParams []Param - containerPipeSize uint32 debug bool trace bool + containerPipeSize uint32 + expectedParams []Param } debugParam := Param{Key: "agent.log", Value: "debug"} @@ -1036,28 +1036,28 @@ func TestKataAgentKernelParams(t *testing.T) { containerPipeSizeParam := Param{Key: vcAnnotations.ContainerPipeSizeKernelParam, Value: "2097152"} data := []testData{ - {debug: false, trace: false, containerPipeSize: 0, expectedParams: []Param{}}, + {false, false, 0, []Param{}}, // Debug - {debug: true, trace: false, containerPipeSize: 0, expectedParams: []Param{debugParam}}, + {true, false, 0, []Param{debugParam}}, // Tracing - {debug: false, trace: true, containerPipeSize: 0, expectedParams: []Param{traceParam}}, + {false, true, 0, []Param{traceParam}}, // Debug + Tracing - {debug: true, trace: true, containerPipeSize: 0, expectedParams: []Param{debugParam, traceParam}}, + {true, true, 0, []Param{debugParam, traceParam}}, // pipesize - {debug: false, trace: false, containerPipeSize: 2097152, expectedParams: []Param{containerPipeSizeParam}}, + {false, false, 0, []Param{containerPipeSizeParam}}, // Debug + pipesize - {debug: true, trace: false, containerPipeSize: 2097152, expectedParams: []Param{debugParam, containerPipeSizeParam}}, + {true, false, 0, []Param{debugParam, containerPipeSizeParam}}, // Tracing + pipesize - {debug: false, trace: true, containerPipeSize: 2097152, expectedParams: []Param{traceParam, containerPipeSizeParam}}, + {false, true, 0, []Param{traceParam, containerPipeSizeParam}}, // Debug + Tracing + pipesize - {debug: true, trace: true, containerPipeSize: 2097152, expectedParams: []Param{debugParam, traceParam, containerPipeSizeParam}}, + {true, true, 0, []Param{debugParam, traceParam, containerPipeSizeParam}}, } for i, d := range data { diff --git a/src/runtime/virtcontainers/monitor.go b/src/runtime/virtcontainers/monitor.go index 75e06fc8f4..ae7843beae 100644 --- a/src/runtime/virtcontainers/monitor.go +++ b/src/runtime/virtcontainers/monitor.go @@ -22,12 +22,15 @@ var monitorLog = virtLog.WithField("subsystem", "virtcontainers/monitor") // nolint: govet type monitor struct { - sandbox *Sandbox - stopCh chan bool - watchers []chan error - checkInterval time.Duration - wg sync.WaitGroup + watchers []chan error + sandbox *Sandbox + + wg sync.WaitGroup sync.Mutex + + stopCh chan bool + checkInterval time.Duration + running bool } diff --git a/src/runtime/virtcontainers/qemu.go b/src/runtime/virtcontainers/qemu.go index 06e35d3e39..514cc9ee3c 100644 --- a/src/runtime/virtcontainers/qemu.go +++ b/src/runtime/virtcontainers/qemu.go @@ -747,9 +747,8 @@ func (q *qemu) checkBpfEnabled() { // There is only 64kB of IO memory each root,switch port will consume 4k hence // only 16 ports possible. func (q *qemu) createPCIeTopology(qemuConfig *govmmQemu.Config, hypervisorConfig *HypervisorConfig) error { - // We do not need to do anything if we want to hotplug a VFIO to a - // pcie-pci-bridge, just return - if hypervisorConfig.HotPlugVFIO == hv.BridgePort { + // If no-port set just return no need to add PCIe Root Port or PCIe Switches + if hypervisorConfig.HotPlugVFIO == hv.NoPort && hypervisorConfig.ColdPlugVFIO == hv.NoPort { return nil } // Add PCIe Root Port or PCIe Switches to the hypervisor @@ -772,37 +771,27 @@ func (q *qemu) createPCIeTopology(qemuConfig *govmmQemu.Config, hypervisorConfig qemuConfig.FwCfg = append(qemuConfig.FwCfg, fwCfg) } - // Get the number of hotpluggable ports needed from the provided devices - var numOfHotPluggablePorts uint32 = 0 - for _, dev := range hypervisorConfig.RawDevices { - hostPath, _ := config.GetHostPath(dev, false, "") - if hostPath == "" { - continue - } - - vfioNumberOrContainer := filepath.Base(hostPath) - // If we want to have VFIO inside of the VM we need to passthrough - // additionally /dev/vfio/vfio which is the VFIO "container". - // Ignore it and handle the remaining devices. - if strings.Compare(vfioNumberOrContainer, "vfio") == 0 { - continue - } - iommuDevicesPath := filepath.Join(config.SysIOMMUGroupPath, vfioNumberOrContainer, "devices") - deviceFiles, err := os.ReadDir(iommuDevicesPath) + // Get the number of hot(cold)-pluggable ports needed from the provided devices + var numOfPluggablePorts uint32 = 0 + for _, dev := range hypervisorConfig.VFIODevices { + var err error + dev.HostPath, err = config.GetHostPath(dev, false, "") if err != nil { - return err + return fmt.Errorf("Cannot get host path for device: %v err: %v", dev, err) } - - for _, deviceFile := range deviceFiles { - deviceBDF, _, _, err := drivers.GetVFIODetails(deviceFile.Name(), iommuDevicesPath) - if err != nil { - return err - } - if drivers.IsPCIeDevice(deviceBDF) { - numOfHotPluggablePorts = numOfHotPluggablePorts + 1 + devicesPerIOMMUGroup, err := drivers.GetAllVFIODevicesFromIOMMUGroup(dev) + if err != nil { + return fmt.Errorf("Cannot get all VFIO devices from IOMMU group with device: %v err: %v", dev, err) + } + for _, vfioDevice := range devicesPerIOMMUGroup { + if drivers.IsPCIeDevice(vfioDevice.BDF) { + numOfPluggablePorts = numOfPluggablePorts + 1 } + // Reset the Rank, the vfio module is going to assign the correct one + vfioDevice.Rank = -1 } } + drivers.AllPCIeDevs = make(map[string]bool) // If number of PCIe root ports > 16 then bail out otherwise we may // use up all slots or IO memory on the root bus and vfio-XXX-pci devices @@ -817,20 +806,20 @@ func (q *qemu) createPCIeTopology(qemuConfig *govmmQemu.Config, hypervisorConfig // If the user provided more root ports than we have detected // use the user provided number of PCIe root ports - if numOfHotPluggablePorts < hypervisorConfig.PCIeRootPort { - numOfHotPluggablePorts = hypervisorConfig.PCIeRootPort + if numOfPluggablePorts < hypervisorConfig.PCIeRootPort { + numOfPluggablePorts = hypervisorConfig.PCIeRootPort } // If the user provided more switch ports than we have detected // use the user provided number of PCIe root ports - if numOfHotPluggablePorts < hypervisorConfig.PCIeSwitchPort { - numOfHotPluggablePorts = hypervisorConfig.PCIeSwitchPort + if numOfPluggablePorts < hypervisorConfig.PCIeSwitchPort { + numOfPluggablePorts = hypervisorConfig.PCIeSwitchPort } - if q.state.HotPlugVFIO == hv.RootPort || q.state.HotplugVFIOOnRootBus { - qemuConfig.Devices = q.arch.appendPCIeRootPortDevice(qemuConfig.Devices, numOfHotPluggablePorts, memSize32bit, memSize64bit) + if q.state.HotPlugVFIO == hv.RootPort || q.state.ColdPlugVFIO == hv.RootPort || q.state.HotplugVFIOOnRootBus { + qemuConfig.Devices = q.arch.appendPCIeRootPortDevice(qemuConfig.Devices, numOfPluggablePorts, memSize32bit, memSize64bit) } - if q.state.HotPlugVFIO == hv.SwitchPort { - qemuConfig.Devices = q.arch.appendPCIeSwitchPortDevice(qemuConfig.Devices, numOfHotPluggablePorts, memSize32bit, memSize64bit) + if q.state.HotPlugVFIO == hv.SwitchPort || q.state.ColdPlugVFIO == hv.SwitchPort { + qemuConfig.Devices = q.arch.appendPCIeSwitchPortDevice(qemuConfig.Devices, numOfPluggablePorts, memSize32bit, memSize64bit) } return nil } @@ -1847,6 +1836,7 @@ func (q *qemu) hotplugVFIODeviceSwitchPort(ctx context.Context, device *config.V return fmt.Errorf("VFIO device is a PCIe device. Hotplug (%v) only supported on PCIe Root (%d) or PCIe Switch Ports (%v)", q.state.HotPlugVFIO, q.state.PCIeRootPort, q.state.PCIeSwitchPort) } + device.Bus = fmt.Sprintf("%s%d", pcieSwitchDownstreamPortPrefix, device.Rank) return q.executeVFIODeviceAdd(device) } @@ -1897,11 +1887,10 @@ func (q *qemu) hotplugVFIODevice(ctx context.Context, device *config.VFIODev, op } if op == AddDevice { - buf, _ := json.Marshal(device) q.Logger().WithFields(logrus.Fields{ "machine-type": q.HypervisorConfig().HypervisorMachineType, - "hotplug-vfio": q.state.HotPlugVFIO, + "hot-plug-vfio": q.state.HotPlugVFIO, "pcie-root-port": q.state.PCIeRootPort, "pcie-switch-port": q.state.PCIeSwitchPort, "device-info": string(buf), diff --git a/src/runtime/virtcontainers/sandbox.go b/src/runtime/virtcontainers/sandbox.go index bda931b435..a5e9ca4a5f 100644 --- a/src/runtime/virtcontainers/sandbox.go +++ b/src/runtime/virtcontainers/sandbox.go @@ -613,22 +613,26 @@ func newSandbox(ctx context.Context, sandboxConfig SandboxConfig, factory Factor if err := validateHypervisorConfig(&sandboxConfig.HypervisorConfig); err != nil { return nil, err } - // Aggregate all the container devices and update the HV config - var devices []config.DeviceInfo - for _, ct := range sandboxConfig.Containers { - devices = append(devices, ct.DeviceInfos...) - } - sandboxConfig.HypervisorConfig.RawDevices = devices // If we have a confidential guest we need to cold-plug the PCIe VFIO devices // until we have TDISP/IDE PCIe support. coldPlugVFIO := (sandboxConfig.HypervisorConfig.ColdPlugVFIO != hv.NoPort) - var devs []config.DeviceInfo + // Aggregate all the containner devices for hot-plug and use them to dedcue + // the correct amount of ports to reserve for the hypervisor. + hotPlugVFIO := (sandboxConfig.HypervisorConfig.HotPlugVFIO != hv.NoPort) + + var vfioHotPlugDevices []config.DeviceInfo + var vfioColdPlugDevices []config.DeviceInfo + for cnt, containers := range sandboxConfig.Containers { for dev, device := range containers.DeviceInfos { - if coldPlugVFIO && deviceManager.IsVFIO(device.ContainerPath) { + isVFIO := deviceManager.IsVFIO(device.ContainerPath) + if hotPlugVFIO && isVFIO { + vfioHotPlugDevices = append(vfioHotPlugDevices, device) + } + if coldPlugVFIO && isVFIO { device.ColdPlug = true - devs = append(devs, device) + vfioColdPlugDevices = append(vfioColdPlugDevices, device) // We need to remove the devices marked for cold-plug // otherwise at the container level the kata-agent // will try to hot-plug them. @@ -638,6 +642,7 @@ func newSandbox(ctx context.Context, sandboxConfig SandboxConfig, factory Factor } } } + sandboxConfig.HypervisorConfig.VFIODevices = vfioHotPlugDevices // store doesn't require hypervisor to be stored immediately if err = s.hypervisor.CreateVM(ctx, s.id, s.network, &sandboxConfig.HypervisorConfig); err != nil { @@ -652,7 +657,7 @@ func newSandbox(ctx context.Context, sandboxConfig SandboxConfig, factory Factor return s, nil } - for _, dev := range devs { + for _, dev := range vfioColdPlugDevices { _, err := s.AddDevice(ctx, dev) if err != nil { s.Logger().WithError(err).Debug("Cannot cold-plug add device") diff --git a/src/runtime/virtcontainers/sandbox_test.go b/src/runtime/virtcontainers/sandbox_test.go index db1aa7f84e..90a2af7ee3 100644 --- a/src/runtime/virtcontainers/sandbox_test.go +++ b/src/runtime/virtcontainers/sandbox_test.go @@ -650,8 +650,8 @@ func TestSandboxCreateAssets(t *testing.T) { // nolint: govet type testData struct { - annotations map[string]string assetType types.AssetType + annotations map[string]string } tmpfile, err := os.CreateTemp("", "virtcontainers-test-") @@ -687,50 +687,50 @@ func TestSandboxCreateAssets(t *testing.T) { data := []testData{ { - assetType: types.FirmwareAsset, - annotations: map[string]string{ + types.FirmwareAsset, + map[string]string{ annotations.FirmwarePath: filename, annotations.FirmwareHash: assetContentHash, }, }, { - assetType: types.HypervisorAsset, - annotations: map[string]string{ + types.HypervisorAsset, + map[string]string{ annotations.HypervisorPath: filename, annotations.HypervisorHash: assetContentHash, }, }, { - assetType: types.HypervisorCtlAsset, - annotations: map[string]string{ + types.HypervisorCtlAsset, + map[string]string{ annotations.HypervisorCtlPath: filename, annotations.HypervisorCtlHash: assetContentHash, }, }, { - assetType: types.ImageAsset, - annotations: map[string]string{ + types.ImageAsset, + map[string]string{ annotations.ImagePath: filename, annotations.ImageHash: assetContentHash, }, }, { - assetType: types.InitrdAsset, - annotations: map[string]string{ + types.InitrdAsset, + map[string]string{ annotations.InitrdPath: filename, annotations.InitrdHash: assetContentHash, }, }, { - assetType: types.JailerAsset, - annotations: map[string]string{ + types.JailerAsset, + map[string]string{ annotations.JailerPath: filename, annotations.JailerHash: assetContentHash, }, }, { - assetType: types.KernelAsset, - annotations: map[string]string{ + types.KernelAsset, + map[string]string{ annotations.KernelPath: filename, annotations.KernelHash: assetContentHash, }, @@ -1407,58 +1407,58 @@ func TestSandbox_Cgroups(t *testing.T) { // nolint: govet tests := []struct { - s *Sandbox name string + s *Sandbox wantErr bool needRoot bool }{ { - name: "New sandbox", - s: &Sandbox{}, - wantErr: true, - needRoot: false, + "New sandbox", + &Sandbox{}, + true, + false, }, { - name: "New sandbox, new config", - s: &Sandbox{config: &SandboxConfig{}}, - wantErr: false, - needRoot: true, + "New sandbox, new config", + &Sandbox{config: &SandboxConfig{}}, + false, + true, }, { - name: "sandbox, container no sandbox type", - s: &Sandbox{ + "sandbox, container no sandbox type", + &Sandbox{ config: &SandboxConfig{Containers: []ContainerConfig{ {}, }}}, - wantErr: false, - needRoot: true, + false, + true, }, { - name: "sandbox, container sandbox type", - s: &Sandbox{ + "sandbox, container sandbox type", + &Sandbox{ config: &SandboxConfig{Containers: []ContainerConfig{ sandboxContainer, }}}, - wantErr: false, - needRoot: true, + false, + true, }, { - name: "sandbox, empty linux json", - s: &Sandbox{ + "sandbox, empty linux json", + &Sandbox{ config: &SandboxConfig{Containers: []ContainerConfig{ emptyJSONLinux, }}}, - wantErr: false, - needRoot: true, + false, + true, }, { - name: "sandbox, successful config", - s: &Sandbox{ + "sandbox, successful config", + &Sandbox{ config: &SandboxConfig{Containers: []ContainerConfig{ successfulContainer, }}}, - wantErr: false, - needRoot: true, + false, + true, }, } for _, tt := range tests { diff --git a/src/runtime/virtcontainers/veth_endpoint_test.go b/src/runtime/virtcontainers/veth_endpoint_test.go index 341185b006..dc6f03270a 100644 --- a/src/runtime/virtcontainers/veth_endpoint_test.go +++ b/src/runtime/virtcontainers/veth_endpoint_test.go @@ -85,16 +85,16 @@ func TestCreateVethNetworkEndpointChooseIfaceName(t *testing.T) { func TestCreateVethNetworkEndpointInvalidArgs(t *testing.T) { // nolint: govet type endpointValues struct { - ifName string idx int + ifName string } assert := assert.New(t) // all elements are expected to result in failure failingValues := []endpointValues{ - {idx: -1, ifName: "bar"}, - {idx: -1, ifName: ""}, + {-1, "bar"}, + {-1, ""}, } for _, d := range failingValues { diff --git a/src/runtime/virtcontainers/virtiofsd_test.go b/src/runtime/virtcontainers/virtiofsd_test.go index 55eb3fb1a3..a4252a2ba9 100644 --- a/src/runtime/virtcontainers/virtiofsd_test.go +++ b/src/runtime/virtcontainers/virtiofsd_test.go @@ -17,13 +17,13 @@ import ( func TestVirtiofsdStart(t *testing.T) { // nolint: govet type fields struct { - ctx context.Context + path string socketPath string cache string - sourcePath string - path string extraArgs []string + sourcePath string PID int + ctx context.Context } sourcePath := t.TempDir() @@ -41,13 +41,13 @@ func TestVirtiofsdStart(t *testing.T) { // nolint: govet tests := []struct { - fields fields name string + fields fields wantErr bool }{ - {name: "empty config", fields: fields{}, wantErr: true}, - {name: "Directory socket does not exist", fields: NoDirectorySocket, wantErr: true}, - {name: "valid config", fields: validConfig, wantErr: false}, + {"empty config", fields{}, true}, + {"Directory socket does not exist", NoDirectorySocket, true}, + {"valid config", validConfig, false}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/tools/osbuilder/image-builder/image_builder.sh b/tools/osbuilder/image-builder/image_builder.sh index f88f3aac04..3e7f0babc0 100755 --- a/tools/osbuilder/image-builder/image_builder.sh +++ b/tools/osbuilder/image-builder/image_builder.sh @@ -271,7 +271,7 @@ calculate_required_disk_size() { readonly image="$(mktemp)" readonly mount_dir="$(mktemp -d)" readonly max_tries=20 - readonly increment=100 + readonly increment=10 for i in $(seq 1 $max_tries); do local img_size="$((rootfs_size_mb + (i * increment)))" diff --git a/tools/packaging/scripts/configure-hypervisor.sh b/tools/packaging/scripts/configure-hypervisor.sh index 9dfc9bb321..8bdfc94de1 100755 --- a/tools/packaging/scripts/configure-hypervisor.sh +++ b/tools/packaging/scripts/configure-hypervisor.sh @@ -1,4 +1,4 @@ -#!/usr/bin/env bash +#!/usr/bin/env bash # # Copyright (c) 2018 Intel Corporation # @@ -14,8 +14,6 @@ # been specified. #--------------------------------------------------------------------- -set -x - script_name=${0##*/} arch="${3:-$(uname -m)}"