config: Allow user-specified kernel params to take priority

Any `kernel_params=` values specified in the config file should take
priority over any in-built defaults (for better or for worse).

Fixes clearcontainers#963.

Signed-off-by: James O. D. Hunt <[email protected]>

config/configuration.toml.in

@@ -1,4 +1,4 @@
-# XXX: Warning: this file is auto-generated.
+# XXX: WARNING: this file is auto-generated.
 # XXX:
 # XXX: Source file: "@CONFIG_IN@"
 # XXX: Project:
@@ -10,9 +10,17 @@ path = "@QEMUPATH@"
 kernel = "@KERNELPATH@"
 image = "@IMAGEPATH@"
 machine_type = "@MACHINETYPE@"
+
 # Optional space-separated list of options to pass to the guest kernel.
 # For example, use `kernel_params = "vsyscall=emulate"` if you are having
-# trouble running pre-2.15 glibc
+# trouble running pre-2.15 glibc.
+#
+# WARNING: - any parameter specified here will take priority over the default
+# parameter value of the same name used to start the virtual machine.
+# Do not set values here unless you understand the impact of doing so as you
+# may stop the virtual machine from booting.
+# To see the list of default parameters, enable hypervisor debug, create a
+# container and look for 'default-kernel-parameters' log entries.
 kernel_params = "@KERNELPARAMS@"
 
 # Path to the firmware.

create.go

@@ -177,14 +177,47 @@ func getKernelParams(containerID string) []vc.Param {
 	}
 }
 
+// setKernelParams adds the user-specified kernel parameters (from the
+// configuration file) to the defaults so that the former take priority.
+func setKernelParams(containerID string, runtimeConfig *oci.RuntimeConfig) error {
+	defaultKernelParams := getKernelParamsFunc(containerID)
+
+	if runtimeConfig.HypervisorConfig.Debug {
+		strParams := vc.SerializeParams(defaultKernelParams, "=")
+		formatted := strings.Join(strParams, " ")
+
+		ccLog.WithField("default-kernel-parameters", formatted).Debug()
+	}
+
+	// retrieve the parameters specified in the config file
+	userKernelParams := runtimeConfig.HypervisorConfig.KernelParams
+
+	// reset
+	runtimeConfig.HypervisorConfig.KernelParams = []vc.Param{}
+
+	// first, add default values
+	for _, p := range defaultKernelParams {
+		if err := (runtimeConfig).AddKernelParam(p); err != nil {
+			return err
+		}
+	}
+
+	// now re-add the user-specified values so that they take priority.
+	for _, p := range userKernelParams {
+		if err := (runtimeConfig).AddKernelParam(p); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
 func createPod(ociSpec oci.CompatOCISpec, runtimeConfig oci.RuntimeConfig,
 	containerID, bundlePath, console string, disableOutput bool) (vc.Process, error) {
-	ccKernelParams := getKernelParamsFunc(containerID)
 
-	for _, p := range ccKernelParams {
-		if err := (&runtimeConfig).AddKernelParam(p); err != nil {
-			return vc.Process{}, err
-		}
+	err := setKernelParams(containerID, &runtimeConfig)
+	if err != nil {
+		return vc.Process{}, err
 	}
 
 	podConfig, err := oci.PodConfig(ociSpec, runtimeConfig, bundlePath, containerID, console, disableOutput)

create_test.go

@@ -15,6 +15,7 @@
 package main
 
 import (
+	"errors"
 	"flag"
 	"fmt"
 	"io/ioutil"
@@ -58,6 +59,28 @@ func testCreateCgroupsFilesSuccessful(t *testing.T, cgroupsDirPath string, cgrou
 	}
 }
 
+// return the value of the *last* param with the specified key
+func findLastParam(key string, params []vc.Param) (string, error) {
+	if key == "" {
+		return "", errors.New("ERROR: need non-nil key")
+	}
+
+	l := len(params)
+	if l == 0 {
+		return "", errors.New("ERROR: no params")
+	}
+
+	for i := l - 1; i >= 0; i-- {
+		p := params[i]
+
+		if key == p.Key {
+			return p.Value, nil
+		}
+	}
+
+	return "", fmt.Errorf("no param called %q found", name)
+}
+
 func TestCgroupsFilesEmptyCgroupsPathSuccessful(t *testing.T) {
 	testCreateCgroupsFilesSuccessful(t, "", []string{}, testPID)
 }
@@ -1119,3 +1142,56 @@ func TestCopyParentCPUSetSuccessful(t *testing.T) {
 	assert.False(isEmptyString(currentCpus))
 	assert.False(isEmptyString(currentMems))
 }
+
+func TestSetKernelParams(t *testing.T) {
+	assert := assert.New(t)
+
+	config := oci.RuntimeConfig{}
+
+	assert.Empty(config.HypervisorConfig.KernelParams)
+
+	err := setKernelParams(testContainerID, &config)
+	assert.NoError(err)
+
+	assert.NotEmpty(config.HypervisorConfig.KernelParams)
+}
+
+func TestSetKernelParamsUserOptionTakesPriority(t *testing.T) {
+	assert := assert.New(t)
+
+	initName := "init"
+	initValue := "/sbin/myinit"
+
+	ipName := "ip"
+	ipValue := "127.0.0.1"
+
+	params := []vc.Param{
+		{Key: initName, Value: initValue},
+		{Key: ipName, Value: ipValue},
+	}
+
+	hypervisorConfig := vc.HypervisorConfig{
+		KernelParams: params,
+	}
+
+	// Config containing user-specified kernel parameters
+	config := oci.RuntimeConfig{
+		HypervisorConfig: hypervisorConfig,
+	}
+
+	assert.NotEmpty(config.HypervisorConfig.KernelParams)
+
+	err := setKernelParams(testContainerID, &config)
+	assert.NoError(err)
+
+	kernelParams := config.HypervisorConfig.KernelParams
+
+	init, err := findLastParam(initName, kernelParams)
+	assert.NoError(err)
+	assert.Equal(initValue, init)
+
+	ip, err := findLastParam(ipName, kernelParams)
+	assert.NoError(err)
+	assert.Equal(ipValue, ip)
+
+}