From 09c5bbd2dccb96ee2c26f188a937bb5ac41eb0bf Mon Sep 17 00:00:00 2001 From: Archana Shinde Date: Thu, 29 Mar 2018 10:44:38 -0700 Subject: [PATCH 1/3] vendor: Vendor github.com/intel/govmm Vendor package for pulling in changes related to support for iothreads with virtio-scsi. Shortlog for govmm: 9130f37 scsi: Allow scsi controller to associate with an IO thread. a54de18 iothread: Add ability to configure iothreads Signed-off-by: Archana Shinde --- Gopkg.lock | 4 ++-- Gopkg.toml | 2 +- vendor/github.com/intel/govmm/qemu/qemu.go | 23 ++++++++++++++++++++++ 3 files changed, 26 insertions(+), 3 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index d111131745..a2ef67389c 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -86,7 +86,7 @@ [[projects]] name = "github.com/intel/govmm" packages = ["qemu"] - revision = "82c67ab9b21e8cd0042b6c2d3be2d3705a511603" + revision = "1509acf1862ae5154c5c096f9318bd3eb434d816" [[projects]] name = "github.com/kata-containers/agent" @@ -250,6 +250,6 @@ [solve-meta] analyzer-name = "dep" analyzer-version = 1 - inputs-digest = "bb2ac1696f8e90526e492a6d54362549094fa844c17fbd06c727198bff6667ef" + inputs-digest = "f7c3a1b7f5cb5f891a3badcb7323f3b5fc0fa79f69dd6532ec2e2be2baafaf98" solver-name = "gps-cdcl" solver-version = 1 diff --git a/Gopkg.toml b/Gopkg.toml index 81c3705509..67f31bead2 100644 --- a/Gopkg.toml +++ b/Gopkg.toml @@ -56,7 +56,7 @@ [[constraint]] name = "github.com/intel/govmm" - revision = "82c67ab9b21e8cd0042b6c2d3be2d3705a511603" + revision = "1509acf1862ae5154c5c096f9318bd3eb434d816" [[constraint]] name = "github.com/kata-containers/agent" diff --git a/vendor/github.com/intel/govmm/qemu/qemu.go b/vendor/github.com/intel/govmm/qemu/qemu.go index 89d053fe95..4b0e41795a 100644 --- a/vendor/github.com/intel/govmm/qemu/qemu.go +++ b/vendor/github.com/intel/govmm/qemu/qemu.go @@ -841,6 +841,9 @@ type SCSIController struct { // DisableModern prevents qemu from relying on fast MMIO. DisableModern bool + + // IOThread is the IO thread on which IO will be handled + IOThread string } // Valid returns true if the SCSIController structure is valid and complete. @@ -867,6 +870,9 @@ func (scsiCon SCSIController) QemuParams(config *Config) []string { if scsiCon.DisableModern { devParams = append(devParams, fmt.Sprintf("disable-modern=true")) } + if scsiCon.IOThread != "" { + devParams = append(devParams, fmt.Sprintf("iothread=%s", scsiCon.IOThread)) + } qemuParams = append(qemuParams, "-device") qemuParams = append(qemuParams, strings.Join(devParams, ",")) @@ -1159,6 +1165,11 @@ type Knobs struct { Realtime bool } +// IOThread allows IO to be performed on a separate thread. +type IOThread struct { + ID string +} + // Config is the qemu configuration structure. // It allows for passing custom settings and parameters to the qemu API. type Config struct { @@ -1213,6 +1224,8 @@ type Config struct { // fds is a list of open file descriptors to be passed to the spawned qemu process fds []*os.File + IOThreads []IOThread + qemuParams []string } @@ -1481,6 +1494,15 @@ func (config *Config) appendBios() { } } +func (config *Config) appendIOThreads() { + for _, t := range config.IOThreads { + if t.ID != "" { + config.qemuParams = append(config.qemuParams, "-object") + config.qemuParams = append(config.qemuParams, fmt.Sprintf("iothread,id=%s", t.ID)) + } + } +} + // LaunchQemu can be used to launch a new qemu instance. // // The Config parameter contains a set of qemu parameters and settings. @@ -1504,6 +1526,7 @@ func LaunchQemu(config Config, logger QMPLog) (string, error) { config.appendKnobs() config.appendKernel() config.appendBios() + config.appendIOThreads() if err := config.appendCPUs(); err != nil { return "", err From 82e42b5dc5726accf07dd04376e1d2ee1695c4f8 Mon Sep 17 00:00:00 2001 From: Archana Shinde Date: Thu, 29 Mar 2018 00:09:30 -0700 Subject: [PATCH 2/3] qemu: iothreads: Add iothread support for scsi Add a hypervisor configuration to specify if IO should be handled in a separate thread. Add support for iothreads for virtio-scsi for now. Since we attach all scsi drives to the same scsi controller, all the drives will be handled in a separate IO thread which would still give better performance. Going forward we need to assess if adding more controllers and attaching iothreasds to each of them with distributing drives among teh scsi controllers should be done, based on more performance analysis. Signed-off-by: Archana Shinde --- virtcontainers/hypervisor.go | 4 ++++ virtcontainers/qemu.go | 7 ++++++- virtcontainers/qemu_arch_base.go | 18 +++++++++++++++--- virtcontainers/qemu_arch_base_test.go | 6 +++++- 4 files changed, 30 insertions(+), 5 deletions(-) diff --git a/virtcontainers/hypervisor.go b/virtcontainers/hypervisor.go index cfbd885fae..b5d0d3d8ce 100644 --- a/virtcontainers/hypervisor.go +++ b/virtcontainers/hypervisor.go @@ -201,6 +201,10 @@ type HypervisorConfig struct { // DisableBlockDeviceUse disallows a block device from being used. DisableBlockDeviceUse bool + // EnableIOThreads enables IO to be processed in a separate thread. + // Supported currently for virtio-scsi driver. + EnableIOThreads bool + // Debug changes the default hypervisor and kernel parameters to // enable debug output where available. Debug bool diff --git a/virtcontainers/qemu.go b/virtcontainers/qemu.go index 93bcf2136e..1eca297402 100644 --- a/virtcontainers/qemu.go +++ b/virtcontainers/qemu.go @@ -379,8 +379,9 @@ func (q *qemu) createPod(podConfig PodConfig) error { return err } + var ioThread *govmmQemu.IOThread if q.config.BlockDeviceDriver == VirtioSCSI { - devices = q.arch.appendSCSIController(devices) + devices, ioThread = q.arch.appendSCSIController(devices, q.config.EnableIOThreads) } cpuModel := q.arch.cpuModel() @@ -414,6 +415,10 @@ func (q *qemu) createPod(podConfig PodConfig) error { Bios: firmwarePath, } + if ioThread != nil { + qemuConfig.IOThreads = []govmmQemu.IOThread{*ioThread} + } + q.qemuConfig = qemuConfig return nil diff --git a/virtcontainers/qemu_arch_base.go b/virtcontainers/qemu_arch_base.go index c377755a45..81fcf8a8eb 100644 --- a/virtcontainers/qemu_arch_base.go +++ b/virtcontainers/qemu_arch_base.go @@ -66,7 +66,7 @@ type qemuArch interface { appendImage(devices []govmmQemu.Device, path string) ([]govmmQemu.Device, error) // appendSCSIController appens a SCSI controller to devices - appendSCSIController(devices []govmmQemu.Device) []govmmQemu.Device + appendSCSIController(devices []govmmQemu.Device, enableIOThreads bool) ([]govmmQemu.Device, *govmmQemu.IOThread) // appendBridges appends bridges to devices appendBridges(devices []govmmQemu.Device, bridges []Bridge) []govmmQemu.Device @@ -300,15 +300,27 @@ func (q *qemuArchBase) appendImage(devices []govmmQemu.Device, path string) ([]g return q.appendBlockDevice(devices, drive), nil } -func (q *qemuArchBase) appendSCSIController(devices []govmmQemu.Device) []govmmQemu.Device { +func (q *qemuArchBase) appendSCSIController(devices []govmmQemu.Device, enableIOThreads bool) ([]govmmQemu.Device, *govmmQemu.IOThread) { scsiController := govmmQemu.SCSIController{ ID: scsiControllerID, DisableModern: q.nestedRun, } + var t *govmmQemu.IOThread + + if enableIOThreads { + randBytes, _ := generateRandomBytes(8) + + t = &govmmQemu.IOThread{ + ID: fmt.Sprintf("%s-%s", "iothread", hex.EncodeToString(randBytes)), + } + + scsiController.IOThread = t.ID + } + devices = append(devices, scsiController) - return devices + return devices, t } // appendBridges appends to devices the given bridges diff --git a/virtcontainers/qemu_arch_base_test.go b/virtcontainers/qemu_arch_base_test.go index a91a06a586..451e01a68c 100644 --- a/virtcontainers/qemu_arch_base_test.go +++ b/virtcontainers/qemu_arch_base_test.go @@ -470,6 +470,10 @@ func TestQemuArchBaseAppendSCSIController(t *testing.T) { }, } - devices = qemuArchBase.appendSCSIController(devices) + devices, ioThread := qemuArchBase.appendSCSIController(devices, false) assert.Equal(expectedOut, devices) + assert.Nil(ioThread) + + _, ioThread = qemuArchBase.appendSCSIController(devices, true) + assert.NotNil(ioThread) } From 204e40297ca5c675e13e74152d6e1ce330e3def3 Mon Sep 17 00:00:00 2001 From: Archana Shinde Date: Thu, 29 Mar 2018 00:15:40 -0700 Subject: [PATCH 3/3] cli: Add configuration option for io-threads. Add option to configure if IO needs to be in a separate IO thread. Add tests to verify option is correctly parsed. The default value is set to false for now. This should be considered to be enabled by default in the future. Fixes #132 Signed-off-by: Archana Shinde --- Makefile | 4 ++++ cli/config.go | 3 +++ cli/config/configuration.toml.in | 6 ++++++ cli/config_test.go | 13 +++++++++++-- cli/kata-env_test.go | 5 ++++- 5 files changed, 28 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 91f9421549..2f190c4a30 100644 --- a/Makefile +++ b/Makefile @@ -101,6 +101,7 @@ DEFNETWORKMODEL := macvtap DEFDISABLEBLOCK := false DEFBLOCKSTORAGEDRIVER := virtio-scsi +DEFENABLEIOTHREADS := false DEFENABLEMEMPREALLOC := false DEFENABLEHUGEPAGES := false DEFENABLESWAP := false @@ -172,6 +173,7 @@ USER_VARS += DEFBRIDGES USER_VARS += DEFNETWORKMODEL USER_VARS += DEFDISABLEBLOCK USER_VARS += DEFBLOCKSTORAGEDRIVER +USER_VARS += DEFENABLEIOTHREADS USER_VARS += DEFENABLEMEMPREALLOC USER_VARS += DEFENABLEHUGEPAGES USER_VARS += DEFENABLESWAP @@ -263,6 +265,7 @@ const defaultBridgesCount uint32 = $(DEFBRIDGES) const defaultInterNetworkingModel = "$(DEFNETWORKMODEL)" const defaultDisableBlockDeviceUse bool = $(DEFDISABLEBLOCK) const defaultBlockDeviceDriver = "$(DEFBLOCKSTORAGEDRIVER)" +const defaultEnableIOThreads bool = $(DEFENABLEIOTHREADS) const defaultEnableMemPrealloc bool = $(DEFENABLEMEMPREALLOC) const defaultEnableHugePages bool = $(DEFENABLEHUGEPAGES) const defaultEnableSwap bool = $(DEFENABLESWAP) @@ -346,6 +349,7 @@ $(GENERATED_FILES): %: %.in Makefile VERSION -e "s|@DEFNETWORKMODEL@|$(DEFNETWORKMODEL)|g" \ -e "s|@DEFDISABLEBLOCK@|$(DEFDISABLEBLOCK)|g" \ -e "s|@DEFBLOCKSTORAGEDRIVER@|$(DEFBLOCKSTORAGEDRIVER)|g" \ + -e "s|@DEFENABLEIOTHREADS@|$(DEFENABLEIOTHREADS)|g" \ -e "s|@DEFENABLEMEMPREALLOC@|$(DEFENABLEMEMPREALLOC)|g" \ -e "s|@DEFENABLEHUGEPAGES@|$(DEFENABLEHUGEPAGES)|g" \ -e "s|@DEFENABLEMSWAP@|$(DEFENABLESWAP)|g" \ diff --git a/cli/config.go b/cli/config.go index e22f857e8c..98629e152e 100644 --- a/cli/config.go +++ b/cli/config.go @@ -92,6 +92,7 @@ type hypervisor struct { Swap bool `toml:"enable_swap"` Debug bool `toml:"enable_debug"` DisableNestingChecks bool `toml:"disable_nesting_checks"` + EnableIOThreads bool `toml:"enable_iothreads"` } type proxy struct { @@ -321,6 +322,7 @@ func newQemuHypervisorConfig(h hypervisor) (vc.HypervisorConfig, error) { Debug: h.Debug, DisableNestingChecks: h.DisableNestingChecks, BlockDeviceDriver: blockDriver, + EnableIOThreads: h.EnableIOThreads, }, nil } @@ -423,6 +425,7 @@ func loadConfiguration(configPath string, ignoreLogging bool) (resolvedConfigPat Debug: defaultEnableDebug, DisableNestingChecks: defaultDisableNestingChecks, BlockDeviceDriver: defaultBlockDeviceDriver, + EnableIOThreads: defaultEnableIOThreads, } err = config.InterNetworkModel.SetModel(defaultInterNetworkingModel) diff --git a/cli/config/configuration.toml.in b/cli/config/configuration.toml.in index e3416adaa9..307d841f35 100644 --- a/cli/config/configuration.toml.in +++ b/cli/config/configuration.toml.in @@ -70,6 +70,12 @@ disable_block_device_use = @DEFDISABLEBLOCK@ # virtio-blk. block_device_driver = "@DEFBLOCKSTORAGEDRIVER@" +# Enable iothreads (data-plane) to be used. This causes IO to be +# handled in a separate IO thread. This is currently only implemented +# for SCSI. +# +enable_iothreads = @DEFENABLEIOTHREADS@ + # Enable pre allocation of VM RAM, default false # Enabling this will result in lower container density # as all of the memory will be allocated and locked diff --git a/cli/config_test.go b/cli/config_test.go index 54f358179b..6b3f7c5818 100644 --- a/cli/config_test.go +++ b/cli/config_test.go @@ -41,7 +41,7 @@ type testRuntimeConfig struct { LogPath string } -func makeRuntimeConfigFileData(hypervisor, hypervisorPath, kernelPath, imagePath, kernelParams, machineType, shimPath, proxyPath, logPath string, disableBlock bool, blockDeviceDriver string) string { +func makeRuntimeConfigFileData(hypervisor, hypervisorPath, kernelPath, imagePath, kernelParams, machineType, shimPath, proxyPath, logPath string, disableBlock bool, blockDeviceDriver string, enableIOThreads bool) string { return ` # Runtime configuration file @@ -55,6 +55,7 @@ func makeRuntimeConfigFileData(hypervisor, hypervisorPath, kernelPath, imagePath default_vcpus = ` + strconv.FormatUint(uint64(defaultVCPUCount), 10) + ` default_memory = ` + strconv.FormatUint(uint64(defaultMemSize), 10) + ` disable_block_device_use = ` + strconv.FormatBool(disableBlock) + ` + enable_iothreads = ` + strconv.FormatBool(enableIOThreads) + ` [proxy.kata] path = "` + proxyPath + `" @@ -101,8 +102,9 @@ func createAllRuntimeConfigFiles(dir, hypervisor string) (config testRuntimeConf machineType := "machineType" disableBlockDevice := true blockDeviceDriver := "virtio-scsi" + enableIOThreads := true - runtimeConfigFileData := makeRuntimeConfigFileData(hypervisor, hypervisorPath, kernelPath, imagePath, kernelParams, machineType, shimPath, proxyPath, logPath, disableBlockDevice, blockDeviceDriver) + runtimeConfigFileData := makeRuntimeConfigFileData(hypervisor, hypervisorPath, kernelPath, imagePath, kernelParams, machineType, shimPath, proxyPath, logPath, disableBlockDevice, blockDeviceDriver, enableIOThreads) configPath := path.Join(dir, "runtime.toml") err = createConfig(configPath, runtimeConfigFileData) @@ -140,6 +142,7 @@ func createAllRuntimeConfigFiles(dir, hypervisor string) (config testRuntimeConf BlockDeviceDriver: defaultBlockDeviceDriver, DefaultBridges: defaultBridgesCount, Mlock: !defaultEnableSwap, + EnableIOThreads: enableIOThreads, } agentConfig := vc.KataAgentConfig{} @@ -569,6 +572,7 @@ func TestNewQemuHypervisorConfig(t *testing.T) { imagePath := path.Join(dir, "image") machineType := "machineType" disableBlock := true + enableIOThreads := true hypervisor := hypervisor{ Path: hypervisorPath, @@ -576,6 +580,7 @@ func TestNewQemuHypervisorConfig(t *testing.T) { Image: imagePath, MachineType: machineType, DisableBlockDeviceUse: disableBlock, + EnableIOThreads: enableIOThreads, } files := []string{hypervisorPath, kernelPath, imagePath} @@ -617,6 +622,10 @@ func TestNewQemuHypervisorConfig(t *testing.T) { t.Errorf("Expected value for disable block usage %v, got %v", disableBlock, config.DisableBlockDeviceUse) } + if config.EnableIOThreads != enableIOThreads { + t.Errorf("Expected value for enable IOThreads %v, got %v", enableIOThreads, config.EnableIOThreads) + } + } func TestNewShimConfig(t *testing.T) { diff --git a/cli/kata-env_test.go b/cli/kata-env_test.go index 9393cb863c..ee21c0e3d0 100644 --- a/cli/kata-env_test.go +++ b/cli/kata-env_test.go @@ -69,6 +69,7 @@ func makeRuntimeConfig(prefixDir string) (configFile string, config oci.RuntimeC proxyPath := filepath.Join(prefixDir, "proxy") disableBlock := true blockStorageDriver := "virtio-scsi" + enableIOThreads := true // override defaultProxyPath = proxyPath @@ -112,7 +113,9 @@ func makeRuntimeConfig(prefixDir string) (configFile string, config oci.RuntimeC testProxyURL, logPath, disableBlock, - blockStorageDriver) + blockStorageDriver, + enableIOThreads, + ) configFile = path.Join(prefixDir, "runtime.toml") err = createConfig(configFile, runtimeConfig)