HyperV machine should reuse hvsock registry entries when possible

pkg/machine/compression/compression_test.go

@@ -2,6 +2,7 @@ package compression
 
 import (
 	"os"
+	"strings"
 	"testing"
 
 	"github.com/containers/podman/v5/pkg/machine/define"
@@ -132,7 +133,11 @@ func Test_Decompress(t *testing.T) {
 			require.NoError(t, err)
 			data, err := os.ReadFile(dstFilePath)
 			require.NoError(t, err)
-			assert.Equal(t, string(tt.want), string(data))
+
+			// Normalize line endings for comparison so that it works on both Windows and Unix-like systems
+			normalizedData := strings.ReplaceAll(string(data), "\r\n", "\n")
+
+			assert.Equal(t, string(tt.want), normalizedData)
 		})
 	}
 }

pkg/machine/e2e/init_windows_test.go

@@ -9,6 +9,7 @@ import (
 	"github.com/Microsoft/go-winio/vhd"
 	"github.com/containers/libhvee/pkg/hypervctl"
 	"github.com/containers/podman/v5/pkg/machine/define"
+	"github.com/containers/podman/v5/pkg/machine/hyperv/vsock"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	. "github.com/onsi/gomega/gexec"
@@ -126,4 +127,56 @@ var _ = Describe("podman machine init - windows only", func() {
 		Expect(err).ToNot(HaveOccurred())
 		Expect(checkSession).To(Exit(125))
 	})
+
+	It("init should create hvsock entries if they do not exist, otherwise should reuse existing ones", func() {
+		skipIfNotVmtype(define.HyperVVirt, "HyperV test only")
+
+		name := randomString()
+
+		// Ensure no HVSock entries exist before we start
+		networkHvSocks, err := vsock.LoadAllHVSockRegistryEntriesByPurpose(vsock.Network)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(networkHvSocks).To(BeEmpty())
+
+		readySocks, err := vsock.LoadAllHVSockRegistryEntriesByPurpose(vsock.Events)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(readySocks).To(BeEmpty())
+
+		// Execute init for the first machine. This should create new HVSock entries
+		i := new(initMachine)
+		session, err := mb.setName(name).setCmd(i.withImage(mb.imagePath)).run()
+		Expect(err).ToNot(HaveOccurred())
+		Expect(session).To(Exit(0))
+
+		// Check that the HVSock entries were created
+		networkHvSocks, err = vsock.LoadAllHVSockRegistryEntriesByPurpose(vsock.Network)
+		Expect(err).ToNot(HaveOccurred())
+		fmt.Println(networkHvSocks)
+		Expect(networkHvSocks).To(HaveLen(1))
+
+		readySocks, err = vsock.LoadAllHVSockRegistryEntriesByPurpose(vsock.Events)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(readySocks).To(HaveLen(1))
+
+		// Execute init	for another machine. This should reuse the existing HVSock entries created above
+		otherName := randomString()
+		i = new(initMachine)
+		session, err = mb.setName(otherName).setCmd(i.withImage(mb.imagePath)).run()
+		Expect(err).ToNot(HaveOccurred())
+		Expect(session).To(Exit(0))
+
+		newNetworkHvSocks, err := vsock.LoadAllHVSockRegistryEntriesByPurpose(vsock.Network)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(newNetworkHvSocks).To(HaveLen(1))
+		Expect(newNetworkHvSocks[0].Port).To(Equal(networkHvSocks[0].Port))
+
+		newReadySocks, err := vsock.LoadAllHVSockRegistryEntriesByPurpose(vsock.Events)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(newReadySocks).To(HaveLen(1))
+		Expect(newReadySocks[0].Port).To(Equal(readySocks[0].Port))
+
+		// Clean up all hvsock entries created during the test
+		err = vsock.RemoveAllHVSockRegistryEntries()
+		Expect(err).ToNot(HaveOccurred())
+	})
 })

pkg/machine/hyperv/hutil.go

@@ -0,0 +1,30 @@
+//go:build windows
+
+package hyperv
+
+import (
+	"github.com/sirupsen/logrus"
+	"golang.org/x/sys/windows"
+)
+
+func HasHyperVAdminRights() bool {
+	sid, err := windows.CreateWellKnownSid(windows.WinBuiltinHyperVAdminsSid)
+	if err != nil {
+		return false
+	}
+
+	//  From MS docs:
+	// "If TokenHandle is NULL, CheckTokenMembership uses the impersonation
+	//  token of the calling thread. If the thread is not impersonating,
+	//  the function duplicates the thread's primary token to create an
+	//  impersonation token."
+	token := windows.Token(0)
+	member, err := token.IsMember(sid)
+
+	if err != nil {
+		logrus.Warnf("Token Membership Error: %s", err)
+		return false
+	}
+
+	return member
+}

pkg/machine/hyperv/stubber.go

@@ -20,6 +20,7 @@ import (
 	"github.com/containers/podman/v5/pkg/machine/hyperv/vsock"
 	"github.com/containers/podman/v5/pkg/machine/ignition"
 	"github.com/containers/podman/v5/pkg/machine/vmconfigs"
+	"github.com/containers/podman/v5/pkg/machine/windows"
 	"github.com/containers/podman/v5/pkg/systemd/parser"
 	"github.com/sirupsen/logrus"
 )
@@ -55,9 +56,24 @@ func (h HyperVStubber) CreateVM(opts define.CreateVMOpts, mc *vmconfigs.MachineC
 		Memory:   uint64(mc.Resources.Memory),
 	}
 
-	networkHVSock, err := vsock.NewHVSockRegistryEntry(mc.Name, vsock.Network)
+	// Ensure the user has administrative privileges or is a member of the Hyper-V Administrators group.
+	// This is required for Hyper-V operations.
+	if !windows.HasAdminRights() && !HasHyperVAdminRights() {
+		return windows.ErrHypervRequiresAdmin
+	}
+
+	// Attempt to load an existing HVSock registry entry for networking.
+	// If no existing entry is found, create a new one.
+	// Creating a new entry requires administrative rights.
+	networkHVSock, err := vsock.LoadHVSockRegistryEntryByPurpose(vsock.Network)
 	if err != nil {
-		return err
+		if !windows.HasAdminRights() {
+			return windows.ErrHypervRequiresAdmin
+		}
+		networkHVSock, err = vsock.NewHVSockRegistryEntry(vsock.Network)
+		if err != nil {
+			return err
+		}
 	}
 
 	mc.HyperVHypervisor.NetworkVSock = *networkHVSock
@@ -68,17 +84,6 @@ func (h HyperVStubber) CreateVM(opts define.CreateVMOpts, mc *vmconfigs.MachineC
 		return err
 	}
 
-	removeShareCallBack := func() error {
-		return removeShares(mc)
-	}
-	callbackFuncs.Add(removeShareCallBack)
-
-	removeRegistrySockets := func() error {
-		removeNetworkAndReadySocketsFromRegistry(mc)
-		return nil
-	}
-	callbackFuncs.Add(removeRegistrySockets)
-
 	netUnitFile, err := createNetworkUnit(mc.HyperVHypervisor.NetworkVSock.Port)
 	if err != nil {
 		return err
@@ -143,9 +148,6 @@ func (h HyperVStubber) Remove(mc *vmconfigs.MachineConfig) ([]string, func() err
 	}
 
 	rmFunc := func() error {
-		// Tear down vsocks
-		removeNetworkAndReadySocketsFromRegistry(mc)
-
 		// Remove ignition registry entries - not a fatal error
 		// for vm removal
 		// TODO we could improve this by recommending an action be done
@@ -160,7 +162,7 @@ func (h HyperVStubber) Remove(mc *vmconfigs.MachineConfig) ([]string, func() err
 }
 
 func (h HyperVStubber) RemoveAndCleanMachines(_ *define.MachineDirs) error {
-	return nil
+	return vsock.RemoveAllHVSockRegistryEntries()
 }
 
 func (h HyperVStubber) StartNetworking(mc *vmconfigs.MachineConfig, cmd *gvproxy.GvproxyCommand) error {
@@ -353,9 +355,19 @@ func (h HyperVStubber) PrepareIgnition(mc *vmconfigs.MachineConfig, ignBuilder *
 	// simply be derived. So we create the HyperVConfig here.
 	mc.HyperVHypervisor = new(vmconfigs.HyperVConfig)
 	var ignOpts ignition.ReadyUnitOpts
-	readySock, err := vsock.NewHVSockRegistryEntry(mc.Name, vsock.Events)
+
+	// Attempt to load an existing HVSock registry entry for events.
+	// If no existing entry is found, create a new one.
+	// Creating a new entry requires administrative rights.
+	readySock, err := vsock.LoadHVSockRegistryEntryByPurpose(vsock.Events)
 	if err != nil {
-		return nil, err
+		if !windows.HasAdminRights() {
+			return nil, windows.ErrHypervRequiresAdmin
+		}
+		readySock, err = vsock.NewHVSockRegistryEntry(vsock.Events)
+		if err != nil {
+			return nil, err
+		}
 	}
 
 	// TODO Stopped here ... fails bc mc.Hypervisor is nil ... this can be nil checked prior and created
@@ -457,20 +469,6 @@ func resizeDisk(newSize strongunits.GiB, imagePath *define.VMFile) error {
 	return nil
 }
 
-// removeNetworkAndReadySocketsFromRegistry removes the Network and Ready sockets
-// from the Windows Registry
-func removeNetworkAndReadySocketsFromRegistry(mc *vmconfigs.MachineConfig) {
-	// Remove the HVSOCK for networking
-	if err := mc.HyperVHypervisor.NetworkVSock.Remove(); err != nil {
-		logrus.Errorf("unable to remove registry entry for %s: %q", mc.HyperVHypervisor.NetworkVSock.KeyName, err)
-	}
-
-	// Remove the HVSOCK for events
-	if err := mc.HyperVHypervisor.ReadyVsock.Remove(); err != nil {
-		logrus.Errorf("unable to remove registry entry for %s: %q", mc.HyperVHypervisor.ReadyVsock.KeyName, err)
-	}
-}
-
 // readAndSplitIgnition reads the ignition file and splits it into key:value pairs
 func readAndSplitIgnition(mc *vmconfigs.MachineConfig, vm *hypervctl.VirtualMachine) error {
 	ignFile, err := mc.IgnitionFile()

pkg/machine/hyperv/volumes.go

@@ -12,35 +12,10 @@ import (
 	"github.com/containers/podman/v5/pkg/machine"
 	"github.com/containers/podman/v5/pkg/machine/hyperv/vsock"
 	"github.com/containers/podman/v5/pkg/machine/vmconfigs"
+	"github.com/containers/podman/v5/pkg/machine/windows"
 	"github.com/sirupsen/logrus"
 )
 
-func removeShares(mc *vmconfigs.MachineConfig) error {
-	var removalErr error
-
-	for _, mount := range mc.Mounts {
-		if mount.VSockNumber == nil {
-			// nothing to do if the vsock number was never defined
-			continue
-		}
-
-		vsockReg, err := vsock.LoadHVSockRegistryEntry(*mount.VSockNumber)
-		if err != nil {
-			logrus.Debugf("Vsock %d for mountpoint %s does not have a valid registry entry, skipping removal", *mount.VSockNumber, mount.Target)
-			continue
-		}
-
-		if err := vsockReg.Remove(); err != nil {
-			if removalErr != nil {
-				logrus.Errorf("Error removing vsock: %v", removalErr)
-			}
-			removalErr = fmt.Errorf("removing vsock %d for mountpoint %s: %w", *mount.VSockNumber, mount.Target, err)
-		}
-	}
-
-	return removalErr
-}
-
 func startShares(mc *vmconfigs.MachineConfig) error {
 	for _, mount := range mc.Mounts {
 		var args []string
@@ -72,11 +47,28 @@ func startShares(mc *vmconfigs.MachineConfig) error {
 }
 
 func createShares(mc *vmconfigs.MachineConfig) (err error) {
-	for _, mount := range mc.Mounts {
-		testVsock, err := vsock.NewHVSockRegistryEntry(mc.Name, vsock.Fileserver)
-		if err != nil {
-			return err
+	fileServerVsocks, err := vsock.LoadAllHVSockRegistryEntriesByPurpose(vsock.Fileserver)
+	if err != nil {
+		return fmt.Errorf("failed to load existing file server vsock registry entries: %w", err)
+	}
+	for i, mount := range mc.Mounts {
+		var testVsock *vsock.HVSockRegistryEntry
+
+		// Check if there's an existing file server vsock entry that can be reused for the current mount.
+		if i < len(fileServerVsocks) {
+			testVsock = fileServerVsocks[i]
+		} else {
+			// If no existing vsock entry can be reused, a new one must be created.
+			// Creating a new HVSockRegistryEntry requires administrator privileges.
+			if !windows.HasAdminRights() {
+				return windows.ErrHypervRequiresAdmin
+			}
+			testVsock, err = vsock.NewHVSockRegistryEntry(vsock.Fileserver)
+			if err != nil {
+				return err
+			}
 		}
+
 		mount.VSockNumber = &testVsock.Port
 		logrus.Debugf("Going to share directory %s via 9p on vsock %d", mount.Source, testVsock.Port)
 	}