Use PowerShell commands instead of WMI calls #258

evidolob · 2025-10-22T08:08:11Z

Replace wmi calls with powershell commands. This should improve error messages and remove need of executing this code with escalating privileges. The workflow should be the same, the most changes is in 'MemorySettings' memory field names and types, it should be set in bytes. And 'ProcessorSettings' has other name for processors count field.
Also this PR adds new sample cli 'managevm' which allow to start, stop, delete VM

openshift-ci · 2025-10-22T08:08:19Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: evidolob
Once this PR has been reviewed and has the lgtm label, please assign l0rd for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

lstocchi · 2025-10-22T16:38:39Z

I didn't dig into the code but at first sight the question i have is, can your split it in smaller commits? Reviewing the first one would be hard otherwise.

evidolob · 2025-10-23T11:05:54Z

@lstocchi I not sure, this PR is basically rewrite all project, I could split first commit in to several smaller, but it would be very hard to keep project buildable.

baude · 2025-10-23T12:58:39Z

@n1hility could you please spend at least a minute or two and review this PR because it fundamentally is a switch in how we originally wrote this.

lstocchi · 2025-10-24T13:48:10Z

pkg/hypervctl/vm.go

-		err = wmiext.WaitJob(service, job)
-	} else {
-		err = &wmiext.JobError{ErrorCode: int(ret)}
+func getKvpSctipt(vmName string, op string, key string, value string) []string {


I tried with podman and the ignition fails. You can replicate by init/start a new machine.
I did some changes to make it work but this requires more investigation, i just wanted to track where the issue was.

Summarizing what i tried, I escaped value to be sure it did not break the quotes in the script, removed comments, changed the arg of the -Command flag in Execute as this should expect a string.

Another thing i noticed is that removing a machine was really slow. Maybe it was my fault but worth mentioning to give it a deeper look

There is a typo in the func name, it should be getKvpScript

It can also be simplified by removing the Write-Host ... as they are never printed or update the code to print them when using --log-level debug

Write-Host is to write text in to stdout from powershell, I could add re-output from go code if debug logging is enabled

lstocchi · 2025-10-24T14:28:54Z

pkg/hypervctl/vmm.go

-// parameter.
-func (vmm *VirtualMachineManager) GetSummaryInformation(requestedFields SummaryRequestSet) ([]SummaryInformation, error) {
-	return vmm.getSummaryInformation("", requestedFields)
+func (vmm *VirtualMachineManager) GetSummaryInformation() ([]PowerShellVM, error) {


is this the only alternative we have? I saw that the old SummaryInformation is completely different from PowerShellVM, not sure if someone is using it. The thing that caught my eye is the old allocatedGPU which is not present now. This could be used for AI stuff

We could add GPU for vm with Set-VMGpuPartitionAdapter command, but I thing it may be a separate PR, as it involves in more changes, and introducing new API, as passing GPU is requiring modification(partitioning) on host system.

my concern was more if someone is using this. It's ok to change wmi calls to powershell commands but here we're completely changing the output. So not sure if we break up something. I'll investigate a bit, maybe @gbraad have more knowledge

pkg/hypervctl/memory_settings.go

pkg/hypervctl/network_adapter_settings.go

pkg/hypervctl/processor_settings.go

lstocchi

have you tried it with podman? It fails for me

pkg/hypervctl/network_adapter_settings.go

lstocchi · 2025-10-28T13:12:27Z

pkg/hypervctl/system_settings.go

+	NewVMName string
+
+	// ProcessorCount specifies the number of virtual processors for the virtual machine.
+	ProcessorCount uint32


int64? https://learn.microsoft.com/en-us/powershell/module/hyper-v/set-vm?view=windowsserver2025-ps#-processorcount

pkg/hypervctl/system_settings.go

lstocchi · 2025-10-28T13:24:22Z

pkg/hypervctl/system_settings.go

+	if val, ok := vmData["AutomaticStopAction"].(string); ok {
+		settings.AutomaticStopAction = AutomaticStopActionType(val)


Same as above

pkg/hypervctl/system_settings.go

pkg/hypervctl/vhd.go

pkg/hypervctl/vm.go

lstocchi · 2025-10-28T15:44:19Z

pkg/hypervctl/vm.go

+	args := []string{"Hyper-V\\Stop-VM", "-Name", vm.Name, "-Force"}
+	if force {
+		args = append(args, "-TurnOff")


this looks weird. If force is true i expect -Force to be added.
Maybe we could change the variable name here? Or provide 3 different stop actions (stop, force stop, turn off)
Based on the example it looks like when -TurnOff is used -Force is not needed -> https://learn.microsoft.com/en-us/powershell/module/hyper-v/stop-vm?view=windowsserver2025-ps#examples

The problem with plain stop-vm is work like Shuts down virtual machine TestVM through the guest operating system. which means guest OS need to have vm addons integrated to allow stop. In case if gest os doesn't have such integrations and you call stop-vm without -Force, the command will hung for 5-10m and fail, and VM will stuck in transition period, you cannot do anything with VM until that timeout is over, and after that VM will still continue to run.

So that is a question does we always run guest OS with hyper-v integrations to relay on stop without forcing?
cc @gbraad

This is a library so i expect the caller to decide what to do. It's their choice to try a graceful stop (call Stop()), force it (call StopWithForce()) or turn it off (call TurnOff()).

I expect new OS image specific to Hyperv to have the addon enabled tbh. At least for the images used with macadam and podman i see they have the Shutdown service running. We could also verify if the service is disabled and directly returns an error to the caller (if they call Stop()) to inform they must use StopWithforce/turnoff so they do not wait 5+ mins ??

Something like

func (vm *VirtualMachine) TurnOff() error { return vm.stop("-TurnOff") } func (vm *VirtualMachine) StopWithForce() error { return vm.stop("-Force") } func (vm *VirtualMachine) Stop() error { return vm.stop("") } func (vm *VirtualMachine) stop(flag string) error { if !Enabled.equal(uint16(vm.State)) { return ErrMachineNotRunning } args := []string{"Stop-VM", "-Name", vm.Name} if flag != "" { args = append(args, flag) } _, stderr, err := powershell.Execute(args...) if err != nil { return NewPSError(stderr) } return nil }

as long as you dont change functionally the library call for folks, i'm cool with this.

pkg/hypervctl/vmm.go

baude · 2025-10-30T13:01:34Z

this PR makes me super nervous but lets see it through. once you have tests passing here, id lke to ask you to vendor this into github.com/containers/podman and create a draft PR and ensure that PR passes the entire test quite. This might be tricky so ask for help if needed. would you be willing to do this ?

Replace wmi calls with powershell commands. This should improve error messages and remove need of executing this code with escalating privileges. The workflow should be the same, the most changes is in 'MemorySettings' memory field names and types, it should be set in bytes. And 'ProcessorSettings' has other name for processors count field. Signed-off-by: Yevhen Vydolob <[email protected]> Signed-off-by: Yevhen Vydolob <[email protected]>

Signed-off-by: Yevhen Vydolob <[email protected]>

evidolob · 2025-10-31T08:46:23Z

@baude Sure, I make draft PR for podman, and ensure that this changes doesn't brake anything on podman.

evidolob · 2025-10-31T08:58:42Z

@lstocchi @baude Do you know how to view win_test results, when I try to look on job page it give me this:

lstocchi · 2025-10-31T09:52:10Z

@lstocchi @baude Do you know how to view win_test results, when I try to look on job page it give me this

Same for me. I also tried to look at successful builds and i don't see any logs.
By logging in the cirrus website i just see a 404 error. I don't have the rights to dig more so no idea what's happening.

baude · 2025-10-31T12:40:43Z

@lstocchi @baude Do you know how to view win_test results, when I try to look on job page it give me this

Same for me. I also tried to look at successful builds and i don't see any logs. By logging in the cirrus website i just see a 404 error. I don't have the rights to dig more so no idea what's happening.

There might be something going on with the CI; in the meanwhile, do all tests pass locally ?

evidolob · 2025-11-03T09:07:40Z

@baude Local test are passing fine

baude · 2025-11-03T19:17:43Z

@evidolob we updated something on our side and i reran your tests and they failed.

openshift-merge-robot added the needs-rebase label Oct 22, 2025

evidolob force-pushed the rework_to_ps branch 3 times, most recently from d04a4bc to 7a7d879 Compare October 22, 2025 08:32

openshift-merge-robot removed the needs-rebase label Oct 22, 2025

evidolob mentioned this pull request Oct 22, 2025

show user friendly error messages instead of error codes #216

Open

lstocchi reviewed Oct 24, 2025

View reviewed changes

openshift-merge-robot added the needs-rebase label Oct 24, 2025

evidolob force-pushed the rework_to_ps branch from 7a7d879 to 0a99894 Compare October 27, 2025 07:58

openshift-merge-robot removed the needs-rebase label Oct 27, 2025

evidolob force-pushed the rework_to_ps branch from 0a99894 to 2aa6ad5 Compare October 27, 2025 09:44

evidolob requested a review from lstocchi October 27, 2025 09:46

lstocchi reviewed Oct 28, 2025

View reviewed changes

evidolob added 2 commits October 31, 2025 10:32

Update dependencies

95ea80c

Signed-off-by: Yevhen Vydolob <[email protected]>

evidolob force-pushed the rework_to_ps branch from 2aa6ad5 to 95ea80c Compare October 31, 2025 08:32

		if val, ok := vmData["AutomaticStopAction"].(string); ok {
		settings.AutomaticStopAction = AutomaticStopActionType(val)

Use PowerShell commands instead of WMI calls #258

Are you sure you want to change the base?

Use PowerShell commands instead of WMI calls #258

Uh oh!

Conversation

evidolob commented Oct 22, 2025

Uh oh!

openshift-ci bot commented Oct 22, 2025

Uh oh!

lstocchi commented Oct 22, 2025

Uh oh!

evidolob commented Oct 23, 2025

Uh oh!

baude commented Oct 23, 2025

Uh oh!

lstocchi Oct 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lstocchi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

baude commented Oct 30, 2025

Uh oh!

evidolob commented Oct 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

evidolob commented Oct 31, 2025

Uh oh!

lstocchi commented Oct 31, 2025

Uh oh!

baude commented Oct 31, 2025

Uh oh!

evidolob commented Nov 3, 2025

Uh oh!

baude commented Nov 3, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

lstocchi Oct 24, 2025 •

edited

Loading

evidolob commented Oct 31, 2025 •

edited

Loading