Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(error): remove unnecessary error return #72

Closed
wants to merge 2 commits into from
Closed

refactor(error): remove unnecessary error return #72

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
71179cc
refactor(error): remove unnecessary error return
BlackHole1 Dec 27, 2023
a7c46b2
feat(config): add AddDevices function
BlackHole1 Dec 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions pkg/config/config.go
View file
Open in desktop
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 {
vm.Devices = append(vm.Devices, dev)
func (vm *VirtualMachine) AddDevice(dev VirtioDevice) {
vm.AddDevices(dev)
}

return nil
// AddDevices adds a list of devices to vm.
BlackHole1 marked this conversation as resolved.
Show resolved Hide resolved
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
74 changes: 24 additions & 50 deletions pkg/config/json_test.go
View file
Open in desktop
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,36 @@ 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")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the main reason why most *New() methods return an error, some of them require an error return, so for me it's better if they are consistent and all return an error, instead of having to remember it. This also makes the API future-proof if one day we need to return an error.

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)
require.NoError(t, err)
usb := USBMassStorageNew("/usbmassstorage")
// 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
Open in desktop
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
Loading
Loading