[shimV2] [VM controller] refactor + adds Update + Device controllers#2663
[shimV2] [VM controller] refactor + adds Update + Device controllers#2663rawahars wants to merge 3 commits intomicrosoft:mainfrom
Conversation
Following the shim V2 standard, we are declaring the interfaces at the place where the APIs are used. Therefore, we are deleting all the interfaces from the existing packages. Signed-off-by: Harsh Rawat <harshrawat@microsoft.com>
In this commit, we are refactoring the vm controller to bring it to the same standard as rest of the controllers. Additionally, we are adding- - Update functionality - RuntimeID API - APIs to obtain device controllers Signed-off-by: Harsh Rawat <harshrawat@microsoft.com>
| ID string | ||
|
|
||
| // HCSDocument specifies the HCS schema document used to create the VM. | ||
| HCSDocument *hcsschema.ComputeSystem |
There was a problem hiding this comment.
Note to self, I would have imagined the Create call is what makes this. How does the caller have it for create opts?
| // Update is used to update the VM configuration on-the-fly. | ||
| // It supports modifying resources like CPU and memory while the VM is running. | ||
| // It also supports injecting policy fragments or updating the CPU group id for the VM. | ||
| func (c *Controller) Update(ctx context.Context, resources interface{}, annots map[string]string) error { |
There was a problem hiding this comment.
Should we really reflect the generic "resources" into the controller? Kinda feels like at this level we should have it scenario based. UpdatePolicyFragment, UpdateCGroup etc. Then you know what you are supporting and what is being called etc. Its not just a generic API
| // NetworkController returns a new controller for managing network devices on the VM. | ||
| // Since we have a namespace per pod, we create a new controller per call. | ||
| func (c *Controller) NetworkController() *network.Controller { | ||
| return network.New(c.uvm, c.guest, c.guest) |
There was a problem hiding this comment.
New? Why not same one each time?
|
|
||
| // Iterate over the well-known controller GUIDs so the slice index gives us | ||
| // the correct controller number directly. | ||
| for ctrlIdx, guid := range guestrequest.ScsiControllerGuids { |
There was a problem hiding this comment.
I dont get this? Are you saying we force all controllers to have the same guid?
| } | ||
|
|
||
| // Translate OCI CPU knobs to HCS processor limits. | ||
| if resources.CPU != nil { |
There was a problem hiding this comment.
Should the OCI layer be in the vm layer? It feels like this call could just accept the hcsschmea ProcessorLimits
| @@ -0,0 +1,30 @@ | |||
| //go:build windows && !wcow && !lcow | |||
There was a problem hiding this comment.
We can delete this now right? Nobody would link the controller outside of those tags?
| } | ||
|
|
||
| // updateVMResources applies Windows VM memory and CPU limits from OCI resources. | ||
| func (c *Controller) updateVMResources(ctx context.Context, data interface{}) error { |
There was a problem hiding this comment.
Why cant this be shared with lcow? They seem the same at a first glance.
Summary
In this change, we are adding the following changes-
Additionally, we are adding-
RuntimeIDAPI to return the HCS compute IDlcowbuilds