Skip to content

Commit

Permalink
refactor(error): remove unnecessary error return
Browse files Browse the repository at this point in the history 
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 <[email protected]>
  • Loading branch information
@BlackHole1
BlackHole1 committed Dec 26, 2023
1 parent 7572104 commit 1dfca99
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 95 deletions.
11 changes: 7 additions & 4 deletions pkg/config/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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) {
Expand Down
73 changes: 24 additions & 49 deletions pkg/config/json_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
},
Expand All @@ -34,26 +33,20 @@ 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"}]}`,
},
"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"}]}`,
Expand All @@ -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
},
Expand Down
40 changes: 20 additions & 20 deletions pkg/config/virtio.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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.
Expand Down
32 changes: 13 additions & 19 deletions pkg/config/virtio_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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
},
Expand All @@ -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{
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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,
},
Expand Down Expand Up @@ -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",
Expand All @@ -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},
Expand All @@ -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
},
Expand Down
Loading

0 comments on commit 1dfca99

Please sign in to comment.