[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 2/2] net: Drop the NetLegacy structure, always use Netdev
From: |
Eric Blake |
Subject: |
Re: [PATCH v2 2/2] net: Drop the NetLegacy structure, always use Netdev instead |
Date: |
Tue, 12 May 2020 09:32:55 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 |
On 5/12/20 7:31 AM, Thomas Huth wrote:
Now that the "name" parameter is gone, there is hardly any difference
between NetLegacy and Netdev anymore. Drop NetLegacy and always use
Netdev to simplify the code quite a bit.
Signed-off-by: Thomas Huth <address@hidden>
---
+++ b/net/net.c
@@ -967,13 +967,14 @@ static int (* const
net_client_init_fun[NET_CLIENT_DRIVER__MAX])(
static int net_client_init1(const void *object, bool is_netdev, Error **errp)
Why do we still need the 'is_netdev' parameter? If all callers are
passing in a netdev, then either this parameter needs to be renamed to
capture the actual difference between callers, or it can be dropped
altogether.
{
- Netdev legacy = {0};
- const Netdev *netdev;
+ const Netdev *netdev = object;
NetClientState *peer = NULL;
if (is_netdev) {
- netdev = object;
-
+ if (!netdev->has_id) {
+ error_setg(errp, QERR_MISSING_PARAMETER, "id");
+ return -1;
+ }
You wouldn't need this if 'id' remained mandatory.
@@ -981,56 +982,11 @@ static int net_client_init1(const void *object, bool
is_netdev, Error **errp)
return -1;
}
} else {
- const NetLegacy *net = object;
- const NetLegacyOptions *opts = net->opts;
- legacy.id = net->id;
- netdev = &legacy;
-
- /* Map the old options to the new flat type */
- switch (opts->type) {
- case NET_LEGACY_OPTIONS_TYPE_NONE:
+ if (netdev->type == NET_CLIENT_DRIVER_NONE) {
return 0; /* nothing to do */
-
- if (!net_client_init_fun[netdev->type]) {
+ if (netdev->type == NET_CLIENT_DRIVER_HUBPORT ||
+ !net_client_init_fun[netdev->type]) {
error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "type",
"a net backend type (maybe it is not compiled "
"into this binary)");
So maybe we still want this legacy-handling code, but renaming
'is_netdev' to 'legacy_handling' may make more sense.
@@ -1039,7 +995,7 @@ static int net_client_init1(const void *object, bool
is_netdev, Error **errp)
/* Do not add to a hub if it's a nic with a netdev= parameter. */
if (netdev->type != NET_CLIENT_DRIVER_NIC ||
- !opts->u.nic.has_netdev) {
+ !netdev->u.nic.has_netdev) {
peer = net_hub_add_port(0, NULL, NULL);
}
}
@@ -1143,21 +1099,13 @@ static int net_client_init(QemuOpts *opts, bool
is_netdev, Error **errp)
}
}
- if (is_netdev) {
- visit_type_Netdev(v, NULL, (Netdev **)&object, &err);
- } else {
- visit_type_NetLegacy(v, NULL, (NetLegacy **)&object, &err);
- }
+ visit_type_Netdev(v, NULL, (Netdev **)&object, &err);
Why do we need to cast? If all callers are passing a Netdev, can't we
just give 'object' the correct type to begin with?
+++ b/qapi/net.json
@@ -453,7 +453,7 @@
# 'l2tpv3' - since 2.1
##
{ 'union': 'Netdev',
- 'base': { 'id': 'str', 'type': 'NetClientDriver' },
+ 'base': { '*id': 'str', 'type': 'NetClientDriver' },
I don't think we need to make 'id' optional.
'discriminator': 'type',
'data': {
'nic': 'NetLegacyNicOptions',
@@ -467,52 +467,6 @@
'netmap': 'NetdevNetmapOptions',
'vhost-user': 'NetdevVhostUserOptions' } }
-##
-# @NetLegacy:
-#
-# Captures the configuration of a network device; legacy.
-#
-# @id: identifier for monitor commands
-#
-# @opts: device type specific properties (legacy)
-#
-# Since: 1.2
-##
-{ 'struct': 'NetLegacy',
- 'data': {
- '*id': 'str',
- 'opts': 'NetLegacyOptions' } }
-
-##
-# @NetLegacyOptionsType:
-#
-# Since: 1.2
-##
-{ 'enum': 'NetLegacyOptionsType',
- 'data': ['none', 'nic', 'user', 'tap', 'l2tpv3', 'socket', 'vde',
- 'bridge', 'netmap', 'vhost-user'] }
NetLegacyOptionsType differs from NetClientDriver only by missing
'hubport', which your code above special-cased, so on that front, the
simplification makes sense.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org