diff --git a/cluster-provision/centos9/vmcli/cmd/qemu/system.go b/cluster-provision/centos9/vmcli/cmd/qemu/system.go index 588dbb7b0b..a49d1979c7 100644 --- a/cluster-provision/centos9/vmcli/cmd/qemu/system.go +++ b/cluster-provision/centos9/vmcli/cmd/qemu/system.go @@ -2,6 +2,7 @@ package qemu import ( "fmt" + "regexp" "strconv" "strings" @@ -20,6 +21,8 @@ type QemuSystem struct { Memory string // The number of the CPUs in the SMP system, wraps the "-smp" argument CpuCount uint64 + // The number of NUMA nodes in the system, wraps "-object memory-backend-ram" and "-numa node" arguments + Numa uint64 // Whether to use KVM, wraps the "-enable-kvm" argument KvmEnabled bool // Select the CPU model, wraps the "-cpu" argument @@ -46,8 +49,67 @@ type QemuSystem struct { Netdev string } +// Parse the memory argument into a value and possibly a unit +func (q QemuSystem) parseMemory() (uint64, string, error) { + regex, err := regexp.Compile(`(\d+)(\w*)`) + if err != nil { + return 0, "", err + } + + submatch := regex.FindStringSubmatch(q.Memory) + if len(submatch) < 3 { + return 0, "", fmt.Errorf("Unable to parse the QEMU memory argument %q", q.Memory) + } + + val, err := strconv.ParseUint(submatch[1], 10, 64) + if err != nil { + return 0, "", err + } + + unit := "" + if len(submatch) >= 3 { + unit = submatch[2] + } + + return val, unit, nil +} + +// Generate the QEMU arguments for creating the NUMA topology +func (q QemuSystem) generateNumaArguments() ([]string, error) { + if q.Numa < 2 { + return []string{}, nil + } + + result := []string{} + + memoryValue, memoryUnit, err := q.parseMemory() + if err != nil { + return []string{}, err + } + + if q.CpuCount%q.Numa > 0 || memoryValue%q.Numa > 0 { + return []string{}, fmt.Errorf("unable to calculate symmetric NUMA topology with vCPUs:%v Memory:%v NUMA:%v", q.CpuCount, q.Memory, q.Numa) + } + + memoryPerNodeValue := memoryValue / q.Numa + memoryPerNode := fmt.Sprintf("%v%v", memoryPerNodeValue, memoryUnit) + cpuPerNode := q.CpuCount / q.Numa + + for nodeId := uint64(0); nodeId < q.Numa; nodeId++ { + nodeFirstCpu := nodeId * cpuPerNode + nodeLastCpu := nodeFirstCpu + cpuPerNode - 1 + + memoryBackendRamArg := fmt.Sprintf("-object memory-backend-ram,size=%v,id=m%v", memoryPerNode, nodeId) + numaNodeArg := fmt.Sprintf("-numa node,nodeid=%v,memdev=m%v,cpus=%v-%v", nodeId, nodeId, nodeFirstCpu, nodeLastCpu) + + result = append(result, memoryBackendRamArg, numaNodeArg) + } + + return result, nil +} + // Generate the command line to use to start QEMU -func (q QemuSystem) GenerateCmdline() string { +func (q QemuSystem) GenerateCmdline() (string, error) { qemuArgs := []string{ fmt.Sprintf(qemuExec, q.Arch), "-m", q.Memory, @@ -75,5 +137,14 @@ func (q QemuSystem) GenerateCmdline() string { qemuArgs = append(qemuArgs, "-device", device) } - return strings.Join(qemuArgs, " ") + numaArgs, err := q.generateNumaArguments() + if err != nil { + return "", err + } + + if len(numaArgs) > 0 { + qemuArgs = append(qemuArgs, numaArgs...) + } + + return strings.Join(qemuArgs, " "), nil } diff --git a/cluster-provision/centos9/vmcli/cmd/qemu/system_test.go b/cluster-provision/centos9/vmcli/cmd/qemu/system_test.go index 0b00eb0a24..894ecf6101 100644 --- a/cluster-provision/centos9/vmcli/cmd/qemu/system_test.go +++ b/cluster-provision/centos9/vmcli/cmd/qemu/system_test.go @@ -1,6 +1,7 @@ package qemu import ( + "reflect" "testing" "github.com/google/uuid" @@ -19,6 +20,7 @@ func TestGeneratedCommandLine(t *testing.T) { Arch: "x86_64", Memory: "2048M", CpuCount: 2, + Numa: 1, KvmEnabled: true, CpuModel: "host,migratable=no,+invtsc", Machine: "q35,accel=kvm,kernel_irqchip=split", @@ -42,8 +44,153 @@ func TestGeneratedCommandLine(t *testing.T) { Netdev: "tap,id=network0,ifname=tap01,script=no,downscript=no", } - cmdline := qemuSystem.GenerateCmdline() + cmdline, err := qemuSystem.GenerateCmdline() + if err != nil { + t.Fatalf("Unable to generate the QEMU command line: %q", err) + } + if cmdline != expectedCmdline { t.Fatalf("The generated command line is invalid.\nExpected: %q\nGot: %q", expectedCmdline, cmdline) } } + +// Test that the memory argument parser fails when the memory arg is invalid +func TestParseMemoryInvalid(t *testing.T) { + qemuSystem := QemuSystem{ + Memory: "invalid", + } + + _, _, err := qemuSystem.parseMemory() + if err == nil { + t.Fatal("No error was reported with an invalid input") + } +} + +// Test that the memory argument parser returns the value when the memory arg only +// contains the size +func TestParseMemoryWithoutUnit(t *testing.T) { + const ( + expectedVal = 2048 + expectedUnit = "" + ) + + qemuSystem := QemuSystem{ + Memory: "2048", + } + + val, unit, err := qemuSystem.parseMemory() + if err != nil { + t.Fatalf("Unable to parse the memory argument: %q", err) + } + + if val != expectedVal { + t.Fatalf("Invalid memory value: Expected %v, got %v", expectedVal, val) + } + + if unit != expectedUnit { + t.Fatalf("Invalid memory unit: Expected %q, got %q", expectedUnit, unit) + } +} + +// Test that the memory argument parser returns the value and unit when the memory +// arg contains both +func TestParseMemory(t *testing.T) { + const ( + expectedVal = 2048 + expectedUnit = "M" + ) + + qemuSystem := QemuSystem{ + Memory: "2048M", + } + + val, unit, err := qemuSystem.parseMemory() + if err != nil { + t.Fatalf("Unable to parse the memory argument: %q", err) + } + + if val != expectedVal { + t.Fatalf("Invalid memory value: Expected %v, got %v", expectedVal, val) + } + + if unit != expectedUnit { + t.Fatalf("Invalid memory unit: Expected %q, got %q", expectedUnit, unit) + } +} + +// Test that there are no NUMA arguments when requesting a single node +func TestNumaArgumentsSingle(t *testing.T) { + expectedNumaArgs := []string{} + + qemuSystem := QemuSystem{ + Memory: "3072M", + CpuCount: 9, + Numa: 1, + } + + numaArgs, err := qemuSystem.generateNumaArguments() + if err != nil { + t.Fatalf("Unable to generate the NUMA arguments: %q", err) + } + + if !reflect.DeepEqual(numaArgs, expectedNumaArgs) { + t.Fatalf("The generated NUMA arguments are invalid.\nExpected: %v\nGot: %v", expectedNumaArgs, numaArgs) + } +} + +// Test that the NUMA arguments are correct when requesting multiple nodes +func TestNumaArguments(t *testing.T) { + expectedNumaArgs := []string{ + "-object memory-backend-ram,size=1024M,id=m0", + "-numa node,nodeid=0,memdev=m0,cpus=0-2", + "-object memory-backend-ram,size=1024M,id=m1", + "-numa node,nodeid=1,memdev=m1,cpus=3-5", + "-object memory-backend-ram,size=1024M,id=m2", + "-numa node,nodeid=2,memdev=m2,cpus=6-8", + } + + qemuSystem := QemuSystem{ + Memory: "3072M", + CpuCount: 9, + Numa: 3, + } + + numaArgs, err := qemuSystem.generateNumaArguments() + if err != nil { + t.Fatalf("Unable to generate the NUMA arguments: %q", err) + } + + if !reflect.DeepEqual(numaArgs, expectedNumaArgs) { + t.Fatalf("The generated NUMA arguments are invalid.\nExpected: %v\nGot: %v", expectedNumaArgs, numaArgs) + } +} + +// Test that the NUMA arguments generation fails if a symmetric topology cannot be done +// because of CPU count +func TestNumaArgumentsInvalidSymmetricCpu(t *testing.T) { + qemuSystem := QemuSystem{ + Memory: "3072M", + CpuCount: 8, + Numa: 3, + } + + _, err := qemuSystem.generateNumaArguments() + if err == nil { + t.Fatal("No error was reported with an invalid CPU count") + } +} + +// Test that the NUMA arguments generation fails if a symmetric topology cannot be done +// because of memory amount +func TestNumaArgumentsInvalidSymmetricMemory(t *testing.T) { + qemuSystem := QemuSystem{ + Memory: "3070M", + CpuCount: 9, + Numa: 3, + } + + _, err := qemuSystem.generateNumaArguments() + if err == nil { + t.Fatal("No error was reported with an invalid amount of memory") + } +} diff --git a/cluster-provision/centos9/vmcli/cmd/root.go b/cluster-provision/centos9/vmcli/cmd/root.go index 4047a88088..1baa958104 100644 --- a/cluster-provision/centos9/vmcli/cmd/root.go +++ b/cluster-provision/centos9/vmcli/cmd/root.go @@ -50,6 +50,7 @@ func NewRootCommand() *cobra.Command { root.Flags().StringP("memory", "m", "3096M", "amount of memory of the VM") root.Flags().Uint64P("cpu", "c", 2, "amount of CPU cores in the VM") + root.Flags().Uint64P("numa", "a", 1, "amount of NUMA nodes") root.Flags().StringP("qemu-args", "q", "", "additional flags to pass to QEMU") root.Flags().StringP("additional-kernel-args", "k", "", "additional arguments passed to the kernel cmdline") root.Flags().StringP("next-disk", "n", "", "path to the primary disk image to create and attach to the VM") @@ -178,6 +179,11 @@ func run(cmd *cobra.Command, args []string) error { return err } + numaFlag, err := cmd.Flags().GetUint64("numa") + if err != nil { + return err + } + qemuArgsFlag, err := cmd.Flags().GetString("qemu-args") if err != nil { return err @@ -309,6 +315,7 @@ func run(cmd *cobra.Command, args []string) error { Arch: "x86_64", Memory: memoryFlag, CpuCount: cpuFlag, + Numa: numaFlag, KvmEnabled: true, CpuModel: "host,migratable=no,+invtsc", Machine: "q35,accel=kvm,kernel_irqchip=split", @@ -357,7 +364,12 @@ func run(cmd *cobra.Command, args []string) error { createSecondaryRawDisks(usbDeviceSizeFlag, "usb") // Start QEMU - qemuCmdline := fmt.Sprintf("%s %s", qemuSystem.GenerateCmdline(), qemuArgsFlag) + generatedQemuCmdline, err := qemuSystem.GenerateCmdline() + if err != nil { + return err + } + + qemuCmdline := fmt.Sprintf("%s %s", generatedQemuCmdline, qemuArgsFlag) fmt.Println("Starting QEMU with the following arguments:") fmt.Println(qemuCmdline) diff --git a/cluster-provision/centos9/vmcli/cmd/utils/disk.go b/cluster-provision/centos9/vmcli/cmd/utils/disk.go index f34f8658a0..1dfa920a0e 100644 --- a/cluster-provision/centos9/vmcli/cmd/utils/disk.go +++ b/cluster-provision/centos9/vmcli/cmd/utils/disk.go @@ -25,7 +25,7 @@ func NewDiskUtil() *DiskUtil { // and the path of the last disk image that was generated func (du DiskUtil) CalcNextDisk(searchDir string, forcedNextDiskPath string) (string, string, error) { // Get all the files at the filesystem root matching the pattern diskX.qcow2 - regex, err := regexp.Compile("^disk(\\d+).qcow2$") + regex, err := regexp.Compile(`^disk(\d+).qcow2$`) if err != nil { return "", "", err }