Allow changing of CPUs, Memory, and Disk Size

Allow podman machine set to change CPUs, Memory and Disk size of a QEMU machine after its been created.
Disk size can only be increased.

If one setting fails to be changed, the other settings will still be applied.

Signed-off-by: Ashley Cui <acui@redhat.com>

cmd/podman/machine/set.go

@@ -4,6 +4,9 @@
 package machine
 
 import (
+	"fmt"
+	"os"
+
 	"github.com/containers/common/pkg/completion"
 	"github.com/containers/podman/v4/cmd/podman/registry"
 	"github.com/containers/podman/v4/pkg/machine"
@@ -23,9 +26,17 @@ var (
 )
 
 var (
-	setOpts = machine.SetOptions{}
+	setFlags = SetFlags{}
+	setOpts  = machine.SetOptions{}
 )
 
+type SetFlags struct {
+	CPUs     uint64
+	DiskSize uint64
+	Memory   uint64
+	Rootful  bool
+}
+
 func init() {
 	registry.Commands = append(registry.Commands, registry.CliCommand{
 		Command: setCmd,
@@ -34,7 +45,32 @@ func init() {
 	flags := setCmd.Flags()
 
 	rootfulFlagName := "rootful"
-	flags.BoolVar(&setOpts.Rootful, rootfulFlagName, false, "Whether this machine should prefer rootful container execution")
+	flags.BoolVar(&setFlags.Rootful, rootfulFlagName, false, "Whether this machine should prefer rootful container execution")
+
+	cpusFlagName := "cpus"
+	flags.Uint64Var(
+		&setFlags.CPUs,
+		cpusFlagName, 0,
+		"Number of CPUs",
+	)
+	_ = setCmd.RegisterFlagCompletionFunc(cpusFlagName, completion.AutocompleteNone)
+
+	diskSizeFlagName := "disk-size"
+	flags.Uint64Var(
+		&setFlags.DiskSize,
+		diskSizeFlagName, 0,
+		"Disk size in GB",
+	)
+
+	_ = setCmd.RegisterFlagCompletionFunc(diskSizeFlagName, completion.AutocompleteNone)
+
+	memoryFlagName := "memory"
+	flags.Uint64VarP(
+		&setFlags.Memory,
+		memoryFlagName, "m", 0,
+		"Memory in MB",
+	)
+	_ = setCmd.RegisterFlagCompletionFunc(memoryFlagName, completion.AutocompleteNone)
 }
 
 func setMachine(cmd *cobra.Command, args []string) error {
@@ -53,5 +89,23 @@ func setMachine(cmd *cobra.Command, args []string) error {
 		return err
 	}
 
-	return vm.Set(vmName, setOpts)
+	if cmd.Flags().Changed("rootful") {
+		setOpts.Rootful = &setFlags.Rootful
+	}
+	if cmd.Flags().Changed("cpus") {
+		setOpts.CPUs = &setFlags.CPUs
+	}
+	if cmd.Flags().Changed("memory") {
+		setOpts.Memory = &setFlags.Memory
+	}
+	if cmd.Flags().Changed("disk-size") {
+		setOpts.DiskSize = &setFlags.DiskSize
+	}
+
+	setErrs, lasterr := vm.Set(vmName, setOpts)
+	for _, err := range setErrs {
+		fmt.Fprintf(os.Stderr, "%v\n", err)
+	}
+
+	return lasterr
 }

docs/source/markdown/podman-machine-set.1.md

@@ -8,17 +8,29 @@ podman\-machine\-set - Sets a virtual machine setting
 
 ## DESCRIPTION
 
-Sets an updatable virtual machine setting.
-
-Options mirror values passed to `podman machine init`. Only a limited
-subset can be changed after machine initialization.
+Change a machine setting.
 
 ## OPTIONS
 
+#### **--cpus**=*number*
+
+Number of CPUs.
+Only supported for QEMU machines.
+
+#### **--disk-size**=*number*
+
+Size of the disk for the guest VM in GB.
+Can only be increased. Only supported for QEMU machines.
+
 #### **--help**
 
 Print usage statement.
 
+#### **--memory**, **-m**=*number*
+
+Memory (in MB).
+Only supported for QEMU machines.
+
 #### **--rootful**=*true|false*
 
 Whether this machine should prefer rootful (`true`) or rootless (`false`)

pkg/machine/config.go

@@ -95,7 +95,10 @@ type ListResponse struct {
 }
 
 type SetOptions struct {
-	Rootful bool
+	CPUs     *uint64
+	DiskSize *uint64
+	Memory   *uint64
+	Rootful  *bool
 }
 
 type SSHOptions struct {
@@ -118,7 +121,7 @@ type InspectOptions struct{}
 type VM interface {
 	Init(opts InitOptions) (bool, error)
 	Remove(name string, opts RemoveOptions) (string, func() error, error)
-	Set(name string, opts SetOptions) error
+	Set(name string, opts SetOptions) ([]error, error)
 	SSH(name string, opts SSHOptions) error
 	Start(name string, opts StartOptions) error
 	State(bypass bool) (Status, error)

pkg/machine/e2e/config_set.go

@@ -0,0 +1,43 @@
+package e2e
+
+import (
+	"strconv"
+)
+
+type setMachine struct {
+	cpus     *uint
+	diskSize *uint
+	memory   *uint
+
+	cmd []string
+}
+
+func (i *setMachine) buildCmd(m *machineTestBuilder) []string {
+	cmd := []string{"machine", "set"}
+	if i.cpus != nil {
+		cmd = append(cmd, "--cpus", strconv.Itoa(int(*i.cpus)))
+	}
+	if i.diskSize != nil {
+		cmd = append(cmd, "--disk-size", strconv.Itoa(int(*i.diskSize)))
+	}
+	if i.memory != nil {
+		cmd = append(cmd, "--memory", strconv.Itoa(int(*i.memory)))
+	}
+	cmd = append(cmd, m.name)
+	i.cmd = cmd
+	return cmd
+}
+
+func (i *setMachine) withCPUs(num uint) *setMachine {
+	i.cpus = &num
+	return i
+}
+func (i *setMachine) withDiskSize(size uint) *setMachine {
+	i.diskSize = &size
+	return i
+}
+
+func (i *setMachine) withMemory(num uint) *setMachine {
+	i.memory = &num
+	return i
+}

pkg/machine/e2e/set_test.go

@@ -0,0 +1,139 @@
+package e2e
+
+import (
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+)
+
+var _ = Describe("podman machine set", func() {
+	var (
+		mb      *machineTestBuilder
+		testDir string
+	)
+
+	BeforeEach(func() {
+		testDir, mb = setup()
+	})
+	AfterEach(func() {
+		teardown(originalHomeDir, testDir, mb)
+	})
+
+	It("set machine cpus", func() {
+		name := randomString(12)
+		i := new(initMachine)
+		session, err := mb.setName(name).setCmd(i.withImagePath(mb.imagePath)).run()
+		Expect(err).To(BeNil())
+		Expect(session.ExitCode()).To(Equal(0))
+
+		set := setMachine{}
+		setSession, err := mb.setName(name).setCmd(set.withCPUs(2)).run()
+		Expect(err).To(BeNil())
+		Expect(setSession.ExitCode()).To(Equal(0))
+
+		s := new(startMachine)
+		startSession, err := mb.setCmd(s).run()
+		Expect(err).To(BeNil())
+		Expect(startSession.ExitCode()).To(Equal(0))
+
+		ssh2 := sshMachine{}
+		sshSession2, err := mb.setName(name).setCmd(ssh2.withSSHComand([]string{"lscpu", "|", "grep", "\"CPU(s):\"", "|", "head", "-1"})).run()
+		Expect(err).To(BeNil())
+		Expect(sshSession2.ExitCode()).To(Equal(0))
+		Expect(sshSession2.outputToString()).To(ContainSubstring("2"))
+
+	})
+
+	It("increase machine disk size", func() {
+		name := randomString(12)
+		i := new(initMachine)
+		session, err := mb.setName(name).setCmd(i.withImagePath(mb.imagePath)).run()
+		Expect(err).To(BeNil())
+		Expect(session.ExitCode()).To(Equal(0))
+
+		set := setMachine{}
+		setSession, err := mb.setName(name).setCmd(set.withDiskSize(102)).run()
+		Expect(err).To(BeNil())
+		Expect(setSession.ExitCode()).To(Equal(0))
+
+		s := new(startMachine)
+		startSession, err := mb.setCmd(s).run()
+		Expect(err).To(BeNil())
+		Expect(startSession.ExitCode()).To(Equal(0))
+
+		ssh2 := sshMachine{}
+		sshSession2, err := mb.setName(name).setCmd(ssh2.withSSHComand([]string{"sudo", "fdisk", "-l", "|", "grep", "Disk"})).run()
+		Expect(err).To(BeNil())
+		Expect(sshSession2.ExitCode()).To(Equal(0))
+		Expect(sshSession2.outputToString()).To(ContainSubstring("102 GiB"))
+	})
+
+	It("decrease machine disk size should fail", func() {
+		name := randomString(12)
+		i := new(initMachine)
+		session, err := mb.setName(name).setCmd(i.withImagePath(mb.imagePath)).run()
+		Expect(err).To(BeNil())
+		Expect(session.ExitCode()).To(Equal(0))
+
+		set := setMachine{}
+		setSession, _ := mb.setName(name).setCmd(set.withDiskSize(50)).run()
+		// TODO seems like stderr is not being returned; re-enabled when fixed
+		// Expect(err).To(BeNil())
+		Expect(setSession.ExitCode()).To(Not(Equal(0)))
+	})
+
+	It("set machine ram", func() {
+
+		name := randomString(12)
+		i := new(initMachine)
+		session, err := mb.setName(name).setCmd(i.withImagePath(mb.imagePath)).run()
+		Expect(err).To(BeNil())
+		Expect(session.ExitCode()).To(Equal(0))
+
+		set := setMachine{}
+		setSession, err := mb.setName(name).setCmd(set.withMemory(4000)).run()
+		Expect(err).To(BeNil())
+		Expect(setSession.ExitCode()).To(Equal(0))
+
+		s := new(startMachine)
+		startSession, err := mb.setCmd(s).run()
+		Expect(err).To(BeNil())
+		Expect(startSession.ExitCode()).To(Equal(0))
+
+		ssh2 := sshMachine{}
+		sshSession2, err := mb.setName(name).setCmd(ssh2.withSSHComand([]string{"cat", "/proc/meminfo", "|", "numfmt", "--field", "2", "--from-unit=Ki", "--to-unit=Mi", "|", "sed", "'s/ kB/M/g'", "|", "grep", "MemTotal"})).run()
+		Expect(err).To(BeNil())
+		Expect(sshSession2.ExitCode()).To(Equal(0))
+		Expect(sshSession2.outputToString()).To(ContainSubstring("3824"))
+	})
+
+	It("no settings should change if no flags", func() {
+		name := randomString(12)
+		i := new(initMachine)
+		session, err := mb.setName(name).setCmd(i.withImagePath(mb.imagePath)).run()
+		Expect(err).To(BeNil())
+		Expect(session.ExitCode()).To(Equal(0))
+
+		set := setMachine{}
+		setSession, err := mb.setName(name).setCmd(&set).run()
+		Expect(err).To(BeNil())
+		Expect(setSession.ExitCode()).To(Equal(0))
+
+		s := new(startMachine)
+		startSession, err := mb.setCmd(s).run()
+		Expect(err).To(BeNil())
+		Expect(startSession.ExitCode()).To(Equal(0))
+
+		ssh2 := sshMachine{}
+		sshSession2, err := mb.setName(name).setCmd(ssh2.withSSHComand([]string{"lscpu", "|", "grep", "\"CPU(s):\"", "|", "head", "-1"})).run()
+		Expect(err).To(BeNil())
+		Expect(sshSession2.ExitCode()).To(Equal(0))
+		Expect(sshSession2.outputToString()).To(ContainSubstring("1"))
+
+		ssh3 := sshMachine{}
+		sshSession3, err := mb.setName(name).setCmd(ssh3.withSSHComand([]string{"sudo", "fdisk", "-l", "|", "grep", "Disk"})).run()
+		Expect(err).To(BeNil())
+		Expect(sshSession3.ExitCode()).To(Equal(0))
+		Expect(sshSession3.outputToString()).To(ContainSubstring("100 GiB"))
+	})
+
+})

pkg/machine/qemu/machine.go

@@ -391,25 +391,9 @@ func (v *MachineVM) Init(opts machine.InitOptions) (bool, error) {
 	if err != nil {
 		return false, err
 	}
-	// Resize the disk image to input disk size
-	// only if the virtualdisk size is less than
-	// the given disk size
-	if opts.DiskSize<<(10*3) > originalDiskSize {
-		// Find the qemu executable
-		cfg, err := config.Default()
-		if err != nil {
-			return false, err
-		}
-		resizePath, err := cfg.FindHelperBinary("qemu-img", true)
-		if err != nil {
-			return false, err
-		}
-		resize := exec.Command(resizePath, []string{"resize", v.getImageFile(), strconv.Itoa(int(opts.DiskSize)) + "G"}...)
-		resize.Stdout = os.Stdout
-		resize.Stderr = os.Stderr
-		if err := resize.Run(); err != nil {
-			return false, errors.Errorf("resizing image: %q", err)
-		}
+
+	if err := v.resizeDisk(opts.DiskSize, originalDiskSize>>(10*3)); err != nil {
+		return false, err
 	}
 	// If the user provides an ignition file, we need to
 	// copy it into the conf dir
@@ -433,14 +417,14 @@ func (v *MachineVM) Init(opts machine.InitOptions) (bool, error) {
 	return err == nil, err
 }
 
-func (v *MachineVM) Set(_ string, opts machine.SetOptions) error {
-	if v.Rootful == opts.Rootful {
-		return nil
-	}
+func (v *MachineVM) Set(_ string, opts machine.SetOptions) ([]error, error) {
+	// If one setting fails to be applied, the others settings will not fail and still be applied.
+	// The setting(s) that failed to be applied will have its errors returned in setErrors
+	var setErrors []error
 
 	state, err := v.State(false)
 	if err != nil {
-		return err
+		return setErrors, err
 	}
 
 	if state == machine.Running {
@@ -448,26 +432,45 @@ func (v *MachineVM) Set(_ string, opts machine.SetOptions) error {
 		if v.Name != machine.DefaultMachineName {
 			suffix = " " + v.Name
 		}
-		return errors.Errorf("cannot change setting while the vm is running, run 'podman machine stop%s' first", suffix)
+		return setErrors, errors.Errorf("cannot change settings while the vm is running, run 'podman machine stop%s' first", suffix)
 	}
 
-	changeCon, err := machine.AnyConnectionDefault(v.Name, v.Name+"-root")
+	if opts.Rootful != nil && v.Rootful != *opts.Rootful {
+		if err := v.setRootful(*opts.Rootful); err != nil {
+			setErrors = append(setErrors, errors.Wrapf(err, "failed to set rootful option"))
+		} else {
+			v.Rootful = *opts.Rootful
+		}
+	}
+
+	if opts.CPUs != nil && v.CPUs != *opts.CPUs {
+		v.CPUs = *opts.CPUs
+		v.editCmdLine("-smp", strconv.Itoa(int(v.CPUs)))
+	}
+
+	if opts.Memory != nil && v.Memory != *opts.Memory {
+		v.Memory = *opts.Memory
+		v.editCmdLine("-m", strconv.Itoa(int(v.Memory)))
+	}
+
+	if opts.DiskSize != nil && v.DiskSize != *opts.DiskSize {
+		if err := v.resizeDisk(*opts.DiskSize, v.DiskSize); err != nil {
+			setErrors = append(setErrors, errors.Wrapf(err, "failed to resize disk"))
+		} else {
+			v.DiskSize = *opts.DiskSize
+		}
+	}
+
+	err = v.writeConfig()
 	if err != nil {
-		return err
+		setErrors = append(setErrors, err)
 	}
 
-	if changeCon {
-		newDefault := v.Name
-		if opts.Rootful {
-			newDefault += "-root"
-		}
-		if err := machine.ChangeDefault(newDefault); err != nil {
-			return err
-		}
+	if len(setErrors) > 0 {
+		return setErrors, setErrors[0]
 	}
 
-	v.Rootful = opts.Rootful
-	return v.writeConfig()
+	return setErrors, nil
 }
 
 // Start executes the qemu command line and forks it
@@ -1464,3 +1467,64 @@ func (v *MachineVM) getImageFile() string {
 func (v *MachineVM) getIgnitionFile() string {
 	return v.IgnitionFilePath.GetPath()
 }
+
+//resizeDisk increases the size of the machine's disk in GB.
+func (v *MachineVM) resizeDisk(diskSize uint64, oldSize uint64) error {
+	// Resize the disk image to input disk size
+	// only if the virtualdisk size is less than
+	// the given disk size
+	if diskSize < oldSize {
+		return errors.Errorf("new disk size must be larger than current disk size: %vGB", oldSize)
+	}
+
+	// Find the qemu executable
+	cfg, err := config.Default()
+	if err != nil {
+		return err
+	}
+	resizePath, err := cfg.FindHelperBinary("qemu-img", true)
+	if err != nil {
+		return err
+	}
+	resize := exec.Command(resizePath, []string{"resize", v.getImageFile(), strconv.Itoa(int(diskSize)) + "G"}...)
+	resize.Stdout = os.Stdout
+	resize.Stderr = os.Stderr
+	if err := resize.Run(); err != nil {
+		return errors.Errorf("resizing image: %q", err)
+	}
+
+	return nil
+}
+
+func (v *MachineVM) setRootful(rootful bool) error {
+	changeCon, err := machine.AnyConnectionDefault(v.Name, v.Name+"-root")
+	if err != nil {
+		return err
+	}
+
+	if changeCon {
+		newDefault := v.Name
+		if rootful {
+			newDefault += "-root"
+		}
+		err := machine.ChangeDefault(newDefault)
+		if err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+func (v *MachineVM) editCmdLine(flag string, value string) {
+	found := false
+	for i, val := range v.CmdLine {
+		if val == flag {
+			found = true
+			v.CmdLine[i+1] = value
+			break
+		}
+	}
+	if !found {
+		v.CmdLine = append(v.CmdLine, []string{flag, value}...)
+	}
+}

pkg/machine/qemu/machine_test.go

@@ -0,0 +1,17 @@
+package qemu
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestEditCmd(t *testing.T) {
+	vm := new(MachineVM)
+	vm.CmdLine = []string{"command", "-flag", "value"}
+
+	vm.editCmdLine("-flag", "newvalue")
+	vm.editCmdLine("-anotherflag", "anothervalue")
+
+	require.Equal(t, vm.CmdLine, []string{"command", "-flag", "newvalue", "-anotherflag", "anothervalue"})
+}

pkg/machine/wsl/machine.go

@@ -736,28 +736,34 @@ func pipeCmdPassThrough(name string, input string, arg ...string) error {
 	return cmd.Run()
 }
 
-func (v *MachineVM) Set(name string, opts machine.SetOptions) error {
-	if v.Rootful == opts.Rootful {
-		return nil
-	}
+func (v *MachineVM) Set(_ string, opts machine.SetOptions) ([]error, error) {
+	// If one setting fails to be applied, the others settings will not fail and still be applied.
+	// The setting(s) that failed to be applied will have its errors returned in setErrors
+	var setErrors []error
 
-	changeCon, err := machine.AnyConnectionDefault(v.Name, v.Name+"-root")
-	if err != nil {
-		return err
-	}
-
-	if changeCon {
-		newDefault := v.Name
-		if opts.Rootful {
-			newDefault += "-root"
-		}
-		if err := machine.ChangeDefault(newDefault); err != nil {
-			return err
+	if opts.Rootful != nil && v.Rootful != *opts.Rootful {
+		err := v.setRootful(*opts.Rootful)
+		if err != nil {
+			setErrors = append(setErrors, errors.Wrapf(err, "error setting rootful option"))
+		} else {
+			v.Rootful = *opts.Rootful
 		}
 	}
 
-	v.Rootful = opts.Rootful
-	return v.writeConfig()
+	if opts.CPUs != nil {
+		setErrors = append(setErrors, errors.Errorf("changing CPUs not suppored for WSL machines"))
+	}
+
+	if opts.Memory != nil {
+		setErrors = append(setErrors, errors.Errorf("changing memory not suppored for WSL machines"))
+
+	}
+
+	if opts.DiskSize != nil {
+		setErrors = append(setErrors, errors.Errorf("changing Disk Size not suppored for WSL machines"))
+	}
+
+	return setErrors, v.writeConfig()
 }
 
 func (v *MachineVM) Start(name string, _ machine.StartOptions) error {
@@ -1362,3 +1368,22 @@ func (p *Provider) IsValidVMName(name string) (bool, error) {
 func (p *Provider) CheckExclusiveActiveVM() (bool, string, error) {
 	return false, "", nil
 }
+
+func (v *MachineVM) setRootful(rootful bool) error {
+	changeCon, err := machine.AnyConnectionDefault(v.Name, v.Name+"-root")
+	if err != nil {
+		return err
+	}
+
+	if changeCon {
+		newDefault := v.Name
+		if rootful {
+			newDefault += "-root"
+		}
+		err := machine.ChangeDefault(newDefault)
+		if err != nil {
+			return err
+		}
+	}
+	return nil
+}