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

hyperv: fix machine rm -r #22140

Merged
merged 1 commit into from
Mar 26, 2024
Merged

hyperv: fix machine rm -r #22140

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ require (
github.com/containers/conmon v2.0.20+incompatible
github.com/containers/gvisor-tap-vsock v0.7.3
github.com/containers/image/v5 v5.30.0
github.com/containers/libhvee v0.7.0
github.com/containers/libhvee v0.7.1
github.com/containers/ocicrypt v1.1.10
github.com/containers/psgo v1.9.0
github.com/containers/storage v1.53.0
Expand Down
4 changes: 2 additions & 2 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,8 @@ github.com/containers/gvisor-tap-vsock v0.7.3 h1:yORnf15sP+sLFhxLNLgmB5/lOhldn9d
github.com/containers/gvisor-tap-vsock v0.7.3/go.mod h1:NI1fLMtKXQZoDrrOeqryGz7x7j/XSFWRmQILva7Fu9c=
github.com/containers/image/v5 v5.30.0 h1:CmHeSwI6W2kTRWnUsxATDFY5TEX4b58gPkaQcEyrLIA=
github.com/containers/image/v5 v5.30.0/go.mod h1:gSD8MVOyqBspc0ynLsuiMR9qmt8UQ4jpVImjmK0uXfk=
github.com/containers/libhvee v0.7.0 h1:TDfidZOduYk0ZW0tigzqpJOl+CeynvHxIZCuH/ak7YM=
github.com/containers/libhvee v0.7.0/go.mod h1:fRKB3AyIqHMvq6xaeYhTpckM2cdoq0oecolyoiuLP7M=
github.com/containers/libhvee v0.7.1 h1:dWGF5GLq9DZvXo3P8aDp3cNieL5eCaSell4UmeA/jY4=
github.com/containers/libhvee v0.7.1/go.mod h1:fRKB3AyIqHMvq6xaeYhTpckM2cdoq0oecolyoiuLP7M=
github.com/containers/libtrust v0.0.0-20230121012942-c1716e8a8d01 h1:Qzk5C6cYglewc+UyGf6lc8Mj2UaPTHy/iF2De0/77CA=
github.com/containers/libtrust v0.0.0-20230121012942-c1716e8a8d01/go.mod h1:9rfv8iPl1ZP7aqh9YA68wnZv2NUDbXdcdPHVz0pFbPY=
github.com/containers/luksy v0.0.0-20240212203526-ceb12d4fd50c h1:6zalnZZODMOqNZBww9VAM1Mq5EZ3J+S8vYGCo2yg39M=
Expand Down
20 changes: 11 additions & 9 deletions pkg/machine/hyperv/stubber.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,16 @@ import (
"os/exec"
"path/filepath"

"github.com/containers/podman/v5/pkg/machine/env"
"github.com/containers/podman/v5/pkg/machine/shim/diskpull"

"github.com/Microsoft/go-winio"
"github.com/containers/common/pkg/strongunits"
gvproxy "github.com/containers/gvisor-tap-vsock/pkg/types"
"github.com/containers/libhvee/pkg/hypervctl"
"github.com/containers/podman/v5/pkg/machine"
"github.com/containers/podman/v5/pkg/machine/define"
"github.com/containers/podman/v5/pkg/machine/env"
"github.com/containers/podman/v5/pkg/machine/hyperv/vsock"
"github.com/containers/podman/v5/pkg/machine/ignition"
"github.com/containers/podman/v5/pkg/machine/shim/diskpull"
"github.com/containers/podman/v5/pkg/machine/vmconfigs"
"github.com/containers/podman/v5/pkg/systemd/parser"
"github.com/sirupsen/logrus"
Expand Down Expand Up @@ -497,12 +496,15 @@ func readAndSplitIgnition(mc *vmconfigs.MachineConfig, vm *hypervctl.VirtualMach
}

func removeIgnitionFromRegistry(vm *hypervctl.VirtualMachine) error {
pairs, err := vm.GetKeyValuePairs()
if err != nil {
return err
}
for key := range pairs {
if err := vm.RemoveKeyValuePair(key); err != nil {
// because the vm is down at this point, we cannot query hyperv for these key value pairs.
// therefore we blindly iterate from 0-50 and delete the key/value pairs. hyperv does not
// raise an error if the key is not present
//
for i := 0; i < 50; i++ {
// this is a well known "key" defined in libhvee and is the vm name
// plus an index starting at 0
key := fmt.Sprintf("%s%d", vm.ElementName, i)
if err := vm.RemoveKeyValuePairNoWait(key); err != nil {
Comment on lines +499 to +507
Copy link
Member

Choose a reason for hiding this comment

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

Ok not like I undersatnd the windows code so consider this non blocking.

First where does this 50 came from? How do we know this is enough or wont change?

Second why is it our task to remove hyper registry keys here, I would think this should be done by the vm.Remove() call in libhvee?

Copy link
Member Author

Choose a reason for hiding this comment

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

the 50 is related to the number of 1023 byte chunks needed to describe the ignition file. windows registry values can only be 1023 bytes long. So, we had to pick a number because you cannot query the vm for how many keys it has when the vm is down ... that is why we had the error before

as for libhvee, it was meant to be a generic library, not something strictly for podman ... i would not object to long term having this code live in there and it just be an API call. But for now, and 5.0.1 plans, this fixes things and puts it behind us.

return err
}
}
Expand Down
38 changes: 29 additions & 9 deletions vendor/github.com/containers/libhvee/pkg/hypervctl/vm.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion vendor/modules.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ github.com/containers/image/v5/transports
github.com/containers/image/v5/transports/alltransports
github.com/containers/image/v5/types
github.com/containers/image/v5/version
# github.com/containers/libhvee v0.7.0
# github.com/containers/libhvee v0.7.1
## explicit; go 1.18
github.com/containers/libhvee/pkg/hypervctl
github.com/containers/libhvee/pkg/kvp/ginsu
Expand Down