From a6ffb5656f190dc7ebd96f0a84693ba9dc61dc36 Mon Sep 17 00:00:00 2001 From: Brent Baude Date: Thu, 21 Mar 2024 13:08:19 -0500 Subject: [PATCH] hyperv: fix machine rm -r this pr fixes two hyperv bugs. previous podman 5 versions of hyperv failed to actually remove the vm from hyperv when machine rm -f was called. also fixes an annoying bug where removal of the hyperv ignition entries were failing because this can only be done (with the current api) when the vm is running. new api in latest libhvee fixes this. Signed-off-by: Brent Baude --- go.mod | 2 +- go.sum | 4 +- pkg/machine/hyperv/stubber.go | 20 +++++----- .../containers/libhvee/pkg/hypervctl/vm.go | 38 ++++++++++++++----- vendor/modules.txt | 2 +- 5 files changed, 44 insertions(+), 22 deletions(-) diff --git a/go.mod b/go.mod index 92665a66de..35400aa1c4 100644 --- a/go.mod +++ b/go.mod @@ -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 diff --git a/go.sum b/go.sum index 43e1ac4f32..bdffae57da 100644 --- a/go.sum +++ b/go.sum @@ -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= diff --git a/pkg/machine/hyperv/stubber.go b/pkg/machine/hyperv/stubber.go index c5f0ec74d8..b9caef1cde 100644 --- a/pkg/machine/hyperv/stubber.go +++ b/pkg/machine/hyperv/stubber.go @@ -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" @@ -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 { return err } } diff --git a/vendor/github.com/containers/libhvee/pkg/hypervctl/vm.go b/vendor/github.com/containers/libhvee/pkg/hypervctl/vm.go index fa0b612587..7fcae23aff 100644 --- a/vendor/github.com/containers/libhvee/pkg/hypervctl/vm.go +++ b/vendor/github.com/containers/libhvee/pkg/hypervctl/vm.go @@ -89,11 +89,11 @@ func (vm *VirtualMachine) SplitAndAddIgnition(keyPrefix string, ignRdr *bytes.Re } func (vm *VirtualMachine) AddKeyValuePair(key string, value string) error { - return vm.kvpOperation("AddKvpItems", key, value, "key already exists?") + return vm.kvpOperation("AddKvpItems", key, value, false, "key already exists?") } func (vm *VirtualMachine) ModifyKeyValuePair(key string, value string) error { - return vm.kvpOperation("ModifyKvpItems", key, value, "key invalid?") + return vm.kvpOperation("ModifyKvpItems", key, value, false, "key invalid?") } func (vm *VirtualMachine) PutKeyValuePair(key string, value string) error { @@ -107,7 +107,11 @@ func (vm *VirtualMachine) PutKeyValuePair(key string, value string) error { } func (vm *VirtualMachine) RemoveKeyValuePair(key string) error { - return vm.kvpOperation("RemoveKvpItems", key, "", "key invalid?") + return vm.kvpOperation("RemoveKvpItems", key, "", false, "key invalid?") +} + +func (vm *VirtualMachine) RemoveKeyValuePairNoWait(key string) error { + return vm.kvpOperation("RemoveKvpItems", key, "", true, "key invalid?") } func (vm *VirtualMachine) GetKeyValuePairs() (map[string]string, error) { @@ -148,9 +152,10 @@ func (vm *VirtualMachine) GetKeyValuePairs() (map[string]string, error) { return parseKvpMapXml(s) } -func (vm *VirtualMachine) kvpOperation(op string, key string, value string, illegalSuggestion string) error { +func (vm *VirtualMachine) kvpOperation(op string, key string, value string, nowait bool, illegalSuggestion string) error { var service *wmiext.Service var vsms, job *wmiext.Instance + var ret int32 var err error if service, err = NewLocalHyperVService(); err != nil { @@ -172,15 +177,26 @@ func (vm *VirtualMachine) kvpOperation(op string, key string, value string, ille execution := vsms.BeginInvoke(op). In("TargetSystem", vm.Path()). In("DataItems", []string{itemStr}). - Execute() + Execute(). + Out("ReturnValue", &ret). + Out("Job", &job) - if err := execution.Out("Job", &job).End(); err != nil { + if err := execution.End(); err != nil { return fmt.Errorf("%s execution failed: %w", op, err) } - err = translateKvpError(wmiext.WaitJob(service, job), illegalSuggestion) defer job.Close() - return err + if ret == 0 || (nowait && ret == 4096) { + return nil + } + + if ret == 4096 { + err = wmiext.WaitJob(service, job) + } else { + err = &wmiext.JobError{ErrorCode: int(ret)} + } + + return translateKvpError(err, illegalSuggestion) } func waitVMResult(res int32, service *wmiext.Service, job *wmiext.Instance, errorMsg string, translate func(int) error) error { @@ -552,8 +568,12 @@ func (vm *VirtualMachine) remove() (int32, error) { srv *wmiext.Service ) + refreshVM, err := vm.vmm.GetMachine(vm.ElementName) + if err != nil { + return 0, err + } // Check for disabled/stopped state - if !Disabled.equal(vm.EnabledState) { + if !Disabled.equal(refreshVM.EnabledState) { return -1, ErrMachineStateInvalid } if srv, err = NewLocalHyperVService(); err != nil { diff --git a/vendor/modules.txt b/vendor/modules.txt index e8ba1a1151..7f52904d7d 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -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