bobuhiro11 / gokvm

KVM based tiny x86 hypervisor written in pure golang, which can boot Linux
https://blog.bobuhiro11.net/tags/gokvm.html
MIT License
206 stars 21 forks source link

Add vmm package #150

Closed ChriMarMe closed 1 year ago

ChriMarMe commented 1 year ago

Add vmm package to provide a intermediate package to represent the VMM and reduce complexity of main.go

ChriMarMe commented 1 year ago

This maybe helps to improve structure for further development. What do you think?

ChriMarMe commented 1 year ago

Commandline argument -T is broken with this one. I have to find out what goes wrong first....

ChriMarMe commented 1 year ago

Commandline argument -T is broken with this one. I have to find out what goes wrong first....

This is only the case if we have multiple CPUs and this issue existed before. If gokvm is called with only one CPU it works just fine.

bobuhiro11 commented 1 year ago

I feel that the complexity of main.go has just been transferred to the vmm package and the vmm package remains complex. The complexity I feel is due to the following loop blocks.

https://github.com/bobuhiro11/gokvm/blob/cc40dd38c7cf878e0a5b1e6fdcb4503e8115969e/main.go#L71-L108

So how about moving these loop blocks to the machine package?

ChriMarMe commented 1 year ago

I can certainly move the annonymous function to the machine package, but I also would like to keep the VMM package, so if we extend gokvm to a more sophisticated program, we have proper structure in place.

My inspiration here is Cloud-Hypervisor.

I know this outlook is far beyond the capabilities at the moment, but I like to think ahead and give good layout/maintainability a higher priority. With more functionality to gokvm, the main function will get bloated. I want to avoid this.

bobuhiro11 commented 1 year ago

Let me just clarify a few things. It would look something like this.

It seems appropriate for the vmm package to externally expose the method names corresponding to operations such as Boot().

Could vmm.Init(), vmm.Setup(), and vmm.Boot() be merged into vmm.Boot()? If needed in the future, it would be good to separate them at that time.

ChriMarMe commented 1 year ago

It can be merged back into one function, but the code of each function is independent.

vmm.Init() code is responsible for the creation of the machine, vcpus, memory, but has nothing to do with the actual configuration with the exception of memory size.

vmm.Setup() is empty but is the place, where I will integrate the selection of bootprotocol, fw-image or kernel/initrd loading into the VM.

vmm.Boot() is independent from both function above, so the responsibility is soley by executing the vm.

With this layout I can integrate the different Boot Protocols (classic, elf, pvh) in the vmm.Setup()-function, without the need of bloating the independet code of vmm.Init() and vmm.Boot().

But I can merge it back together now and split it in the next PR. I 'll do as you decide here.