From 80bbdfed683f69c89e89db893a37a0d0fa174b85 Mon Sep 17 00:00:00 2001 From: Dominik De Zordo Date: Thu, 16 Mar 2023 16:44:07 +0100 Subject: [PATCH 1/4] fix: Preserve kernel argument ordering Signed-off-by: Dominik De Zordo --- kernelargs.go | 64 ++++++++++++++++++++++++++++++++++++++------------- machine.go | 4 ++-- 2 files changed, 50 insertions(+), 18 deletions(-) diff --git a/kernelargs.go b/kernelargs.go index fba88c70..6236249b 100644 --- a/kernelargs.go +++ b/kernelargs.go @@ -13,7 +13,11 @@ package firecracker -import "strings" +import ( + "fmt" + "sort" + "strings" +) // kernelArgs serializes+deserializes kernel boot parameters from/into a map. // Kernel docs: https://www.kernel.org/doc/Documentation/admin-guide/kernel-parameters.txt @@ -21,38 +25,66 @@ import "strings" // "key=value" will result in map["key"] = &"value" // "key=" will result in map["key"] = &"" // "key" will result in map["key"] = nil -type kernelArgs map[string]*string +type kernelArg struct { + position uint + key string + value *string +} + +func (karg kernelArg) String() string { + if karg.value != nil { + return fmt.Sprintf("%s=%s", karg.key, *karg.value) + } + return karg.key +} -// serialize the kernelArgs back to a string that can be provided +type kernelArgs map[string]kernelArg + +// serialize the sorted kernelArgs back to a string that can be provided // to the kernel func (kargs kernelArgs) String() string { - var fields []string - for key, value := range kargs { - field := key - if value != nil { - field += "=" + *value - } - fields = append(fields, field) + sortedArgs := make([]kernelArg, 0) + for _, arg := range kargs { + sortedArgs = append(sortedArgs, arg) + } + sort.SliceStable(sortedArgs, func(i, j int) bool { + return sortedArgs[i].position < sortedArgs[j].position + }) + + args := make([]string, 0) + for _, arg := range sortedArgs { + args = append(args, arg.String()) + } + return strings.Join(args, " ") +} + +func (kargs kernelArgs) Add(key string, value *string) { + kargs[key] = kernelArg{ + position: uint(len(kargs)), + key: key, + value: value, } - return strings.Join(fields, " ") } // deserialize the provided string to a kernelArgs map func parseKernelArgs(rawString string) kernelArgs { - argMap := make(map[string]*string) - for _, kv := range strings.Fields(rawString) { + args := make(map[string]kernelArg) + for index, kv := range strings.Fields(rawString) { // only split into up to 2 fields (before and after the first "=") kvSplit := strings.SplitN(kv, "=", 2) key := kvSplit[0] - var value *string if len(kvSplit) == 2 { value = &kvSplit[1] } - argMap[key] = value + args[key] = kernelArg{ + position: uint(index), + key: key, + value: value, + } } - return argMap + return args } diff --git a/machine.go b/machine.go index 56fa3e25..4d7c2ff7 100644 --- a/machine.go +++ b/machine.go @@ -504,7 +504,7 @@ func (m *Machine) setupKernelArgs(ctx context.Context) error { // Validation that we are not overriding an existing "ip=" setting happens in the network validation if staticIPInterface := m.Cfg.NetworkInterfaces.staticIPInterface(); staticIPInterface != nil { ipBootParam := staticIPInterface.StaticConfiguration.IPConfiguration.ipBootParam() - kernelArgs["ip"] = &ipBootParam + kernelArgs.Add("ip", &ipBootParam) } m.Cfg.KernelArgs = kernelArgs.String() @@ -649,7 +649,7 @@ func (m *Machine) startVMM(ctx context.Context) error { return nil } -//StopVMM stops the current VMM. +// StopVMM stops the current VMM. func (m *Machine) StopVMM() error { return m.stopVMM() } From fdbd6ef771bef13121a2898b97b088a777440dd5 Mon Sep 17 00:00:00 2001 From: Dominik De Zordo Date: Thu, 16 Mar 2023 16:52:44 +0100 Subject: [PATCH 2/4] fix: Kernel parsing test Signed-off-by: Dominik De Zordo --- kernelargs_test.go | 37 +++++++++++++++++++++++++++++-------- 1 file changed, 29 insertions(+), 8 deletions(-) diff --git a/kernelargs_test.go b/kernelargs_test.go index b6e0c21a..19cd2a81 100644 --- a/kernelargs_test.go +++ b/kernelargs_test.go @@ -34,14 +34,35 @@ func TestKernelArgsSerder(t *testing.T) { booVal, ) - expectedParsedArgs := kernelArgs(map[string]*string{ - "foo": &fooVal, - "doo": &dooVal, - "blah": nil, - "huh": &emptyVal, - "bleh": nil, - "duh": &emptyVal, - "boo": &booVal, + expectedParsedArgs := kernelArgs(map[string]kernelArg{ + "foo": { + position: 0, + value: &fooVal, + }, + "blah": { + position: 1, + value: nil, + }, + "doo": { + position: 2, + value: &dooVal, + }, + "huh": { + position: 3, + value: &emptyVal, + }, + "bleh": { + position: 4, + value: nil, + }, + "duh": { + position: 5, + value: &emptyVal, + }, + "boo": { + position: 6, + value: &booVal, + }, }) actualParsedArgs := parseKernelArgs(argsString) From 3da71c9af4b51dea0f1c08a7af29f13af79fe467 Mon Sep 17 00:00:00 2001 From: Dominik De Zordo Date: Thu, 16 Mar 2023 16:54:02 +0100 Subject: [PATCH 3/4] fix: Also add key to the test Signed-off-by: Dominik De Zordo --- kernelargs_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/kernelargs_test.go b/kernelargs_test.go index 19cd2a81..068d0c57 100644 --- a/kernelargs_test.go +++ b/kernelargs_test.go @@ -37,30 +37,37 @@ func TestKernelArgsSerder(t *testing.T) { expectedParsedArgs := kernelArgs(map[string]kernelArg{ "foo": { position: 0, + key: "foo", value: &fooVal, }, "blah": { position: 1, + key: "blah", value: nil, }, "doo": { position: 2, + key: "doo", value: &dooVal, }, "huh": { position: 3, + key: "huh", value: &emptyVal, }, "bleh": { position: 4, + key: "bleh", value: nil, }, "duh": { position: 5, + key: "duh", value: &emptyVal, }, "boo": { position: 6, + key: "boo", value: &booVal, }, }) From 4f469e12205c461eb3516c8ebe33b127868ccf1a Mon Sep 17 00:00:00 2001 From: Dominik De Zordo Date: Thu, 16 Mar 2023 19:09:48 +0100 Subject: [PATCH 4/4] chore: Updated documentation Signed-off-by: Dominik De Zordo --- kernelargs.go | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/kernelargs.go b/kernelargs.go index 6236249b..c2fab768 100644 --- a/kernelargs.go +++ b/kernelargs.go @@ -19,12 +19,10 @@ import ( "strings" ) -// kernelArgs serializes+deserializes kernel boot parameters from/into a map. -// Kernel docs: https://www.kernel.org/doc/Documentation/admin-guide/kernel-parameters.txt -// -// "key=value" will result in map["key"] = &"value" -// "key=" will result in map["key"] = &"" -// "key" will result in map["key"] = nil +// kernelArg represents a key and optional value pair for passing an argument +// into the kernel. Additionally, it also saves the position of the argument +// in the whole command line input. This is important because the kernel stops reading +// everything after `--` and passes these keys into the init process type kernelArg struct { position uint key string @@ -38,10 +36,17 @@ func (karg kernelArg) String() string { return karg.key } +// kernelArgs serializes + deserializes kernel boot parameters from/into a map. +// Kernel docs: https://www.kernel.org/doc/Documentation/admin-guide/kernel-parameters.txt +// +// "key=value flag emptykey=" will be converted to +// map["key"] = { position: 0, key: "key", value: &"value" } +// map["flag"] = { position: 1, key: "flag", value: nil } +// map["emptykey"] = { position: 2, key: "emptykey", value: &"" } type kernelArgs map[string]kernelArg -// serialize the sorted kernelArgs back to a string that can be provided -// to the kernel +// Sorts the arguments by its position +// and serializes the map back into a single string func (kargs kernelArgs) String() string { sortedArgs := make([]kernelArg, 0) for _, arg := range kargs { @@ -58,6 +63,7 @@ func (kargs kernelArgs) String() string { return strings.Join(args, " ") } +// Add a new kernel argument to the kernelArgs, also the position is saved func (kargs kernelArgs) Add(key string, value *string) { kargs[key] = kernelArg{ position: uint(len(kargs)), @@ -66,7 +72,8 @@ func (kargs kernelArgs) Add(key string, value *string) { } } -// deserialize the provided string to a kernelArgs map +// Parses an input string and deserializes it into a map +// saving its position in the command line func parseKernelArgs(rawString string) kernelArgs { args := make(map[string]kernelArg) for index, kv := range strings.Fields(rawString) {