-
-
Notifications
You must be signed in to change notification settings - Fork 75
feat: add VZVmnetNetworkDeviceAttachment support (macOS 26.0)
#205
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.
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
base: main
Are you sure you want to change the base?
feat: add VZVmnetNetworkDeviceAttachment support (macOS 26.0)
#205
Conversation
VmnetNetworkDeviceAttachment support (macOS 26.0)VZVmnetNetworkDeviceAttachment support (macOS 26.0)
6617c8f to
6a1f741
Compare
Based on `VMNET_SHARED_MODE`, and `VMNET_HOST_MODE` ```yaml networks: - vzShared: true - vzHost: true ``` But, to sharing network between multiple VMs, `VZVmnetNetworkDeviceAttachment` requires VMs are launched by same process. It depends on Code-Hex/vz#205 Signed-off-by: Norio Nomura <[email protected]>
vmnet.go
Outdated
| const ( | ||
| HostMode VmnetMode = C.VMNET_HOST_MODE | ||
| SharedMode VmnetMode = C.VMNET_SHARED_MODE | ||
| // Deprecated: BridgedMode is not supported by NewVmnetNetworkConfiguration |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you know why not supported?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know. It's documented on:
https://developer.apple.com/documentation/vmnet/vmnet_network_configuration_create(_:_:)?language=objc
Parameters
mode
Shared mode or host-only mode.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why BridgedMode is Deprecated? Can you link to the docs about this?
Since it is not supported we should not include it, marking it as Deprecated look wrong.
|
This can be used by multiple processes like this:
|
5a7a116 to
72cc1d4
Compare
In this procedure, I confirmed that VMs launched from multiple processes can share networks with each other. 👍🏻 |
72cc1d4 to
9506cbd
Compare
Added unit test and |
I'll try this added xpc package with lima to make it work. Until then, it's a draft. |
7bf24c1 to
007c2a5
Compare
Based on `VMNET_SHARED_MODE`, and `VMNET_HOST_MODE` ```yaml networks: - vzShared: true - vzHost: true ``` But, to sharing network between multiple VMs, `VZVmnetNetworkDeviceAttachment` requires VMs are launched by same process. It depends on Code-Hex/vz#205 Signed-off-by: Norio Nomura <[email protected]>
aba95bd to
ba619f5
Compare
d3fad75 to
7a58378
Compare
Based on `VMNET_SHARED_MODE`, and `VMNET_HOST_MODE` ```yaml networks: - vzShared: true - vzHost: true ``` But, to sharing network between multiple VMs, `VZVmnetNetworkDeviceAttachment` requires VMs are launched by same process. It depends on Code-Hex/vz#205 Signed-off-by: Norio Nomura <[email protected]>
7a58378 to
33858c0
Compare
nirs
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did not review most of this change, just the stange part of about marking bridged mode as depracated.
vmnet.go
Outdated
| const ( | ||
| HostMode VmnetMode = C.VMNET_HOST_MODE | ||
| SharedMode VmnetMode = C.VMNET_SHARED_MODE | ||
| // Deprecated: BridgedMode is not supported by NewVmnetNetworkConfiguration |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why BridgedMode is Deprecated? Can you link to the docs about this?
Since it is not supported we should not include it, marking it as Deprecated look wrong.
f048f6e to
3b512d7
Compare
Based on `VMNET_SHARED_MODE`, and `VMNET_HOST_MODE` ```yaml networks: - vzShared: true - vzHost: true ``` But, to sharing network between multiple VMs, `VZVmnetNetworkDeviceAttachment` requires VMs are launched by same process. It depends on Code-Hex/vz#205 Signed-off-by: Norio Nomura <[email protected]>
b4f69f8 to
ebc8b36
Compare
Based on `VMNET_SHARED_MODE`, and `VMNET_HOST_MODE` ```yaml networks: - vzShared: true - vzHost: true ``` But, to sharing network between multiple VMs, `VZVmnetNetworkDeviceAttachment` requires VMs are launched by same process. It depends on Code-Hex/vz#205 Signed-off-by: Norio Nomura <[email protected]>
Based on `VMNET_SHARED_MODE`, and `VMNET_HOST_MODE` ```yaml networks: - vzShared: true - vzHost: true ``` But, to sharing network between multiple VMs, `VZVmnetNetworkDeviceAttachment` requires VMs are launched by same process. It depends on Code-Hex/vz#205 Signed-off-by: Norio Nomura <[email protected]>
Based on `VMNET_SHARED_MODE`, and `VMNET_HOST_MODE` ```yaml networks: - vzShared: true - vzHost: true ``` But, to sharing network between multiple VMs, `VZVmnetNetworkDeviceAttachment` requires VMs are launched by same process. It depends on Code-Hex/vz#205 Signed-off-by: Norio Nomura <[email protected]>
Based on `VMNET_SHARED_MODE`, and `VMNET_HOST_MODE` ```yaml networks: - vzShared: true - vzHost: true ``` But, to sharing network between multiple VMs, `VZVmnetNetworkDeviceAttachment` requires VMs are launched by same process. It depends on Code-Hex/vz#205 Signed-off-by: Norio Nomura <[email protected]>
|
@norio-nomura Thanks for creating your PR |
Code-Hex
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you very much for creating a great PR.
I don't think it's ready for review yet, but I'll review it in its current state.
- When providing it as a library, it is not desirable to create a
pkgdirectory and create detailed packages within it. Please remove thepkgdirectory and createvmnet,xpcdirectories at the same level asnetwork.go, such as vz/network.go, vz/vmnet, vz/xpc - Similar to the above, create a directory for the
osversionpackage under internal and moveosversion.goit to use themacOSAvailablefunction in thevmnetpackage. - Overall, the code seems to have a tendency to overlap with existing code (does it support generative AI?). It would be appreciated if you could align it as much as possible with the packages in the internal directory.
|
|
||
| // Object represents a generic XPC object. | ||
| // - https://developer.apple.com/documentation/xpc/xpc_object_t?language=objc | ||
| type Object interface { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is better to extend NSObject interface under internal/objc package.
Making this change, you should notice that most of the code defined in pkg/xpc/xpc_object.go is duplicated. It is desirable to use an NSObject interface as an argument rather than generics.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since objc is an internal package, it cannot be used to retrieve unsafe.Pointer on Lima, etc.
xpc and vmnet did not want to implement all APIs, but instead of exposing unsafe.Pointer, so they avoided completely imitating the NSObject interface.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@norio-nomura Thanks for your explanation. I do not wish users to interact with unsafe.Pointer as a feature provided by the library. The reason being that users may inadvertently utilise unintended library specifications, which are then treated as bugs requiring correction.
However, I understand your point, but I'd be very happy if we could find a solution that doesn't require exposing unsafe.Pointer.
| // size_t xpcDataGetBytes(void *object, void *buffer, size_t offset, size_t length); | ||
| // const void *xpcDataGetBytesPtr(void *object); | ||
| // size_t xpcDataGetLength(void *object); | ||
|
|
||
| // MARK: - Number objects |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove any parts that are not being used. They are noise.
The reason why it contains duplicate code is because it was written to work alone when this PR was not accepted. |
|
@norio-nomura I agree with the basic policy, so I would appreciate it if you could remove any duplicated codes. |
Signed-off-by: Norio Nomura <[email protected]>
7eace16 to
b9433cf
Compare
`VZVmnetNetworkDeviceAttachment` is an API that creates vmnet devices on VMs added in macOS 26. see: https://developer.apple.com/documentation/virtualization/vzvmnetnetworkdeviceattachment?language=objc It does not require the `com.apple.vm.networking` entitlement nor root privileges. `HostMode` and `SharedMode` are supported. In order for multiple VMs to communicate with each other in `SharedMode`, they must be started in the same executable and the same `VmnetNetwork` must be passed to `NewVmnetNetworkDeviceAttachment()` to create an attachment. This change adds: - `vz.VmnetNetworkDeviceAttachment` represents `VZVmnetNetworkDeviceAttachment` in Go - `vmnet` package to use vmnet APIs that added on macOS 26.0 - `Return` represents `vmnet_return_t` as `error` - `ErrSuccess`, `ErrFailure`, ... - `Mode` represents `operating_modes_t` - `HostMode`, `SharedMode` - `NetworkConfiguration` represents `vmnet_network_configuration_t` - `Network` represents `vmnet_network_ref` - `vz_test.TestVmnetSharedModeAllowsCommunicationBetweenMultipleVMs` - `vz_test.TestVmnetSharedModeWithConfiguringIPv4` Signed-off-by: Norio Nomura <[email protected]>
- Add `TestVmnetNetworkShareModeSharingOverXpc` to `vmnet_test.go` `TestVmnetNetworkShareModeSharingOverXpc` tests sharing `vmnet.Network` in `SharedMode` over XPC communication. This test registers test executable as an Mach service and launches it using `launchctl`. The launched Mach service provides `vmnet.Network` serialization to clients upon request, after booting a VM using the provided `vmnet.Network` to ensure the network is functional on the server side. The client boots VM using the provided `vmnet.Network` serialization. - Add `pkg/xpc` package that providing `<xpc/xpc.h>` APIs to support implementing Mach service server and client Signed-off-by: Norio Nomura <[email protected]>
c62cc26 to
2d5a3ed
Compare
- .github/workflows/compile.yml: Extend test job's timeout from 3m to 5m - Makefile: Extend test target's timeout from 2m to 4m Signed-off-by: Norio Nomura <[email protected]>
2d5a3ed to
0ce4c32
Compare
Signed-off-by: Norio Nomura <[email protected]>
|
I'll add:
|
Signed-off-by: Norio Nomura <[email protected]>
| // This is only supported on macOS 26 and newer, error will be returned on older versions. | ||
| // - https://developer.apple.com/documentation/vmnet/vmnet_network_create_with_serialization(_:_:)?language=objc | ||
| func NewNetworkWithSerialization(serialization unsafe.Pointer) (*Network, error) { | ||
| func NewNetworkWithSerialization(serialization xpc.Object) (*Network, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With this change, if I have xpc_object_t I got from a xpc service, I need to wrap it like this, right?
// serialization is xpc_object_t
network, err := NewNetworkWithSerialization(xpc.NewObject(serialization))So we did not really got rid of the unsafe.Pointer - I think we cannot avoid this since this is the only way to integrate with code retuning xpc_object_t. The vmnet interface vment_network_create_with_serialization() accepts xpc_object_t so we cannot implement it without accepting xpc_object.
But I think this wrapping makes sense to minimize the scope of a raw xpc_object_t.
|
I tested the latest version, it works for me. |
feat: add
VZVmnetNetworkDeviceAttachmentsupport (macOS 26.0)VZVmnetNetworkDeviceAttachmentis an API that creates vmnet devices on VMs added in macOS 26.see: https://developer.apple.com/documentation/virtualization/vzvmnetnetworkdeviceattachment?language=objc
It does not require the
com.apple.vm.networkingentitlement nor root privileges.HostModeandSharedModeare supported.In order for multiple VMs to communicate with each other in
SharedMode, they must be started in the same executable and the sameVmnetNetworkmust be passed toNewVmnetNetworkDeviceAttachment()to create an attachment.This change adds:
vz.VmnetNetworkDeviceAttachmentrepresentsVZVmnetNetworkDeviceAttachmentin Govmnetpackage to use vmnet APIs that added on macOS 26.0Returnrepresentsvmnet_return_taserrorErrSuccess,ErrFailure, ...Moderepresentsoperating_modes_tHostMode,SharedModeNetworkConfigurationrepresentsvmnet_network_configuration_tNetworkrepresentsvmnet_network_refvz_test.TestVmnetSharedModeAllowsCommunicationBetweenMultipleVMsvz_test.TestVmnetSharedModeWithConfiguringIPv4pkg/xpcpackage that providing<xpc/xpc.h>APIs to support implementing Mach service server and clientvz_test.TestVmnetNetworkShareModeSharingOverXpcTestVmnetNetworkShareModeSharingOverXpctests sharingvmnet.NetworkinSharedModeover XPC communication.This test registers test executable as an Mach service and launches it using
launchctl.The launched Mach service provides
vmnet.Networkserialization to clients upon request, after bootinga VM using the provided
vmnet.Networkto ensure the network is functional on the server side.The client boots VM using the provided
vmnet.Networkserialization.Which issue(s) this PR fixes:
Mentioned in #198 (comment)