configure | 2 +- net/clients.h | 6 + net/meson.build | 1 + net/net.c | 3 + net/vmnet-macos.c | 437 ++++++++++++++++++++++++++++++++++++++++++++++ qapi/net.json | 120 ++++++++++++- qemu-options.hx | 9 + 7 files changed, 575 insertions(+), 3 deletions(-) create mode 100644 net/vmnet-macos.c
From: Phillip Tennen <phillip@axleos.com>
This patch implements a new netdev device, reachable via -netdev
vmnet-macos, that’s backed by macOS’s vmnet framework.
The vmnet framework provides native bridging support, and its usage in
this patch is intended as a replacement for attempts to use a tap device
via the tuntaposx kernel extension. Notably, the tap/tuntaposx approach
never would have worked in the first place, as QEMU interacts with the
tap device via poll(), and macOS does not support polling device files.
vmnet requires either a special entitlement, granted via a provisioning
profile, or root access. Otherwise attempts to create the virtual
interface will fail with a “generic error” status code. QEMU may not
currently be signed with an entitlement granted in a provisioning
profile, as this would necessitate pre-signed binary build distribution,
rather than source-code distribution. As such, using this netdev
currently requires that qemu be run with root access. I’ve opened a
feedback report with Apple to allow the use of the relevant entitlement
with this use case:
https://openradar.appspot.com/radar?id=5007417364447232
vmnet offers three operating modes, all of which are supported by this
patch via the “mode=host|shared|bridge” option:
* "Host" mode: Allows the vmnet interface to communicate with other
* vmnet
interfaces that are in host mode and also with the native host.
* "Shared" mode: Allows traffic originating from the vmnet interface to
reach the Internet through a NAT. The vmnet interface can also
communicate with the native host.
* "Bridged" mode: Bridges the vmnet interface with a physical network
interface.
Each of these modes also provide some extra configuration that’s
supported by this patch:
* "Bridged" mode: The user may specify the physical interface to bridge
with. Defaults to en0.
* "Host" mode / "Shared" mode: The user may specify the DHCP range and
subnet. Allocated by vmnet if not provided.
vmnet also offers some extra configuration options that are not
supported by this patch:
* Enable isolation from other VMs using vmnet
* Port forwarding rules
* Enabling TCP segmentation offload
* Only applicable in "shared" mode: specifying the NAT IPv6 prefix
* Only available in "host" mode: specifying the IP address for the VM
within an isolated network
Note that this patch requires macOS 10.15 as a minimum, as this is when
bridging support was implemented in vmnet.framework.
Signed-off-by: Phillip Tennen <phillip@axleos.com>
---
configure | 2 +-
net/clients.h | 6 +
net/meson.build | 1 +
net/net.c | 3 +
net/vmnet-macos.c | 437 ++++++++++++++++++++++++++++++++++++++++++++++
qapi/net.json | 120 ++++++++++++-
qemu-options.hx | 9 +
7 files changed, 575 insertions(+), 3 deletions(-)
create mode 100644 net/vmnet-macos.c
diff --git a/configure b/configure
index 4afd22bdf5..f449198db1 100755
--- a/configure
+++ b/configure
@@ -778,7 +778,7 @@ Darwin)
fi
audio_drv_list="coreaudio try-sdl"
audio_possible_drivers="coreaudio sdl"
- QEMU_LDFLAGS="-framework CoreFoundation -framework IOKit $QEMU_LDFLAGS"
+ QEMU_LDFLAGS="-framework CoreFoundation -framework IOKit -framework vmnet $QEMU_LDFLAGS"
# Disable attempts to use ObjectiveC features in os/object.h since they
# won't work when we're compiling with gcc as a C compiler.
QEMU_CFLAGS="-DOS_OBJECT_USE_OBJC=0 $QEMU_CFLAGS"
diff --git a/net/clients.h b/net/clients.h
index 92f9b59aed..463a9b2f67 100644
--- a/net/clients.h
+++ b/net/clients.h
@@ -63,4 +63,10 @@ int net_init_vhost_user(const Netdev *netdev, const char *name,
int net_init_vhost_vdpa(const Netdev *netdev, const char *name,
NetClientState *peer, Error **errp);
+
+#ifdef CONFIG_DARWIN
+int net_init_vmnet_macos(const Netdev *netdev, const char *name,
+ NetClientState *peer, Error **errp);
+#endif
+
#endif /* QEMU_NET_CLIENTS_H */
diff --git a/net/meson.build b/net/meson.build
index 1076b0a7ab..8c7c32f775 100644
--- a/net/meson.build
+++ b/net/meson.build
@@ -37,5 +37,6 @@ endif
softmmu_ss.add(when: 'CONFIG_POSIX', if_true: files(tap_posix))
softmmu_ss.add(when: 'CONFIG_WIN32', if_true: files('tap-win32.c'))
softmmu_ss.add(when: 'CONFIG_VHOST_NET_VDPA', if_true: files('vhost-vdpa.c'))
+softmmu_ss.add(when: 'CONFIG_DARWIN', if_true: files('vmnet-macos.c'))
subdir('can')
diff --git a/net/net.c b/net/net.c
index c1cd9c75f6..e68a410a89 100644
--- a/net/net.c
+++ b/net/net.c
@@ -977,6 +977,9 @@ static int (* const net_client_init_fun[NET_CLIENT_DRIVER__MAX])(
#ifdef CONFIG_L2TPV3
[NET_CLIENT_DRIVER_L2TPV3] = net_init_l2tpv3,
#endif
+#ifdef CONFIG_DARWIN
+ [NET_CLIENT_DRIVER_VMNET_MACOS] = net_init_vmnet_macos,
+#endif
};
diff --git a/net/vmnet-macos.c b/net/vmnet-macos.c
new file mode 100644
index 0000000000..1b713de05c
--- /dev/null
+++ b/net/vmnet-macos.c
@@ -0,0 +1,437 @@
+/*
+ * vmnet.framework backed netdev for macOS 10.15+ hosts
+ *
+ * Copyright (c) 2021 Phillip Tennen <phillip@axleos.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+#include "qemu/osdep.h"
+#include "qemu/main-loop.h"
+#include "qemu/error-report.h"
+#include "qapi/qapi-types-net.h"
+#include "net/net.h"
+/* macOS vmnet framework header */
+#include <vmnet/vmnet.h>
+
+typedef struct vmnet_state {
+ NetClientState nc;
+ interface_ref vmnet_iface_ref;
+ /* Switched on after vmnet informs us that the interface has started */
+ bool link_up;
+ /*
+ * If qemu_send_packet_async returns 0, this is switched off until our
+ * delivery callback is invoked
+ */
+ bool qemu_ready_to_receive;
+} vmnet_state_t;
+
+int net_init_vmnet_macos(const Netdev *netdev, const char *name,
+ NetClientState *peer, Error **errp);
+
+static const char *_vmnet_status_repr(vmnet_return_t status)
+{
+ switch (status) {
+ case VMNET_SUCCESS:
+ return "success";
+ case VMNET_FAILURE:
+ return "generic failure";
+ case VMNET_MEM_FAILURE:
+ return "out of memory";
+ case VMNET_INVALID_ARGUMENT:
+ return "invalid argument";
+ case VMNET_SETUP_INCOMPLETE:
+ return "setup is incomplete";
+ case VMNET_INVALID_ACCESS:
+ return "insufficient permissions";
+ case VMNET_PACKET_TOO_BIG:
+ return "packet size exceeds MTU";
+ case VMNET_BUFFER_EXHAUSTED:
+ return "kernel buffers temporarily exhausted";
+ case VMNET_TOO_MANY_PACKETS:
+ return "number of packets exceeds system limit";
+ case VMNET_SHARING_SERVICE_BUSY:
+ return "sharing service busy";
+ default:
+ return "unknown status code";
+ }
+}
+
+static operating_modes_t _vmnet_operating_mode_enum_compat(
+ VmnetOperatingMode mode)
+{
+ switch (mode) {
+ case VMNET_OPERATING_MODE_HOST:
+ return VMNET_HOST_MODE;
+ case VMNET_OPERATING_MODE_SHARED:
+ return VMNET_SHARED_MODE;
+ case VMNET_OPERATING_MODE_BRIDGED:
+ return VMNET_BRIDGED_MODE;
+ default:
+ /* Should never happen as the modes are parsed before we get here */
+ assert(false);
+ }
+}
+
+static bool vmnet_can_receive(NetClientState *nc)
+{
+ vmnet_state_t *s = DO_UPCAST(vmnet_state_t, nc, nc);
+ return s->link_up;
+}
+
+static ssize_t vmnet_receive_iov(NetClientState *nc,
+ const struct iovec *iovs,
+ int iovcnt)
+{
+ vmnet_state_t *s = DO_UPCAST(vmnet_state_t, nc, nc);
+
+ /* Combine the provided iovs into a single vmnet packet */
+ struct vmpktdesc *packet = g_new0(struct vmpktdesc, 1);
+ packet->vm_pkt_iov = g_new0(struct iovec, iovcnt);
+ memcpy(packet->vm_pkt_iov, iovs, sizeof(struct iovec) * iovcnt);
+ packet->vm_pkt_iovcnt = iovcnt;
+ packet->vm_flags = 0;
+
+ /* Figure out the packet size by iterating the iov's */
+ for (int i = 0; i < iovcnt; i++) {
+ const struct iovec *iov = iovs + i;
+ packet->vm_pkt_size += iov->iov_len;
+ }
+
+ /* Finally, write the packet to the vmnet interface */
+ int packet_count = 1;
+ vmnet_return_t result = vmnet_write(s->vmnet_iface_ref, packet,
+ &packet_count);
+ if (result != VMNET_SUCCESS || packet_count != 1) {
+ error_printf("Failed to send packet to host: %s\n",
+ _vmnet_status_repr(result));
+ }
+ ssize_t wrote_bytes = packet->vm_pkt_size;
+ g_free(packet->vm_pkt_iov);
+ g_free(packet);
+ return wrote_bytes;
+}
+
+static void vmnet_send_completed(NetClientState *nc, ssize_t len)
+{
+ vmnet_state_t *vmnet_client_state = DO_UPCAST(vmnet_state_t, nc, nc);
+ /* Ready to receive more packets! */
+ vmnet_client_state->qemu_ready_to_receive = true;
+}
+
+static NetClientInfo net_vmnet_macos_info = {
+ .type = NET_CLIENT_DRIVER_VMNET_MACOS,
+ .size = sizeof(vmnet_state_t),
+ .receive_iov = vmnet_receive_iov,
+ .can_receive = vmnet_can_receive,
+};
+
+static bool _validate_ifname_is_valid_bridge_target(const char *ifname)
+{
+ /* Iterate available bridge interfaces, ensure the provided one is valid */
+ xpc_object_t bridge_interfaces = vmnet_copy_shared_interface_list();
+ bool failed_to_match_iface_name = xpc_array_apply(
+ bridge_interfaces,
+ ^bool(size_t index, xpc_object_t _Nonnull value) {
+ if (!strcmp(xpc_string_get_string_ptr(value), ifname)) {
+ /* The interface name is valid! Stop iterating */
+ return false;
+ }
+ return true;
+ });
+
+ if (failed_to_match_iface_name) {
+ error_printf("Invalid bridge interface name provided: %s\n", ifname);
+ error_printf("Valid bridge interfaces:\n");
+ xpc_array_apply(
+ vmnet_copy_shared_interface_list(),
+ ^bool(size_t index, xpc_object_t _Nonnull value) {
+ error_printf("\t%s\n", xpc_string_get_string_ptr(value));
+ /* Keep iterating */
+ return true;
+ });
+ exit(1);
+ return false;
+ }
+
+ return true;
+}
+
+static xpc_object_t _construct_vmnet_interface_description(
+ const NetdevVmnetModeOptions *vmnet_opts)
+{
+ operating_modes_t mode = _vmnet_operating_mode_enum_compat(
+ vmnet_opts->mode);
+
+ /* Validate options */
+ if (mode == VMNET_HOST_MODE || mode == VMNET_SHARED_MODE) {
+ NetdevVmnetModeOptionsHostOrShared mode_opts = vmnet_opts->u.host;
+ /* If one DHCP parameter is configured, all 3 are required */
+ if (mode_opts.has_dhcp_start_address ||
+ mode_opts.has_dhcp_end_address ||
+ mode_opts.has_dhcp_subnet_mask) {
+ if (!(mode_opts.has_dhcp_start_address &&
+ mode_opts.has_dhcp_end_address &&
+ mode_opts.has_dhcp_subnet_mask)) {
+ error_printf("Incomplete DHCP configuration provided\n");
+ exit(1);
+ }
+ }
+ } else if (mode == VMNET_BRIDGED_MODE) {
+ /* Nothing to validate */
+ } else {
+ error_printf("Unknown vmnet mode %d\n", mode);
+ exit(1);
+ }
+
+ xpc_object_t interface_desc = xpc_dictionary_create(NULL, NULL, 0);
+ xpc_dictionary_set_uint64(
+ interface_desc,
+ vmnet_operation_mode_key,
+ mode
+ );
+
+ if (mode == VMNET_BRIDGED_MODE) {
+ /*
+ * Configure the provided physical interface to act
+ * as a bridge with QEMU
+ */
+ NetdevVmnetModeOptionsBridged mode_opts = vmnet_opts->u.bridged;
+ /* Bridge with en0 by default */
+ const char *physical_ifname = mode_opts.has_ifname ? mode_opts.ifname :
+ "en0";
+ _validate_ifname_is_valid_bridge_target(physical_ifname);
+ xpc_dictionary_set_string(interface_desc,
+ vmnet_shared_interface_name_key,
+ physical_ifname);
+ } else if (mode == VMNET_HOST_MODE || mode == VMNET_SHARED_MODE) {
+ /* Pass the DHCP configuration to vmnet, if the user provided one */
+ NetdevVmnetModeOptionsHostOrShared mode_opts = vmnet_opts->u.host;
+ if (mode_opts.has_dhcp_start_address) {
+ /* All DHCP arguments are available, as per the checks above */
+ xpc_dictionary_set_string(interface_desc,
+ vmnet_start_address_key,
+ mode_opts.dhcp_start_address);
+ xpc_dictionary_set_string(interface_desc,
+ vmnet_end_address_key,
+ mode_opts.dhcp_end_address);
+ xpc_dictionary_set_string(interface_desc,
+ vmnet_subnet_mask_key,
+ mode_opts.dhcp_subnet_mask);
+ }
+ }
+
+ return interface_desc;
+}
+
+int net_init_vmnet_macos(const Netdev *netdev, const char *name,
+ NetClientState *peer, Error **errp)
+{
+ assert(netdev->type == NET_CLIENT_DRIVER_VMNET_MACOS);
+
+ NetdevVmnetModeOptions *vmnet_opts = netdev->u.vmnet_macos.options;
+ xpc_object_t iface_desc = _construct_vmnet_interface_description(vmnet_opts);
+
+ NetClientState *nc = qemu_new_net_client(&net_vmnet_macos_info, peer,
+ "vmnet", name);
+ vmnet_state_t *vmnet_client_state = DO_UPCAST(vmnet_state_t, nc, nc);
+
+ dispatch_queue_t vmnet_dispatch_queue = dispatch_queue_create(
+ "org.qemu.vmnet.iface_queue",
+ DISPATCH_QUEUE_SERIAL
+ );
+
+ __block vmnet_return_t vmnet_start_status = 0;
+ __block uint64_t vmnet_iface_mtu = 0;
+ __block uint64_t vmnet_max_packet_size = 0;
+ __block const char *vmnet_mac_address = NULL;
+ /*
+ * We can't refer to an array type directly within a block,
+ * so hold a pointer instead.
+ */
+ uuid_string_t vmnet_iface_uuid = {0};
+ __block uuid_string_t *vmnet_iface_uuid_ptr = &vmnet_iface_uuid;
+ /* These are only provided in VMNET_HOST_MODE and VMNET_SHARED_MODE */
+ bool vmnet_provides_dhcp_info = (
+ vmnet_opts->mode == VMNET_OPERATING_MODE_HOST ||
+ vmnet_opts->mode == VMNET_OPERATING_MODE_SHARED);
+ __block const char *vmnet_subnet_mask = NULL;
+ __block const char *vmnet_dhcp_range_start = NULL;
+ __block const char *vmnet_dhcp_range_end = NULL;
+
+ /* Create the vmnet interface */
+ dispatch_semaphore_t vmnet_iface_sem = dispatch_semaphore_create(0);
+ interface_ref vmnet_iface_ref = vmnet_start_interface(
+ iface_desc,
+ vmnet_dispatch_queue,
+ ^(vmnet_return_t status, xpc_object_t _Nullable interface_param) {
+ vmnet_start_status = status;
+ if (vmnet_start_status != VMNET_SUCCESS || !interface_param) {
+ /* Early return if the interface couldn't be started */
+ dispatch_semaphore_signal(vmnet_iface_sem);
+ return;
+ }
+
+ /*
+ * Read the configuration that vmnet provided us.
+ * The provided dictionary is owned by XPC and may be freed
+ * shortly after this block's execution.
+ * So, copy data buffers now.
+ */
+ vmnet_iface_mtu = xpc_dictionary_get_uint64(
+ interface_param,
+ vmnet_mtu_key
+ );
+ vmnet_max_packet_size = xpc_dictionary_get_uint64(
+ interface_param,
+ vmnet_max_packet_size_key
+ );
+ vmnet_mac_address = strdup(xpc_dictionary_get_string(
+ interface_param,
+ vmnet_mac_address_key
+ ));
+
+ const uint8_t *iface_uuid = xpc_dictionary_get_uuid(
+ interface_param,
+ vmnet_interface_id_key
+ );
+ uuid_unparse_upper(iface_uuid, *vmnet_iface_uuid_ptr);
+
+ /* If we're in a mode that provides DHCP info, read it out now */
+ if (vmnet_provides_dhcp_info) {
+ vmnet_dhcp_range_start = strdup(xpc_dictionary_get_string(
+ interface_param,
+ vmnet_start_address_key
+ ));
+ vmnet_dhcp_range_end = strdup(xpc_dictionary_get_string(
+ interface_param,
+ vmnet_end_address_key
+ ));
+ vmnet_subnet_mask = strdup(xpc_dictionary_get_string(
+ interface_param,
+ vmnet_subnet_mask_key
+ ));
+ }
+ dispatch_semaphore_signal(vmnet_iface_sem);
+ });
+
+ /* And block until we receive a response from vmnet */
+ dispatch_semaphore_wait(vmnet_iface_sem, DISPATCH_TIME_FOREVER);
+
+ /* Did we manage to start the interface? */
+ if (vmnet_start_status != VMNET_SUCCESS || !vmnet_iface_ref) {
+ error_printf("Failed to start interface: %s\n",
+ _vmnet_status_repr(vmnet_start_status));
+ if (vmnet_start_status == VMNET_FAILURE) {
+ error_printf("Hint: vmnet requires running with root access\n");
+ }
+ return -1;
+ }
+
+ info_report("Started vmnet interface with configuration:");
+ info_report("MTU: %llu", vmnet_iface_mtu);
+ info_report("Max packet size: %llu", vmnet_max_packet_size);
+ info_report("MAC: %s", vmnet_mac_address);
+ if (vmnet_provides_dhcp_info) {
+ info_report("DHCP IPv4 start: %s", vmnet_dhcp_range_start);
+ info_report("DHCP IPv4 end: %s", vmnet_dhcp_range_end);
+ info_report("IPv4 subnet mask: %s", vmnet_subnet_mask);
+ }
+ info_report("UUID: %s", vmnet_iface_uuid);
+
+ /* The interface is up! Set a block to run when packets are received */
+ vmnet_client_state->vmnet_iface_ref = vmnet_iface_ref;
+ vmnet_return_t event_cb_stat = vmnet_interface_set_event_callback(
+ vmnet_iface_ref,
+ VMNET_INTERFACE_PACKETS_AVAILABLE,
+ vmnet_dispatch_queue,
+ ^(interface_event_t event_mask, xpc_object_t _Nonnull event) {
+ if (event_mask != VMNET_INTERFACE_PACKETS_AVAILABLE) {
+ error_printf("Unknown vmnet interface event 0x%08x\n", event_mask);
+ return;
+ }
+
+ /* If we're unable to handle more packets now, drop this packet */
+ if (!vmnet_client_state->qemu_ready_to_receive) {
+ return;
+ }
+
+ /*
+ * TODO(Phillip Tennen <phillip@axleos.com>): There may be more than
+ * one packet available.
+ * As an optimization, we could read
+ * vmnet_estimated_packets_available_key packets now.
+ */
+ char *packet_buf = g_malloc0(vmnet_max_packet_size);
+ struct iovec *iov = g_new0(struct iovec, 1);
+ iov->iov_base = packet_buf;
+ iov->iov_len = vmnet_max_packet_size;
+
+ int pktcnt = 1;
+ struct vmpktdesc *v = g_new0(struct vmpktdesc, pktcnt);
+ v->vm_pkt_size = vmnet_max_packet_size;
+ v->vm_pkt_iov = iov;
+ v->vm_pkt_iovcnt = 1;
+ v->vm_flags = 0;
+
+ vmnet_return_t result = vmnet_read(vmnet_iface_ref, v, &pktcnt);
+ if (result != VMNET_SUCCESS) {
+ error_printf("Failed to read packet from host: %s\n",
+ _vmnet_status_repr(result));
+ }
+
+ /* Ensure we read exactly one packet */
+ assert(pktcnt == 1);
+
+ dispatch_async(dispatch_get_main_queue(), ^{
+ qemu_mutex_lock_iothread();
+
+ /*
+ * Deliver the packet to the guest
+ * If the delivery succeeded synchronously, this returns the length
+ * of the sent packet.
+ */
+ if (qemu_send_packet_async(nc, iov->iov_base,
+ v->vm_pkt_size,
+ vmnet_send_completed) == 0) {
+ vmnet_client_state->qemu_ready_to_receive = false;
+ }
+
+ /*
+ * It's safe to free the packet buffers.
+ * Even if delivery needs to wait, qemu_net_queue_append copies
+ * the packet buffer.
+ */
+ g_free(v);
+ g_free(iov);
+ g_free(packet_buf);
+
+ qemu_mutex_unlock_iothread();
+ });
+ });
+
+ /* Did we manage to set an event callback? */
+ if (event_cb_stat != VMNET_SUCCESS) {
+ error_printf("Failed to set up a callback to receive packets: %s\n",
+ _vmnet_status_repr(vmnet_start_status));
+ exit(1);
+ }
+
+ /* We're now ready to receive packets */
+ vmnet_client_state->qemu_ready_to_receive = true;
+ vmnet_client_state->link_up = true;
+
+ /* Include DHCP info if we're in a relevant mode */
+ if (vmnet_provides_dhcp_info) {
+ snprintf(nc->info_str, sizeof(nc->info_str),
+ "dhcp_start=%s,dhcp_end=%s,mask=%s",
+ vmnet_dhcp_range_start, vmnet_dhcp_range_end,
+ vmnet_subnet_mask);
+ } else {
+ snprintf(nc->info_str, sizeof(nc->info_str),
+ "mac=%s", vmnet_mac_address);
+ }
+
+ return 0;
+}
diff --git a/qapi/net.json b/qapi/net.json
index c31748c87f..e4d4143243 100644
--- a/qapi/net.json
+++ b/qapi/net.json
@@ -450,6 +450,115 @@
'*vhostdev': 'str',
'*queues': 'int' } }
+##
+# @VmnetOperatingMode:
+#
+# The operating modes in which a vmnet netdev can run
+# Only available on macOS
+#
+# @host: the guest may communicate with the host
+# and other guest network interfaces
+#
+# @shared: the guest may reach the Internet through a NAT,
+# and may communicate with the host and other guest
+# network interfaces
+#
+# @bridged: the guest's traffic is bridged with a
+# physical network interface of the host
+#
+# Since: 6.0
+##
+{ 'enum': 'VmnetOperatingMode',
+ 'data': [ 'host', 'shared', 'bridged' ],
+ 'if': 'defined(CONFIG_DARWIN)' }
+
+##
+# @NetdevVmnetModeOptionsBridged:
+#
+# Options for the vmnet-macos netdev
+# that are only available in 'bridged' mode
+# Only available on macOS
+#
+# @ifname: the physical network interface to bridge with
+# (defaults to en0 if not specified)
+#
+# Since: 6.0
+##
+{ 'struct': 'NetdevVmnetModeOptionsBridged',
+ 'data': { '*ifname': 'str' },
+ 'if': 'defined(CONFIG_DARWIN)' }
+
+##
+# @NetdevVmnetModeOptionsHostOrShared:
+#
+# Options for the vmnet-macos netdev
+# that are only available in 'host' or 'shared' mode
+# Only available on macOS
+#
+# @dhcp-start-address: the gateway address to use for the interface.
+# The range to dhcp_end_address is placed in the DHCP pool.
+# (only valid with mode=host|shared)
+# (must be specified with dhcp-end-address and
+# dhcp-subnet-mask)
+# (allocated automatically if unset)
+#
+# @dhcp-end-address: the DHCP IPv4 range end address to use for the interface.
+# (only valid with mode=host|shared)
+# (must be specified with dhcp-start-address and
+# dhcp-subnet-mask)
+# (allocated automatically if unset)
+#
+# @dhcp-subnet-mask: the IPv4 subnet mask (string) to use on the interface.
+# (only valid with mode=host|shared)
+# (must be specified with dhcp-start-address and
+# dhcp-end-address)
+# (allocated automatically if unset)
+#
+# Since: 6.0
+##
+{ 'struct': 'NetdevVmnetModeOptionsHostOrShared',
+ 'data': {
+ '*dhcp-start-address': 'str' ,
+ '*dhcp-end-address': 'str',
+ '*dhcp-subnet-mask': 'str' },
+ 'if': 'defined(CONFIG_DARWIN)' }
+
+##
+# @NetdevVmnetModeOptions:
+#
+# Options specific to different operating modes of a vmnet netdev
+# Only available on macOS
+#
+# @mode: the operating mode vmnet should run in
+#
+# Since: 6.0
+##
+{ 'union': 'NetdevVmnetModeOptions',
+ 'base': { 'mode': 'VmnetOperatingMode' },
+ 'discriminator': 'mode',
+ 'data': {
+ 'bridged': 'NetdevVmnetModeOptionsBridged',
+ 'host': 'NetdevVmnetModeOptionsHostOrShared',
+ 'shared': 'NetdevVmnetModeOptionsHostOrShared' },
+ 'if': 'defined(CONFIG_DARWIN)' }
+
+##
+# @NetdevVmnetOptions:
+#
+# vmnet network backend
+# Only available on macOS
+#
+# @options: a structure specifying the mode and mode-specific options
+# (once QAPI supports a union type as a branch to another union type,
+# this structure can be changed to a union, and the contents of
+# NetdevVmnetModeOptions moved here)
+#
+# Since: 6.0
+##
+{ 'struct': 'NetdevVmnetOptions',
+ 'data': {'options': 'NetdevVmnetModeOptions' },
+ 'if': 'defined(CONFIG_DARWIN)' }
+
##
# @NetClientDriver:
#
@@ -458,10 +567,13 @@
# Since: 2.7
#
# @vhost-vdpa since 5.1
+#
+# @vmnet-macos since 6.0 (only available on macOS)
##
{ 'enum': 'NetClientDriver',
'data': [ 'none', 'nic', 'user', 'tap', 'l2tpv3', 'socket', 'vde',
- 'bridge', 'hubport', 'netmap', 'vhost-user', 'vhost-vdpa' ] }
+ 'bridge', 'hubport', 'netmap', 'vhost-user', 'vhost-vdpa',
+ { 'name': 'vmnet-macos', 'if': 'defined(CONFIG_DARWIN)' } ] }
##
# @Netdev:
@@ -475,6 +587,8 @@
# Since: 1.2
#
# 'l2tpv3' - since 2.1
+#
+# 'vmnet-macos' since 6.0 (only available on macOS)
##
{ 'union': 'Netdev',
'base': { 'id': 'str', 'type': 'NetClientDriver' },
@@ -490,7 +604,9 @@
'hubport': 'NetdevHubPortOptions',
'netmap': 'NetdevNetmapOptions',
'vhost-user': 'NetdevVhostUserOptions',
- 'vhost-vdpa': 'NetdevVhostVDPAOptions' } }
+ 'vhost-vdpa': 'NetdevVhostVDPAOptions',
+ 'vmnet-macos': { 'type': 'NetdevVmnetOptions',
+ 'if': 'defined(CONFIG_DARWIN)' } } }
##
# @NetFilterDirection:
diff --git a/qemu-options.hx b/qemu-options.hx
index 9172d51659..ec6b40b079 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2483,6 +2483,15 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
#ifdef __linux__
"-netdev vhost-vdpa,id=str,vhostdev=/path/to/dev\n"
" configure a vhost-vdpa network,Establish a vhost-vdpa netdev\n"
+#endif
+#ifdef CONFIG_DARWIN
+ "-netdev vmnet-macos,id=str,mode=bridged[,ifname=ifname]\n"
+ " configure a macOS-provided vmnet network in \"physical interface bridge\" mode\n"
+ " the physical interface to bridge with defaults to en0 if unspecified\n"
+ "-netdev vmnet-macos,id=str,mode=host|shared\n"
+ " [,dhcp_start_address=addr,dhcp_end_address=addr,dhcp_subnet_mask=mask]\n"
+ " configure a macOS-provided vmnet network in \"host\" or \"shared\" mode\n"
+ " the DHCP configuration will be set automatically if unspecified\n"
#endif
"-netdev hubport,id=str,hubid=n[,netdev=nd]\n"
" configure a hub port on the hub with ID 'n'\n", QEMU_ARCH_ALL)
--
2.24.3 (Apple Git-128)
On Fri, Feb 5, 2021 at 5:54 PM <phillip.ennen@gmail.com> wrote:
>
> From: Phillip Tennen <phillip@axleos.com>
>
> This patch implements a new netdev device, reachable via -netdev
> vmnet-macos, that’s backed by macOS’s vmnet framework.
>
> The vmnet framework provides native bridging support, and its usage in
> this patch is intended as a replacement for attempts to use a tap device
> via the tuntaposx kernel extension. Notably, the tap/tuntaposx approach
> never would have worked in the first place, as QEMU interacts with the
> tap device via poll(), and macOS does not support polling device files.
>
> vmnet requires either a special entitlement, granted via a provisioning
> profile, or root access. Otherwise attempts to create the virtual
> interface will fail with a “generic error” status code. QEMU may not
> currently be signed with an entitlement granted in a provisioning
> profile, as this would necessitate pre-signed binary build distribution,
> rather than source-code distribution. As such, using this netdev
> currently requires that qemu be run with root access. I’ve opened a
> feedback report with Apple to allow the use of the relevant entitlement
> with this use case:
> https://openradar.appspot.com/radar?id=5007417364447232
>
> vmnet offers three operating modes, all of which are supported by this
> patch via the “mode=host|shared|bridge” option:
>
> * "Host" mode: Allows the vmnet interface to communicate with other
> * vmnet
> interfaces that are in host mode and also with the native host.
> * "Shared" mode: Allows traffic originating from the vmnet interface to
> reach the Internet through a NAT. The vmnet interface can also
> communicate with the native host.
> * "Bridged" mode: Bridges the vmnet interface with a physical network
> interface.
>
> Each of these modes also provide some extra configuration that’s
> supported by this patch:
>
> * "Bridged" mode: The user may specify the physical interface to bridge
> with. Defaults to en0.
> * "Host" mode / "Shared" mode: The user may specify the DHCP range and
> subnet. Allocated by vmnet if not provided.
>
> vmnet also offers some extra configuration options that are not
> supported by this patch:
>
> * Enable isolation from other VMs using vmnet
> * Port forwarding rules
> * Enabling TCP segmentation offload
> * Only applicable in "shared" mode: specifying the NAT IPv6 prefix
> * Only available in "host" mode: specifying the IP address for the VM
> within an isolated network
>
> Note that this patch requires macOS 10.15 as a minimum, as this is when
> bridging support was implemented in vmnet.framework.
>
> Signed-off-by: Phillip Tennen <phillip@axleos.com>
>
Hi Phillip,
Thank you very much for this patch. As you wrote it should apply from
10.15 upwards. I have no problem building on Big Sur, but Catalina
stumbles. See below. Latest Xcode installed, other requirements
installed through brew.
Thanks for looking into this,
Best,
Howard
[1181/2135] Compiling C object libcommon.fa.p/net_vmnet-macos.c.o
FAILED: libcommon.fa.p/net_vmnet-macos.c.o
cc -Ilibcommon.fa.p -I. -I.. -I../capstone/include/capstone
-I../dtc/libfdt -I../slirp -I../slirp/src -Iqapi -Itrace -Iui
-Iui/shader -I/usr/local/Cellar/libffi/3.3_2/include
-I/usr/local/Cellar/glib/2.66.6/include
-I/usr/local/Cellar/glib/2.66.6/include/glib-2.0
-I/usr/local/Cellar/glib/2.66.6/lib/glib-2.0/include
-I/usr/local/opt/gettext/include -I/usr/local/Cellar/pcre/8.44/include
-I/usr/local/Cellar/glib/2.66.6/include/gio-unix-2.0
-I/usr/local/Cellar/libusb/1.0.24/include/libusb-1.0
-I/usr/local/Cellar/pixman/0.40.0/include/pixman-1 -Xclang
-fcolor-diagnostics -pipe -Wall -Winvalid-pch -std=gnu99 -O2 -g
-iquote . -iquote /Users/hsp/src/qemu-master -iquote
/Users/hsp/src/qemu-master/include -iquote
/Users/hsp/src/qemu-master/disas/libvixl -iquote
/Users/hsp/src/qemu-master/tcg/i386 -iquote
/Users/hsp/src/qemu-master/accel/tcg -m64 -mcx16
-DOS_OBJECT_USE_OBJC=0 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64
-D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wundef
-Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common
-fwrapv -Wold-style-definition -Wtype-limits -Wformat-security
-Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body
-Wnested-externs -Wendif-labels -Wexpansion-to-defined
-Wno-initializer-overrides -Wno-missing-include-dirs
-Wno-shift-negative-value -Wno-string-plus-int
-Wno-typedef-redefinition -Wno-tautological-type-limit-compare
-fstack-protector-strong -DSTRUCT_IOVEC_DEFINED -MD -MQ
libcommon.fa.p/net_vmnet-macos.c.o -MF
libcommon.fa.p/net_vmnet-macos.c.o.d -o
libcommon.fa.p/net_vmnet-macos.c.o -c ../net/vmnet-macos.c
../net/vmnet-macos.c:54:10: error: use of undeclared identifier
'VMNET_SHARING_SERVICE_BUSY'
case VMNET_SHARING_SERVICE_BUSY:
^
Hi Howard, Thanks very much for taking a look and testing it out! This is a good catch - VMNET_SHARING_SERVICE_BUSY was added in macOS Big Sur, and is not available in prior versions. I'll submit a v3 of this patch that adds a compile-guard around this constant (assuming the #available macro is... available), and will validate the other vmnet symbols I reference as well. Phillip On Thu, Feb 11, 2021 at 7:51 AM Howard Spoelstra <hsp.cat7@gmail.com> wrote: > On Fri, Feb 5, 2021 at 5:54 PM <phillip.ennen@gmail.com> wrote: > > > > From: Phillip Tennen <phillip@axleos.com> > > > > This patch implements a new netdev device, reachable via -netdev > > vmnet-macos, that’s backed by macOS’s vmnet framework. > > > > The vmnet framework provides native bridging support, and its usage in > > this patch is intended as a replacement for attempts to use a tap device > > via the tuntaposx kernel extension. Notably, the tap/tuntaposx approach > > never would have worked in the first place, as QEMU interacts with the > > tap device via poll(), and macOS does not support polling device files. > > > > vmnet requires either a special entitlement, granted via a provisioning > > profile, or root access. Otherwise attempts to create the virtual > > interface will fail with a “generic error” status code. QEMU may not > > currently be signed with an entitlement granted in a provisioning > > profile, as this would necessitate pre-signed binary build distribution, > > rather than source-code distribution. As such, using this netdev > > currently requires that qemu be run with root access. I’ve opened a > > feedback report with Apple to allow the use of the relevant entitlement > > with this use case: > > https://openradar.appspot.com/radar?id=5007417364447232 > > > > vmnet offers three operating modes, all of which are supported by this > > patch via the “mode=host|shared|bridge” option: > > > > * "Host" mode: Allows the vmnet interface to communicate with other > > * vmnet > > interfaces that are in host mode and also with the native host. > > * "Shared" mode: Allows traffic originating from the vmnet interface to > > reach the Internet through a NAT. The vmnet interface can also > > communicate with the native host. > > * "Bridged" mode: Bridges the vmnet interface with a physical network > > interface. > > > > Each of these modes also provide some extra configuration that’s > > supported by this patch: > > > > * "Bridged" mode: The user may specify the physical interface to bridge > > with. Defaults to en0. > > * "Host" mode / "Shared" mode: The user may specify the DHCP range and > > subnet. Allocated by vmnet if not provided. > > > > vmnet also offers some extra configuration options that are not > > supported by this patch: > > > > * Enable isolation from other VMs using vmnet > > * Port forwarding rules > > * Enabling TCP segmentation offload > > * Only applicable in "shared" mode: specifying the NAT IPv6 prefix > > * Only available in "host" mode: specifying the IP address for the VM > > within an isolated network > > > > Note that this patch requires macOS 10.15 as a minimum, as this is when > > bridging support was implemented in vmnet.framework. > > > > Signed-off-by: Phillip Tennen <phillip@axleos.com> > > > Hi Phillip, > > Thank you very much for this patch. As you wrote it should apply from > 10.15 upwards. I have no problem building on Big Sur, but Catalina > stumbles. See below. Latest Xcode installed, other requirements > installed through brew. > > Thanks for looking into this, > > Best, > Howard > > [1181/2135] Compiling C object libcommon.fa.p/net_vmnet-macos.c.o > FAILED: libcommon.fa.p/net_vmnet-macos.c.o > cc -Ilibcommon.fa.p -I. -I.. -I../capstone/include/capstone > -I../dtc/libfdt -I../slirp -I../slirp/src -Iqapi -Itrace -Iui > -Iui/shader -I/usr/local/Cellar/libffi/3.3_2/include > -I/usr/local/Cellar/glib/2.66.6/include > -I/usr/local/Cellar/glib/2.66.6/include/glib-2.0 > -I/usr/local/Cellar/glib/2.66.6/lib/glib-2.0/include > -I/usr/local/opt/gettext/include -I/usr/local/Cellar/pcre/8.44/include > -I/usr/local/Cellar/glib/2.66.6/include/gio-unix-2.0 > -I/usr/local/Cellar/libusb/1.0.24/include/libusb-1.0 > -I/usr/local/Cellar/pixman/0.40.0/include/pixman-1 -Xclang > -fcolor-diagnostics -pipe -Wall -Winvalid-pch -std=gnu99 -O2 -g > -iquote . -iquote /Users/hsp/src/qemu-master -iquote > /Users/hsp/src/qemu-master/include -iquote > /Users/hsp/src/qemu-master/disas/libvixl -iquote > /Users/hsp/src/qemu-master/tcg/i386 -iquote > /Users/hsp/src/qemu-master/accel/tcg -m64 -mcx16 > -DOS_OBJECT_USE_OBJC=0 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 > -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wundef > -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common > -fwrapv -Wold-style-definition -Wtype-limits -Wformat-security > -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body > -Wnested-externs -Wendif-labels -Wexpansion-to-defined > -Wno-initializer-overrides -Wno-missing-include-dirs > -Wno-shift-negative-value -Wno-string-plus-int > -Wno-typedef-redefinition -Wno-tautological-type-limit-compare > -fstack-protector-strong -DSTRUCT_IOVEC_DEFINED -MD -MQ > libcommon.fa.p/net_vmnet-macos.c.o -MF > libcommon.fa.p/net_vmnet-macos.c.o.d -o > libcommon.fa.p/net_vmnet-macos.c.o -c ../net/vmnet-macos.c > ../net/vmnet-macos.c:54:10: error: use of undeclared identifier > 'VMNET_SHARING_SERVICE_BUSY' > case VMNET_SHARING_SERVICE_BUSY: > ^ >
Phillip, this doesn't apply anymore. I'm reviewing the QAPI schema part
anyway.
Peter, there is a question for you below. Search for "Sphinx".
phillip.ennen@gmail.com writes:
> From: Phillip Tennen <phillip@axleos.com>
>
> This patch implements a new netdev device, reachable via -netdev
> vmnet-macos, that’s backed by macOS’s vmnet framework.
>
> The vmnet framework provides native bridging support, and its usage in
> this patch is intended as a replacement for attempts to use a tap device
> via the tuntaposx kernel extension. Notably, the tap/tuntaposx approach
> never would have worked in the first place, as QEMU interacts with the
> tap device via poll(), and macOS does not support polling device files.
>
> vmnet requires either a special entitlement, granted via a provisioning
> profile, or root access. Otherwise attempts to create the virtual
> interface will fail with a “generic error” status code. QEMU may not
> currently be signed with an entitlement granted in a provisioning
> profile, as this would necessitate pre-signed binary build distribution,
> rather than source-code distribution. As such, using this netdev
> currently requires that qemu be run with root access. I’ve opened a
> feedback report with Apple to allow the use of the relevant entitlement
> with this use case:
> https://openradar.appspot.com/radar?id=5007417364447232
>
> vmnet offers three operating modes, all of which are supported by this
> patch via the “mode=host|shared|bridge” option:
>
> * "Host" mode: Allows the vmnet interface to communicate with other
> * vmnet
> interfaces that are in host mode and also with the native host.
> * "Shared" mode: Allows traffic originating from the vmnet interface to
> reach the Internet through a NAT. The vmnet interface can also
> communicate with the native host.
> * "Bridged" mode: Bridges the vmnet interface with a physical network
> interface.
>
> Each of these modes also provide some extra configuration that’s
> supported by this patch:
>
> * "Bridged" mode: The user may specify the physical interface to bridge
> with. Defaults to en0.
> * "Host" mode / "Shared" mode: The user may specify the DHCP range and
> subnet. Allocated by vmnet if not provided.
>
> vmnet also offers some extra configuration options that are not
> supported by this patch:
>
> * Enable isolation from other VMs using vmnet
> * Port forwarding rules
> * Enabling TCP segmentation offload
> * Only applicable in "shared" mode: specifying the NAT IPv6 prefix
> * Only available in "host" mode: specifying the IP address for the VM
> within an isolated network
>
> Note that this patch requires macOS 10.15 as a minimum, as this is when
> bridging support was implemented in vmnet.framework.
>
> Signed-off-by: Phillip Tennen <phillip@axleos.com>
> ---
[...]
> diff --git a/qapi/net.json b/qapi/net.json
> index c31748c87f..e4d4143243 100644
> --- a/qapi/net.json
> +++ b/qapi/net.json
> @@ -450,6 +450,115 @@
> '*vhostdev': 'str',
> '*queues': 'int' } }
>
> +##
> +# @VmnetOperatingMode:
> +#
> +# The operating modes in which a vmnet netdev can run
> +# Only available on macOS
Please end these sentences with a period.
I'm not sure we need "Only available on macOS". Rendered documentation
shows the 'if' like
If
defined(CONFIG_DARWIN)
More of the same below.
> +#
> +# @host: the guest may communicate with the host
> +# and other guest network interfaces
> +#
> +# @shared: the guest may reach the Internet through a NAT,
> +# and may communicate with the host and other guest
> +# network interfaces
> +#
> +# @bridged: the guest's traffic is bridged with a
> +# physical network interface of the host
> +#
> +# Since: 6.0
> +##
> +{ 'enum': 'VmnetOperatingMode',
> + 'data': [ 'host', 'shared', 'bridged' ],
> + 'if': 'defined(CONFIG_DARWIN)' }
> +
> +##
> +# @NetdevVmnetModeOptionsBridged:
> +#
> +# Options for the vmnet-macos netdev
> +# that are only available in 'bridged' mode
> +# Only available on macOS
Likewise.
> +#
> +# @ifname: the physical network interface to bridge with
> +# (defaults to en0 if not specified)
> +#
> +# Since: 6.0
> +##
> +{ 'struct': 'NetdevVmnetModeOptionsBridged',
> + 'data': { '*ifname': 'str' },
> + 'if': 'defined(CONFIG_DARWIN)' }
> +
> +##
> +# @NetdevVmnetModeOptionsHostOrShared:
> +#
> +# Options for the vmnet-macos netdev
> +# that are only available in 'host' or 'shared' mode
> +# Only available on macOS
> +#
> +# @dhcp-start-address: the gateway address to use for the interface.
> +# The range to dhcp_end_address is placed in the DHCP pool.
Recommend to wrap lines around column 75.
> +# (only valid with mode=host|shared)
Isn't that trivial? The type's only use is as union branch for modes
host and shared.
> +# (must be specified with dhcp-end-address and
> +# dhcp-subnet-mask)
Does that mean you have to specify all three parameters or none?
Sphinx warns:
qapi/net.json:492:Unexpected indentation.
Known restriction of our tooling: the position is less than useful.
Unindenting the second line like
# (must be specified with dhcp-end-address and
# dhcp-subnet-mask)
here and similarly below makes Sphinx shut up.
I'm not 100% sure our tooling behaves as intended here. Peter?
> +# (allocated automatically if unset)
How?
The sequence of (parenthesized sentences) looks pretty bad in rendered
documentation. Please proof-read docs/manual/interop/qemu-qmp-ref.html.
> +#
> +# @dhcp-end-address: the DHCP IPv4 range end address to use for the interface.
> +# (only valid with mode=host|shared)
> +# (must be specified with dhcp-start-address and
> +# dhcp-subnet-mask)
> +# (allocated automatically if unset)
> +#
> +# @dhcp-subnet-mask: the IPv4 subnet mask (string) to use on the interface.
> +# (only valid with mode=host|shared)
> +# (must be specified with dhcp-start-address and
> +# dhcp-end-address)
> +# (allocated automatically if unset)
> +#
> +# Since: 6.0
> +##
> +{ 'struct': 'NetdevVmnetModeOptionsHostOrShared',
> + 'data': {
> + '*dhcp-start-address': 'str' ,
> + '*dhcp-end-address': 'str',
> + '*dhcp-subnet-mask': 'str' },
> + 'if': 'defined(CONFIG_DARWIN)' }
> +
> +##
> +# @NetdevVmnetModeOptions:
> +#
> +# Options specific to different operating modes of a vmnet netdev
> +# Only available on macOS
> +#
> +# @mode: the operating mode vmnet should run in
> +#
> +# Since: 6.0
> +##
> +{ 'union': 'NetdevVmnetModeOptions',
> + 'base': { 'mode': 'VmnetOperatingMode' },
> + 'discriminator': 'mode',
> + 'data': {
> + 'bridged': 'NetdevVmnetModeOptionsBridged',
> + 'host': 'NetdevVmnetModeOptionsHostOrShared',
> + 'shared': 'NetdevVmnetModeOptionsHostOrShared' },
> + 'if': 'defined(CONFIG_DARWIN)' }
> +
> +##
> +# @NetdevVmnetOptions:
> +#
> +# vmnet network backend
> +# Only available on macOS
> +#
> +# @options: a structure specifying the mode and mode-specific options
> +# (once QAPI supports a union type as a branch to another union type,
> +# this structure can be changed to a union, and the contents of
> +# NetdevVmnetModeOptions moved here)
Sphinx warns:
qapi/net.json:546:Unexpected indentation.
> +#
> +# Since: 6.0
> +##
> +{ 'struct': 'NetdevVmnetOptions',
> + 'data': {'options': 'NetdevVmnetModeOptions' },
> + 'if': 'defined(CONFIG_DARWIN)' }
> +
Awkward.
You can't use make NetdevVmnetModeOptions a branch of union Netdev,
because NetdevVmnetModeOptions is a union, and a branch must be a
struct. To work around, you wrap struct NetdevVmnetOptions around union
NetdevVmnetModeOptions.
NetdevVmnetModeOptions has no common members other than the union
discriminator. Why not add them as three branches to Netdev?
> ##
> # @NetClientDriver:
> #
> @@ -458,10 +567,13 @@
> # Since: 2.7
> #
> # @vhost-vdpa since 5.1
> +#
> +# @vmnet-macos since 6.0 (only available on macOS)
> ##
> { 'enum': 'NetClientDriver',
> 'data': [ 'none', 'nic', 'user', 'tap', 'l2tpv3', 'socket', 'vde',
> - 'bridge', 'hubport', 'netmap', 'vhost-user', 'vhost-vdpa' ] }
> + 'bridge', 'hubport', 'netmap', 'vhost-user', 'vhost-vdpa',
> + { 'name': 'vmnet-macos', 'if': 'defined(CONFIG_DARWIN)' } ] }
>
> ##
> # @Netdev:
> @@ -475,6 +587,8 @@
> # Since: 1.2
> #
> # 'l2tpv3' - since 2.1
> +#
> +# 'vmnet-macos' since 6.0 (only available on macOS)
> ##
> { 'union': 'Netdev',
> 'base': { 'id': 'str', 'type': 'NetClientDriver' },
> @@ -490,7 +604,9 @@
> 'hubport': 'NetdevHubPortOptions',
> 'netmap': 'NetdevNetmapOptions',
> 'vhost-user': 'NetdevVhostUserOptions',
> - 'vhost-vdpa': 'NetdevVhostVDPAOptions' } }
> + 'vhost-vdpa': 'NetdevVhostVDPAOptions',
> + 'vmnet-macos': { 'type': 'NetdevVmnetOptions',
> + 'if': 'defined(CONFIG_DARWIN)' } } }
>
> ##
> # @NetFilterDirection:
[...]
Markus, thanks for the review. I apologize for my lateness in getting back
to you.
I've integrated most of your suggestions, and will submit a v5 that
incorporates them. I've left a couple comments and questions for you below.
Aside: I haven't responded inline to emails like this before, I'm hoping it
shows
up correctly for you! I appreciate how understanding everyone's been
towards my
newness to this development & review format. I cut out the irrelevant bits
for brevity and am unsure if that breaks anything.
Phillip
On Tue, Mar 2, 2021 at 11:49 AM Markus Armbruster <armbru@redhat.com> wrote:
> Phillip, this doesn't apply anymore. I'm reviewing the QAPI schema part
> anyway.
>
> Peter, there is a question for you below. Search for "Sphinx".
>
> phillip.ennen@gmail.com writes:
>
> > From: Phillip Tennen <phillip@axleos.com>
> >
> > This patch implements a new netdev device, reachable via -netdev
> > vmnet-macos, that’s backed by macOS’s vmnet framework.
> >
>
[...]
> > diff --git a/qapi/net.json b/qapi/net.json
> > index c31748c87f..e4d4143243 100644
> > --- a/qapi/net.json
> > +++ b/qapi/net.json
> > @@ -450,6 +450,115 @@
> > '*vhostdev': 'str',
> > '*queues': 'int' } }
> >
> > +##
> > +# @VmnetOperatingMode:
> > +#
> > +# The operating modes in which a vmnet netdev can run
> > +# Only available on macOS
>
> Please end these sentences with a period.
>
> I'm not sure we need "Only available on macOS". Rendered documentation
> shows the 'if' like
>
> [...]
> > +# (only valid with mode=host|shared)
>
> Isn't that trivial? The type's only use is as union branch for modes
> host and shared.
>
True. I added comments like this for clarity, but I accept that the schema
should make it clear alone.
>
> > +# (must be specified with dhcp-end-address and
> > +# dhcp-subnet-mask)
>
> Does that mean you have to specify all three parameters or none?
>
That's correct. You may provide either none or all three.
[...]
> > +# (allocated automatically if unset)
>
> How?
>
vmnet automatically allocates specifics like the MAC address, DHCP pool,
etc,
if not explicitly supplied. I'll add some wording to this effect.
[...]
>
> > +#
> > +# Since: 6.0
> > +##
> > +{ 'struct': 'NetdevVmnetOptions',
> > + 'data': {'options': 'NetdevVmnetModeOptions' },
> > + 'if': 'defined(CONFIG_DARWIN)' }
> > +
>
> Awkward.
>
> You can't use make NetdevVmnetModeOptions a branch of union Netdev,
> because NetdevVmnetModeOptions is a union, and a branch must be a
> struct. To work around, you wrap struct NetdevVmnetOptions around union
> NetdevVmnetModeOptions.
>
> NetdevVmnetModeOptions has no common members other than the union
> discriminator. Why not add them as three branches to Netdev?
>
> Just to be sure I understand, you're proposing adding 3 new fields to
Netdev,
like so:
'vmnet-macos-bridged': { 'type': 'NetdevVmnetModeOptionsBridged',
'if': 'defined(CONFIG_DARWIN)' },
'vmnet-macos-host': { 'type': 'NetdevVmnetModeOptionsHostOrShared',
'if': 'defined(CONFIG_DARWIN)' },
'vmnet-macos-shared': { 'type': 'NetdevVmnetModeOptionsHostOrShared',
'if': 'defined(CONFIG_DARWIN)' },
... where each of those "ModeOptions" structs contains a new "mode" field
extracted from the union. Did I get your intent right? I'm assuming there
wouldn't be issues with "vmnet-macos" referenced elsewhere.
Thank you!
Phillip
Phillip Tennen <phillip.ennen@gmail.com> writes:
> Markus, thanks for the review. I apologize for my lateness in getting back
> to you.
>
> I've integrated most of your suggestions, and will submit a v5 that
> incorporates them. I've left a couple comments and questions for you below.
>
> Aside: I haven't responded inline to emails like this before, I'm hoping it
> shows
> up correctly for you! I appreciate how understanding everyone's been
> towards my
> newness to this development & review format. I cut out the irrelevant bits
> for brevity and am unsure if that breaks anything.
We *try* not to be jerks ;)
Your reply looks fine to me.
> Phillip
>
> On Tue, Mar 2, 2021 at 11:49 AM Markus Armbruster <armbru@redhat.com> wrote:
>
>> Phillip, this doesn't apply anymore. I'm reviewing the QAPI schema part
>> anyway.
>>
>> Peter, there is a question for you below. Search for "Sphinx".
>>
>> phillip.ennen@gmail.com writes:
>>
>> > From: Phillip Tennen <phillip@axleos.com>
>> >
>> > This patch implements a new netdev device, reachable via -netdev
>> > vmnet-macos, that’s backed by macOS’s vmnet framework.
>> >
>>
> [...]
>
>> > diff --git a/qapi/net.json b/qapi/net.json
>> > index c31748c87f..e4d4143243 100644
>> > --- a/qapi/net.json
>> > +++ b/qapi/net.json
>> > @@ -450,6 +450,115 @@
>> > '*vhostdev': 'str',
>> > '*queues': 'int' } }
>> >
>> > +##
>> > +# @VmnetOperatingMode:
>> > +#
>> > +# The operating modes in which a vmnet netdev can run
>> > +# Only available on macOS
>>
>> Please end these sentences with a period.
>>
>> I'm not sure we need "Only available on macOS". Rendered documentation
>> shows the 'if' like
>>
>> [...]
>
>> > +# (only valid with mode=host|shared)
>>
>> Isn't that trivial? The type's only use is as union branch for modes
>> host and shared.
>>
> True. I added comments like this for clarity, but I accept that the schema
> should make it clear alone.
Clarity is in the eye of the beholder. We try to find the sweet spot
between bafflingly terse and tiresomely verbose.
>> > +# (must be specified with dhcp-end-address and
>> > +# dhcp-subnet-mask)
>>
>> Does that mean you have to specify all three parameters or none?
>>
> That's correct. You may provide either none or all three.
In bridged mode, none.
In host or shared mode, either all three or none.
Correct?
> [...]
>
>> > +# (allocated automatically if unset)
>>
>> How?
>>
> vmnet automatically allocates specifics like the MAC address, DHCP pool,
> etc,
> if not explicitly supplied. I'll add some wording to this effect.
> [...]
>
>>
>> > +#
>> > +# Since: 6.0
>> > +##
>> > +{ 'struct': 'NetdevVmnetOptions',
>> > + 'data': {'options': 'NetdevVmnetModeOptions' },
>> > + 'if': 'defined(CONFIG_DARWIN)' }
>> > +
>>
>> Awkward.
>>
>> You can't use make NetdevVmnetModeOptions a branch of union Netdev,
>> because NetdevVmnetModeOptions is a union, and a branch must be a
>> struct. To work around, you wrap struct NetdevVmnetOptions around union
>> NetdevVmnetModeOptions.
>>
>> NetdevVmnetModeOptions has no common members other than the union
>> discriminator. Why not add them as three branches to Netdev?
>
> Just to be sure I understand, you're proposing adding 3 new fields to
> Netdev,
> like so:
> 'vmnet-macos-bridged': { 'type': 'NetdevVmnetModeOptionsBridged',
> 'if': 'defined(CONFIG_DARWIN)' },
> 'vmnet-macos-host': { 'type': 'NetdevVmnetModeOptionsHostOrShared',
> 'if': 'defined(CONFIG_DARWIN)' },
> 'vmnet-macos-shared': { 'type': 'NetdevVmnetModeOptionsHostOrShared',
> 'if': 'defined(CONFIG_DARWIN)' },
> ... where each of those "ModeOptions" structs contains a new "mode" field
> extracted from the union. Did I get your intent right? I'm assuming there
> wouldn't be issues with "vmnet-macos" referenced elsewhere.
Yes, except you don't need a @mode member, you can derive the mode from
Netdev member @type.
Clear now?
© 2016 - 2024 Red Hat, Inc.