From 1dfca9983f8aa249d9f201f5d866cc24fe6f2f7d Mon Sep 17 00:00:00 2001 From: Black-Hole1 Date: Tue, 26 Dec 2023 11:43:29 +0800 Subject: [PATCH] refactor(error): remove unnecessary error return If a function does not produce any errors, it should not return an error (except for functions defined by an interface). This will reduce the mental burden on users. The AddDevices method was also added, on the side. Signed-off-by: Black-Hole1 --- pkg/config/config.go | 11 +++--- pkg/config/json_test.go | 73 +++++++++++++-------------------------- pkg/config/virtio.go | 40 ++++++++++----------- pkg/config/virtio_test.go | 32 +++++++---------- pkg/rest/vf/vm_config.go | 6 ++-- 5 files changed, 67 insertions(+), 95 deletions(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index 61e55f12..f818a167 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -142,10 +142,13 @@ func (vm *VirtualMachine) VirtioVsockDevices() []*VirtioVsock { // AddDevice adds a dev to vm. This device can be created with one of the // VirtioXXXNew methods. -func (vm *VirtualMachine) AddDevice(dev VirtioDevice) error { +func (vm *VirtualMachine) AddDevice(dev VirtioDevice) { vm.Devices = append(vm.Devices, dev) +} - return nil +// AddDevices adds a list of devices to vm. +func (vm *VirtualMachine) AddDevices(dev ...VirtioDevice) { + vm.Devices = append(vm.Devices, dev...) } func (vm *VirtualMachine) AddTimeSyncFromCmdLine(cmdlineOpts string) error { @@ -165,10 +168,10 @@ func (vm *VirtualMachine) TimeSync() *TimeSync { return vm.Timesync } -func TimeSyncNew(vsockPort uint) (VMComponent, error) { +func TimeSyncNew(vsockPort uint) VMComponent { return &TimeSync{ VsockPort: vsockPort, - }, nil + } } func (ts *TimeSync) ToCmdLine() ([]string, error) { diff --git a/pkg/config/json_test.go b/pkg/config/json_test.go index 64ac0864..f8b91133 100644 --- a/pkg/config/json_test.go +++ b/pkg/config/json_test.go @@ -24,8 +24,7 @@ var jsonTests = map[string]jsonTest{ "TestTimeSync": { newVM: func(t *testing.T) *VirtualMachine { vm := newLinuxVM(t) - timesync, err := TimeSyncNew(1234) - require.NoError(t, err) + timesync := TimeSyncNew(1234) vm.Timesync = timesync.(*TimeSync) return vm }, @@ -34,10 +33,8 @@ var jsonTests = map[string]jsonTest{ "TestVirtioRNG": { newVM: func(t *testing.T) *VirtualMachine { vm := newLinuxVM(t) - virtioRng, err := VirtioRngNew() - require.NoError(t, err) - err = vm.AddDevice(virtioRng) - require.NoError(t, err) + virtioRng := VirtioRngNew() + vm.AddDevice(virtioRng) return vm }, expectedJSON: `{"vcpus":3,"memoryBytes":4000000000,"bootloader":{"kind":"linuxBootloader","VmlinuzPath":"/vmlinuz","KernelCmdLine":"/initrd","InitrdPath":"console=hvc0"},"devices":[{"kind":"virtiorng"}]}`, @@ -45,15 +42,11 @@ var jsonTests = map[string]jsonTest{ "TestMultipleVirtioBlk": { newVM: func(t *testing.T) *VirtualMachine { vm := newLinuxVM(t) - virtioBlk, err := VirtioBlkNew("/virtioblk1") - require.NoError(t, err) - err = vm.AddDevice(virtioBlk) - require.NoError(t, err) - virtioBlk, err = VirtioBlkNew("/virtioblk2") - require.NoError(t, err) + virtioBlk := VirtioBlkNew("/virtioblk1") + vm.AddDevice(virtioBlk) + virtioBlk = VirtioBlkNew("/virtioblk2") virtioBlk.SetDeviceIdentifier("virtio-blk2") - err = vm.AddDevice(virtioBlk) - require.NoError(t, err) + vm.AddDevice(virtioBlk) return vm }, expectedJSON: `{"vcpus":3,"memoryBytes":4000000000,"bootloader":{"kind":"linuxBootloader","VmlinuzPath":"/vmlinuz","KernelCmdLine":"/initrd","InitrdPath":"console=hvc0"},"devices":[{"kind":"virtioblk","DevName":"virtio-blk","ImagePath":"/virtioblk1","ReadOnly":false,"DeviceIdentifier":""},{"kind":"virtioblk","DevName":"virtio-blk","ImagePath":"/virtioblk2","ReadOnly":false,"DeviceIdentifier":"virtio-blk2"}]}`, @@ -62,55 +55,37 @@ var jsonTests = map[string]jsonTest{ newVM: func(t *testing.T) *VirtualMachine { vm := newLinuxVM(t) // virtio-serial - dev, err := VirtioSerialNew("/virtioserial") - require.NoError(t, err) - err = vm.AddDevice(dev) - require.NoError(t, err) + dev := VirtioSerialNew("/virtioserial") + vm.AddDevice(dev) // virtio-input - dev, err = VirtioInputNew(VirtioInputKeyboardDevice) - require.NoError(t, err) - err = vm.AddDevice(dev) + dev, err := VirtioInputNew(VirtioInputKeyboardDevice) require.NoError(t, err) + vm.AddDevice(dev) // virtio-gpu - dev, err = VirtioGPUNew() - require.NoError(t, err) - err = vm.AddDevice(dev) + dev = VirtioGPUNew() require.NoError(t, err) + vm.AddDevice(dev) // virtio-net dev, err = VirtioNetNew("00:11:22:33:44:55") require.NoError(t, err) - err = vm.AddDevice(dev) - require.NoError(t, err) + vm.AddDevice(dev) // virtio-rng - dev, err = VirtioRngNew() - require.NoError(t, err) - err = vm.AddDevice(dev) - require.NoError(t, err) + dev = VirtioRngNew() + vm.AddDevice(dev) // virtio-blk - dev, err = VirtioBlkNew("/virtioblk") - require.NoError(t, err) - err = vm.AddDevice(dev) - require.NoError(t, err) + dev = VirtioBlkNew("/virtioblk") + vm.AddDevice(dev) // virtio-vsock - dev, err = VirtioVsockNew(1234, "/virtiovsock", false) - require.NoError(t, err) - err = vm.AddDevice(dev) - require.NoError(t, err) + dev = VirtioVsockNew(1234, "/virtiovsock", false) + vm.AddDevice(dev) // virtio-fs - dev, err = VirtioFsNew("/virtiofs", "tag") - require.NoError(t, err) - err = vm.AddDevice(dev) - require.NoError(t, err) + fs := VirtioFsNew("/virtiofs", "tag") // USB mass storage - dev, err = USBMassStorageNew("/usbmassstorage") - require.NoError(t, err) - err = vm.AddDevice(dev) + usb := USBMassStorageNew("/usbmassstorage") require.NoError(t, err) // rosetta - dev, err = RosettaShareNew("vz-rosetta") - require.NoError(t, err) - err = vm.AddDevice(dev) - require.NoError(t, err) + rosetta := RosettaShareNew("vz-rosetta") + vm.AddDevices(fs, usb, rosetta) return vm }, diff --git a/pkg/config/virtio.go b/pkg/config/virtio.go index 318e08d7..3a46f352 100644 --- a/pkg/config/virtio.go +++ b/pkg/config/virtio.go @@ -176,16 +176,16 @@ func deviceFromCmdLine(deviceOpts string) (VirtioDevice, error) { // VirtioSerialNew creates a new serial device for the virtual machine. The // output the virtual machine sent to the serial port will be written to the // file at logFilePath. -func VirtioSerialNew(logFilePath string) (VirtioDevice, error) { +func VirtioSerialNew(logFilePath string) VirtioDevice { return &VirtioSerial{ LogFile: logFilePath, - }, nil + } } -func VirtioSerialNewStdio() (VirtioDevice, error) { +func VirtioSerialNewStdio() VirtioDevice { return &VirtioSerial{ UsesStdio: true, - }, nil + } } func (dev *VirtioSerial) validate() error { @@ -276,14 +276,14 @@ func (dev *VirtioInput) FromOptions(options []option) error { // VirtioGPUNew creates a new gpu device for the virtual machine. // The usesGUI parameter determines whether a graphical application window will // be displayed -func VirtioGPUNew() (VirtioDevice, error) { +func VirtioGPUNew() VirtioDevice { return &VirtioGPU{ UsesGUI: false, VirtioGPUResolution: VirtioGPUResolution{ Width: defaultVirtioGPUResolutionWidth, Height: defaultVirtioGPUResolutionHeight, }, - }, nil + } } func (dev *VirtioGPU) validate() error { @@ -349,7 +349,7 @@ func VirtioNetNew(macAddress string) (*VirtioNet, error) { }, nil } -// Set the socket to use for the network communication +// SetSocket Set the socket to use for the network communication // // This maps the virtual machine network interface to a connected datagram // socket. This means all network traffic on this interface will go through @@ -437,8 +437,8 @@ func (dev *VirtioNet) FromOptions(options []option) error { // VirtioRngNew creates a new random number generator device to feed entropy // into the virtual machine. -func VirtioRngNew() (VirtioDevice, error) { - return &VirtioRng{}, nil +func VirtioRngNew() VirtioDevice { + return &VirtioRng{} } func (dev *VirtioRng) ToCmdLine() ([]string, error) { @@ -463,11 +463,11 @@ func virtioBlkNewEmpty() *VirtioBlk { // VirtioBlkNew creates a new disk to use in the virtual machine. It will use // the file at imagePath as the disk image. This image must be in raw format. -func VirtioBlkNew(imagePath string) (*VirtioBlk, error) { +func VirtioBlkNew(imagePath string) *VirtioBlk { virtioBlk := virtioBlkNewEmpty() virtioBlk.ImagePath = imagePath - return virtioBlk, nil + return virtioBlk } func (dev *VirtioBlk) SetDeviceIdentifier(devID string) { @@ -507,12 +507,12 @@ func (dev *VirtioBlk) ToCmdLine() ([]string, error) { // vsock port, and on the host it will use the unix socket at socketURL. // When listen is true, the host will be listening for connections over vsock. // When listen is false, the guest will be listening for connections over vsock. -func VirtioVsockNew(port uint, socketURL string, listen bool) (VirtioDevice, error) { +func VirtioVsockNew(port uint, socketURL string, listen bool) VirtioDevice { return &VirtioVsock{ Port: port, SocketURL: socketURL, Listen: listen, - }, nil + } } func (dev *VirtioVsock) ToCmdLine() ([]string, error) { @@ -555,13 +555,13 @@ func (dev *VirtioVsock) FromOptions(options []option) error { // VirtioFsNew creates a new virtio-fs device for file sharing. It will share // the directory at sharedDir with the virtual machine. This directory can be // mounted in the VM using `mount -t virtiofs mountTag /some/dir` -func VirtioFsNew(sharedDir string, mountTag string) (VirtioDevice, error) { +func VirtioFsNew(sharedDir string, mountTag string) VirtioDevice { return &VirtioFs{ DirectorySharingConfig: DirectorySharingConfig{ MountTag: mountTag, }, SharedDir: sharedDir, - }, nil + } } func (dev *VirtioFs) ToCmdLine() ([]string, error) { @@ -589,16 +589,16 @@ func (dev *VirtioFs) FromOptions(options []option) error { return nil } -// RosettaShare creates a new rosetta share for running x86_64 binaries on M1 machines. +// RosettaShareNew creates a new rosetta share for running x86_64 binaries on M1 machines. // It will share a directory containing the linux rosetta binaries with the // virtual machine. This directory can be mounted in the VM using `mount -t // virtiofs mountTag /some/dir` -func RosettaShareNew(mountTag string) (VirtioDevice, error) { +func RosettaShareNew(mountTag string) VirtioDevice { return &RosettaShare{ DirectorySharingConfig: DirectorySharingConfig{ MountTag: mountTag, }, - }, nil + } } func (dev *RosettaShare) ToCmdLine() ([]string, error) { @@ -643,11 +643,11 @@ func usbMassStorageNewEmpty() *USBMassStorage { // USBMassStorageNew creates a new USB disk to use in the virtual machine. It will use // the file at imagePath as the disk image. This image must be in raw or ISO format. -func USBMassStorageNew(imagePath string) (VMComponent, error) { +func USBMassStorageNew(imagePath string) VMComponent { usbMassStorage := usbMassStorageNewEmpty() usbMassStorage.ImagePath = imagePath - return usbMassStorage, nil + return usbMassStorage } // StorageConfig configures a disk device. diff --git a/pkg/config/virtio_test.go b/pkg/config/virtio_test.go index 0b947a18..2ffb2ea7 100644 --- a/pkg/config/virtio_test.go +++ b/pkg/config/virtio_test.go @@ -16,7 +16,7 @@ type virtioDevTest struct { var virtioDevTests = map[string]virtioDevTest{ "NewVirtioBlk": { - newDev: func() (VirtioDevice, error) { return VirtioBlkNew("/foo/bar") }, + newDev: func() (VirtioDevice, error) { return VirtioBlkNew("/foo/bar"), nil }, expectedDev: &VirtioBlk{ StorageConfig: StorageConfig{ DevName: "virtio-blk", @@ -28,10 +28,7 @@ var virtioDevTests = map[string]virtioDevTest{ }, "NewVirtioBlkWithDevId": { newDev: func() (VirtioDevice, error) { - dev, err := VirtioBlkNew("/foo/bar") - if err != nil { - return nil, err - } + dev := VirtioBlkNew("/foo/bar") dev.SetDeviceIdentifier("test") return dev, nil }, @@ -46,14 +43,14 @@ var virtioDevTests = map[string]virtioDevTest{ alternateCmdLine: []string{"--device", "virtio-blk,deviceId=test,path=/foo/bar"}, }, "NewVirtioFs": { - newDev: func() (VirtioDevice, error) { return VirtioFsNew("/foo/bar", "") }, + newDev: func() (VirtioDevice, error) { return VirtioFsNew("/foo/bar", ""), nil }, expectedDev: &VirtioFs{ SharedDir: "/foo/bar", }, expectedCmdLine: []string{"--device", "virtio-fs,sharedDir=/foo/bar"}, }, "NewVirtioFsWithTag": { - newDev: func() (VirtioDevice, error) { return VirtioFsNew("/foo/bar", "myTag") }, + newDev: func() (VirtioDevice, error) { return VirtioFsNew("/foo/bar", "myTag"), nil }, expectedDev: &VirtioFs{ SharedDir: "/foo/bar", DirectorySharingConfig: DirectorySharingConfig{ @@ -64,7 +61,7 @@ var virtioDevTests = map[string]virtioDevTest{ alternateCmdLine: []string{"--device", "virtio-fs,mountTag=myTag,sharedDir=/foo/bar"}, }, "NewRosettaShare": { - newDev: func() (VirtioDevice, error) { return RosettaShareNew("myTag") }, + newDev: func() (VirtioDevice, error) { return RosettaShareNew("myTag"), nil }, expectedDev: &RosettaShare{ DirectorySharingConfig: DirectorySharingConfig{ MountTag: "myTag", @@ -73,7 +70,7 @@ var virtioDevTests = map[string]virtioDevTest{ expectedCmdLine: []string{"--device", "rosetta,mountTag=myTag"}, }, "NewVirtioVsock": { - newDev: func() (VirtioDevice, error) { return VirtioVsockNew(1234, "/foo/bar.unix", false) }, + newDev: func() (VirtioDevice, error) { return VirtioVsockNew(1234, "/foo/bar.unix", false), nil }, expectedDev: &VirtioVsock{ Port: 1234, SocketURL: "/foo/bar.unix", @@ -82,7 +79,7 @@ var virtioDevTests = map[string]virtioDevTest{ alternateCmdLine: []string{"--device", "virtio-vsock,socketURL=/foo/bar.unix,connect,port=1234"}, }, "NewVirtioVsockWithListen": { - newDev: func() (VirtioDevice, error) { return VirtioVsockNew(1234, "/foo/bar.unix", true) }, + newDev: func() (VirtioDevice, error) { return VirtioVsockNew(1234, "/foo/bar.unix", true), nil }, expectedDev: &VirtioVsock{ Port: 1234, SocketURL: "/foo/bar.unix", @@ -92,19 +89,19 @@ var virtioDevTests = map[string]virtioDevTest{ alternateCmdLine: []string{"--device", "virtio-vsock,socketURL=/foo/bar.unix,listen,port=1234"}, }, "NewVirtioRng": { - newDev: VirtioRngNew, + newDev: func() (VirtioDevice, error) { return VirtioRngNew(), nil }, expectedDev: &VirtioRng{}, expectedCmdLine: []string{"--device", "virtio-rng"}, }, "NewVirtioSerial": { - newDev: func() (VirtioDevice, error) { return VirtioSerialNew("/foo/bar.log") }, + newDev: func() (VirtioDevice, error) { return VirtioSerialNew("/foo/bar.log"), nil }, expectedDev: &VirtioSerial{ LogFile: "/foo/bar.log", }, expectedCmdLine: []string{"--device", "virtio-serial,logFilePath=/foo/bar.log"}, }, "NewVirtioSerialStdio": { - newDev: VirtioSerialNewStdio, + newDev: func() (VirtioDevice, error) { return VirtioSerialNewStdio(), nil }, expectedDev: &VirtioSerial{ UsesStdio: true, }, @@ -142,7 +139,7 @@ var virtioDevTests = map[string]virtioDevTest{ alternateCmdLine: []string{"--device", "virtio-net,mac=00:11:22:33:44:55,nat"}, }, "NewUSBMassStorage": { - newDev: func() (VirtioDevice, error) { return USBMassStorageNew("/foo/bar") }, + newDev: func() (VirtioDevice, error) { return USBMassStorageNew("/foo/bar"), nil }, expectedDev: &USBMassStorage{ StorageConfig: StorageConfig{ DevName: "usb-mass-storage", @@ -166,7 +163,7 @@ var virtioDevTests = map[string]virtioDevTest{ expectedCmdLine: []string{"--device", "virtio-input,keyboard"}, }, "NewVirtioGPUDevice": { - newDev: VirtioGPUNew, + newDev: func() (VirtioDevice, error) { return VirtioGPUNew(), nil }, expectedDev: &VirtioGPU{ false, VirtioGPUResolution{Width: 800, Height: 600}, @@ -175,10 +172,7 @@ var virtioDevTests = map[string]virtioDevTest{ }, "NewVirtioGPUDeviceWithDimensions": { newDev: func() (VirtioDevice, error) { - dev, err := VirtioGPUNew() - if err != nil { - return nil, err - } + dev := VirtioGPUNew() dev.(*VirtioGPU).VirtioGPUResolution = VirtioGPUResolution{Width: 1920, Height: 1080} return dev, nil }, diff --git a/pkg/rest/vf/vm_config.go b/pkg/rest/vf/vm_config.go index 82fc7a33..5f5cbbd5 100644 --- a/pkg/rest/vf/vm_config.go +++ b/pkg/rest/vf/vm_config.go @@ -18,7 +18,7 @@ func NewVzVirtualMachine(vm *vz.VirtualMachine, config *vz.VirtualMachineConfigu return &VzVirtualMachine{config: config, VzVM: vm} } -// inspect returns information about the virtual machine like hw resources +// Inspect returns information about the virtual machine like hw resources // and devices func (vm *VzVirtualMachine) Inspect(c *gin.Context) { ii := define.InspectResponse{ @@ -30,7 +30,7 @@ func (vm *VzVirtualMachine) Inspect(c *gin.Context) { c.JSON(http.StatusOK, ii) } -// getVMState retrieves the current vm state +// GetVMState retrieves the current vm state func (vm *VzVirtualMachine) GetVMState(c *gin.Context) { current := vm.GetState() c.JSON(http.StatusOK, gin.H{ @@ -43,7 +43,7 @@ func (vm *VzVirtualMachine) GetVMState(c *gin.Context) { }) } -// setVMState requests a state change on a virtual machine. At this time only +// SetVMState requests a state change on a virtual machine. At this time only // the following states are valid: // Pause - pause a running machine // Resume - resume a paused machine