From 4d2fc293c04b66ef6397fbe5bed1d572524eeae6 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Wed, 6 Mar 2024 15:59:55 +0100 Subject: [PATCH] machine: make more use of strongunits To make it very clear in the code what unit the uint represents. Signed-off-by: Paul Holzinger --- cmd/podman/machine/set.go | 9 +++++---- pkg/machine/applehv/stubber.go | 7 +++---- pkg/machine/define/setopts.go | 2 +- pkg/machine/e2e/init_test.go | 4 ++-- pkg/machine/hyperv/stubber.go | 13 +++++++------ pkg/machine/qemu/command/command.go | 6 ++++-- pkg/machine/qemu/stubber.go | 2 +- pkg/machine/shim/host.go | 5 ++--- pkg/machine/vmconfigs/config.go | 5 +++-- pkg/machine/vmconfigs/machine.go | 5 +++-- pkg/machine/wsl/machine.go | 14 ++++++++------ 11 files changed, 39 insertions(+), 33 deletions(-) diff --git a/cmd/podman/machine/set.go b/cmd/podman/machine/set.go index f6aad5e41d..f1b5944ec0 100644 --- a/cmd/podman/machine/set.go +++ b/cmd/podman/machine/set.go @@ -117,15 +117,16 @@ func setMachine(cmd *cobra.Command, args []string) error { setOpts.CPUs = &mc.Resources.CPUs } if cmd.Flags().Changed("memory") { - mc.Resources.Memory = setFlags.Memory + mc.Resources.Memory = strongunits.MiB(setFlags.Memory) setOpts.Memory = &mc.Resources.Memory } if cmd.Flags().Changed("disk-size") { - if setFlags.DiskSize <= mc.Resources.DiskSize { + newDiskSizeGB := strongunits.GiB(setFlags.DiskSize) + if newDiskSizeGB <= mc.Resources.DiskSize { return fmt.Errorf("new disk size must be larger than %d GB", mc.Resources.DiskSize) } - mc.Resources.DiskSize = setFlags.DiskSize - newDiskSizeGB := strongunits.GiB(setFlags.DiskSize) + mc.Resources.DiskSize = newDiskSizeGB + setOpts.DiskSize = &newDiskSizeGB } if cmd.Flags().Changed("user-mode-networking") { diff --git a/pkg/machine/applehv/stubber.go b/pkg/machine/applehv/stubber.go index 49caf3321a..2772e89e78 100644 --- a/pkg/machine/applehv/stubber.go +++ b/pkg/machine/applehv/stubber.go @@ -11,7 +11,6 @@ import ( "time" "github.com/containers/common/pkg/config" - "github.com/containers/common/pkg/strongunits" gvproxy "github.com/containers/gvisor-tap-vsock/pkg/types" "github.com/containers/podman/v5/pkg/machine" "github.com/containers/podman/v5/pkg/machine/applehv/vfkit" @@ -55,7 +54,7 @@ func (a AppleHVStubber) CreateVM(opts define.CreateVMOpts, mc *vmconfigs.Machine mc.AppleHypervisor = new(vmconfigs.AppleHVConfig) mc.AppleHypervisor.Vfkit = vfkit.VfkitHelper{} bl := vfConfig.NewEFIBootloader(fmt.Sprintf("%s/efi-bl-%s", opts.Dirs.DataDir.GetPath(), opts.Name), true) - mc.AppleHypervisor.Vfkit.VirtualMachine = vfConfig.NewVirtualMachine(uint(mc.Resources.CPUs), mc.Resources.Memory, bl) + mc.AppleHypervisor.Vfkit.VirtualMachine = vfConfig.NewVirtualMachine(uint(mc.Resources.CPUs), uint64(mc.Resources.Memory), bl) randPort, err := utils.GetRandomPort() if err != nil { @@ -71,7 +70,7 @@ func (a AppleHVStubber) CreateVM(opts define.CreateVMOpts, mc *vmconfigs.Machine // Populate the ignition file with virtiofs stuff ignBuilder.WithUnit(generateSystemDFilesForVirtiofsMounts(virtiofsMounts)...) - return resizeDisk(mc, strongunits.GiB(mc.Resources.DiskSize)) + return resizeDisk(mc, mc.Resources.DiskSize) } func (a AppleHVStubber) Exists(name string) (bool, error) { @@ -164,7 +163,7 @@ func (a AppleHVStubber) StartVM(mc *vmconfigs.MachineConfig) (func() error, func // create a one-time virtual machine for starting because we dont want all this information in the // machineconfig if possible. the preference was to derive this stuff - vm := vfConfig.NewVirtualMachine(uint(mc.Resources.CPUs), mc.Resources.Memory, mc.AppleHypervisor.Vfkit.VirtualMachine.Bootloader) + vm := vfConfig.NewVirtualMachine(uint(mc.Resources.CPUs), uint64(mc.Resources.Memory), mc.AppleHypervisor.Vfkit.VirtualMachine.Bootloader) defaultDevices, readySocket, err := getDefaultDevices(mc) if err != nil { diff --git a/pkg/machine/define/setopts.go b/pkg/machine/define/setopts.go index 4f6ba24489..6b00478ec0 100644 --- a/pkg/machine/define/setopts.go +++ b/pkg/machine/define/setopts.go @@ -5,7 +5,7 @@ import "github.com/containers/common/pkg/strongunits" type SetOptions struct { CPUs *uint64 DiskSize *strongunits.GiB - Memory *uint64 + Memory *strongunits.MiB Rootful *bool UserModeNetworking *bool USBs *[]string diff --git a/pkg/machine/e2e/init_test.go b/pkg/machine/e2e/init_test.go index b0877e9c9f..daf4880ef6 100644 --- a/pkg/machine/e2e/init_test.go +++ b/pkg/machine/e2e/init_test.go @@ -94,7 +94,7 @@ var _ = Describe("podman machine init", func() { Expect(testMachine.Name).To(Equal(mb.names[0])) if testProvider.VMType() != define.WSLVirt { // WSL hardware specs are hardcoded Expect(testMachine.Resources.CPUs).To(Equal(uint64(cpus))) - Expect(testMachine.Resources.Memory).To(Equal(uint64(2048))) + Expect(testMachine.Resources.Memory).To(BeEquivalentTo(uint64(2048))) } }) @@ -150,7 +150,7 @@ var _ = Describe("podman machine init", func() { Expect(testMachine.Name).To(Equal(mb.names[0])) if testProvider.VMType() != define.WSLVirt { // memory and cpus something we cannot set with WSL Expect(testMachine.Resources.CPUs).To(Equal(uint64(cpus))) - Expect(testMachine.Resources.Memory).To(Equal(uint64(2048))) + Expect(testMachine.Resources.Memory).To(BeEquivalentTo(uint64(2048))) } Expect(testMachine.SSHConfig.RemoteUsername).To(Equal(remoteUsername)) diff --git a/pkg/machine/hyperv/stubber.go b/pkg/machine/hyperv/stubber.go index 70029e9e35..c5f0ec74d8 100644 --- a/pkg/machine/hyperv/stubber.go +++ b/pkg/machine/hyperv/stubber.go @@ -53,8 +53,8 @@ func (h HyperVStubber) CreateVM(opts define.CreateVMOpts, mc *vmconfigs.MachineC hwConfig := hypervctl.HardwareConfig{ CPUs: uint16(mc.Resources.CPUs), DiskPath: mc.ImagePath.GetPath(), - DiskSize: mc.Resources.DiskSize, - Memory: mc.Resources.Memory, + DiskSize: uint64(mc.Resources.DiskSize), + Memory: uint64(mc.Resources.Memory), } networkHVSock, err := vsock.NewHVSockRegistryEntry(mc.Name, vsock.Network) @@ -120,7 +120,7 @@ func (h HyperVStubber) CreateVM(opts define.CreateVMOpts, mc *vmconfigs.MachineC } callbackFuncs.Add(vmRemoveCallback) - err = resizeDisk(strongunits.GiB(mc.Resources.DiskSize), mc.ImagePath) + err = resizeDisk(mc.Resources.DiskSize, mc.ImagePath) return err } @@ -337,9 +337,10 @@ func (h HyperVStubber) SetProviderAttrs(mc *vmconfigs.MachineConfig, opts define }, func(ms *hypervctl.MemorySettings) { if memoryChanged { ms.DynamicMemoryEnabled = false - ms.VirtualQuantity = *opts.Memory - ms.Limit = *opts.Memory - ms.Reservation = *opts.Memory + mem := uint64(*opts.Memory) + ms.VirtualQuantity = mem + ms.Limit = mem + ms.Reservation = mem } }) if err != nil { diff --git a/pkg/machine/qemu/command/command.go b/pkg/machine/qemu/command/command.go index 53d1628f6d..7869b73f7d 100644 --- a/pkg/machine/qemu/command/command.go +++ b/pkg/machine/qemu/command/command.go @@ -10,6 +10,7 @@ import ( "strconv" "time" + "github.com/containers/common/pkg/strongunits" "github.com/containers/podman/v5/pkg/machine/define" ) @@ -32,8 +33,9 @@ func NewQemuBuilder(binary string, options []string) QemuCmd { } // SetMemory adds the specified amount of memory for the machine -func (q *QemuCmd) SetMemory(m uint64) { - *q = append(*q, "-m", strconv.FormatUint(m, 10)) +func (q *QemuCmd) SetMemory(m strongunits.MiB) { + // qemu accepts the memory in MiB + *q = append(*q, "-m", strconv.FormatUint(uint64(m), 10)) } // SetCPUs adds the number of CPUs the machine will have diff --git a/pkg/machine/qemu/stubber.go b/pkg/machine/qemu/stubber.go index 9fd1580630..750e444a49 100644 --- a/pkg/machine/qemu/stubber.go +++ b/pkg/machine/qemu/stubber.go @@ -116,7 +116,7 @@ func (q *QEMUStubber) CreateVM(opts define.CreateVMOpts, mc *vmconfigs.MachineCo mc.QEMUHypervisor = &qemuConfig mc.QEMUHypervisor.QEMUPidPath = qemuPidPath - return q.resizeDisk(strongunits.GiB(mc.Resources.DiskSize), mc.ImagePath) + return q.resizeDisk(mc.Resources.DiskSize, mc.ImagePath) } func runStartVMCommand(cmd *exec.Cmd) error { diff --git a/pkg/machine/shim/host.go b/pkg/machine/shim/host.go index 7fa3237d75..787607943a 100644 --- a/pkg/machine/shim/host.go +++ b/pkg/machine/shim/host.go @@ -10,7 +10,6 @@ import ( "strings" "time" - "github.com/containers/common/pkg/strongunits" "github.com/containers/podman/v5/pkg/machine" "github.com/containers/podman/v5/pkg/machine/connection" machineDefine "github.com/containers/podman/v5/pkg/machine/define" @@ -53,8 +52,8 @@ func List(vmstubbers []vmconfigs.VMProvider, _ machine.ListOptions) ([]*machine. //Stream: "", // No longer applicable VMType: s.VMType().String(), CPUs: mc.Resources.CPUs, - Memory: strongunits.MiB(mc.Resources.Memory), - DiskSize: strongunits.GiB(mc.Resources.DiskSize), + Memory: mc.Resources.Memory, + DiskSize: mc.Resources.DiskSize, Port: mc.SSH.Port, RemoteUsername: mc.SSH.RemoteUsername, IdentityPath: mc.SSH.IdentityPath, diff --git a/pkg/machine/vmconfigs/config.go b/pkg/machine/vmconfigs/config.go index 2acd075952..b92132093f 100644 --- a/pkg/machine/vmconfigs/config.go +++ b/pkg/machine/vmconfigs/config.go @@ -3,6 +3,7 @@ package vmconfigs import ( "time" + "github.com/containers/common/pkg/strongunits" gvproxy "github.com/containers/gvisor-tap-vsock/pkg/types" "github.com/containers/podman/v5/pkg/machine/define" "github.com/containers/podman/v5/pkg/machine/ignition" @@ -124,9 +125,9 @@ type ResourceConfig struct { // CPUs to be assigned to the VM CPUs uint64 // Disk size in gigabytes assigned to the vm - DiskSize uint64 + DiskSize strongunits.GiB // Memory in megabytes assigned to the vm - Memory uint64 + Memory strongunits.MiB // Usbs USBs []define.USBConfig } diff --git a/pkg/machine/vmconfigs/machine.go b/pkg/machine/vmconfigs/machine.go index dea667e315..4cf9a27186 100644 --- a/pkg/machine/vmconfigs/machine.go +++ b/pkg/machine/vmconfigs/machine.go @@ -10,6 +10,7 @@ import ( "strings" "time" + "github.com/containers/common/pkg/strongunits" define2 "github.com/containers/podman/v5/libpod/define" "github.com/containers/podman/v5/pkg/errorhandling" "github.com/containers/podman/v5/pkg/machine/connection" @@ -72,8 +73,8 @@ func NewMachineConfig(opts define.InitOptions, dirs *define.MachineDirs, sshIden // System Resources mrc := ResourceConfig{ CPUs: opts.CPUS, - DiskSize: opts.DiskSize, - Memory: opts.Memory, + DiskSize: strongunits.GiB(opts.DiskSize), + Memory: strongunits.MiB(opts.Memory), USBs: usbs, } mc.Resources = mrc diff --git a/pkg/machine/wsl/machine.go b/pkg/machine/wsl/machine.go index bc79614e3a..21a5f2004a 100644 --- a/pkg/machine/wsl/machine.go +++ b/pkg/machine/wsl/machine.go @@ -15,6 +15,7 @@ import ( "time" "github.com/containers/common/pkg/config" + "github.com/containers/common/pkg/strongunits" "github.com/containers/podman/v5/pkg/machine" "github.com/containers/podman/v5/pkg/machine/define" "github.com/containers/podman/v5/pkg/machine/env" @@ -702,7 +703,7 @@ func isRunning(name string) (bool, error) { } //nolint:unused -func getDiskSize(name string) uint64 { +func getDiskSize(name string) strongunits.GiB { vmDataDir, err := env.GetDataDir(vmtype) if err != nil { return 0 @@ -713,7 +714,7 @@ func getDiskSize(name string) uint64 { if err != nil { return 0 } - return uint64(info.Size()) + return strongunits.ToGiB(strongunits.B(info.Size())) } //nolint:unused @@ -742,7 +743,7 @@ func getCPUs(name string) (uint64, error) { } //nolint:unused -func getMem(name string) (uint64, error) { +func getMem(name string) (strongunits.MiB, error) { dist := machine.ToDist(name) if run, _ := isWSLRunning(dist); !run { return 0, nil @@ -761,13 +762,14 @@ func getMem(name string) (uint64, error) { t, a int ) for scanner.Scan() { + // fields are in kB so div to mb fields := strings.Fields(scanner.Text()) if strings.HasPrefix(fields[0], "MemTotal") && len(fields) >= 2 { t, err = strconv.Atoi(fields[1]) - total = uint64(t) * 1024 + total = uint64(t) / 1024 } else if strings.HasPrefix(fields[0], "MemAvailable") && len(fields) >= 2 { a, err = strconv.Atoi(fields[1]) - available = uint64(a) * 1024 + available = uint64(a) / 1024 } if err != nil { break @@ -775,7 +777,7 @@ func getMem(name string) (uint64, error) { } _ = cmd.Wait() - return total - available, err + return strongunits.MiB(total - available), err } //nolint:unused