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 <thuth@redhat.com>
---
net/net.c | 74 ++++++++-------------------------------------------
qapi/net.json | 51 +----------------------------------
2 files changed, 12 insertions(+), 113 deletions(-)
diff --git a/net/net.c b/net/net.c
index ee4a76eb3e..cfe524b4d1 100644
--- a/net/net.c
+++ 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)
{
- 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;
+ }
if (netdev->type == NET_CLIENT_DRIVER_NIC ||
!net_client_init_fun[netdev->type]) {
error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "type",
@@ -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 */
- case NET_LEGACY_OPTIONS_TYPE_NIC:
- legacy.type = NET_CLIENT_DRIVER_NIC;
- legacy.u.nic = opts->u.nic;
- break;
- case NET_LEGACY_OPTIONS_TYPE_USER:
- legacy.type = NET_CLIENT_DRIVER_USER;
- legacy.u.user = opts->u.user;
- break;
- case NET_LEGACY_OPTIONS_TYPE_TAP:
- legacy.type = NET_CLIENT_DRIVER_TAP;
- legacy.u.tap = opts->u.tap;
- break;
- case NET_LEGACY_OPTIONS_TYPE_L2TPV3:
- legacy.type = NET_CLIENT_DRIVER_L2TPV3;
- legacy.u.l2tpv3 = opts->u.l2tpv3;
- break;
- case NET_LEGACY_OPTIONS_TYPE_SOCKET:
- legacy.type = NET_CLIENT_DRIVER_SOCKET;
- legacy.u.socket = opts->u.socket;
- break;
- case NET_LEGACY_OPTIONS_TYPE_VDE:
- legacy.type = NET_CLIENT_DRIVER_VDE;
- legacy.u.vde = opts->u.vde;
- break;
- case NET_LEGACY_OPTIONS_TYPE_BRIDGE:
- legacy.type = NET_CLIENT_DRIVER_BRIDGE;
- legacy.u.bridge = opts->u.bridge;
- break;
- case NET_LEGACY_OPTIONS_TYPE_NETMAP:
- legacy.type = NET_CLIENT_DRIVER_NETMAP;
- legacy.u.netmap = opts->u.netmap;
- break;
- case NET_LEGACY_OPTIONS_TYPE_VHOST_USER:
- legacy.type = NET_CLIENT_DRIVER_VHOST_USER;
- legacy.u.vhost_user = opts->u.vhost_user;
- break;
- default:
- abort();
}
-
- 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)");
@@ -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);
}
}
@@ -1137,21 +1093,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);
if (!err) {
ret = net_client_init1(object, is_netdev, &err);
}
- if (is_netdev) {
- qapi_free_Netdev(object);
- } else {
- qapi_free_NetLegacy(object);
- }
+ qapi_free_Netdev(object);
out:
error_propagate(errp, err);
diff --git a/qapi/net.json b/qapi/net.json
index ff280ccd16..fcf693924e 100644
--- a/qapi/net.json
+++ b/qapi/net.json
@@ -467,7 +467,7 @@
# 'l2tpv3' - since 2.1
##
{ 'union': 'Netdev',
- 'base': { 'id': 'str', 'type': 'NetClientDriver' },
+ 'base': { '*id': 'str', 'type': 'NetClientDriver' },
'discriminator': 'type',
'data': {
'nic': 'NetLegacyNicOptions',
@@ -481,55 +481,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
-#
-# 'vlan': dropped in 3.0
-# 'name': dropped in 5.0
-##
-{ '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'] }
-
-##
-# @NetLegacyOptions:
-#
-# Like Netdev, but for use only by the legacy command line options
-#
-# Since: 1.2
-##
-{ 'union': 'NetLegacyOptions',
- 'base': { 'type': 'NetLegacyOptionsType' },
- 'discriminator': 'type',
- 'data': {
- 'nic': 'NetLegacyNicOptions',
- 'user': 'NetdevUserOptions',
- 'tap': 'NetdevTapOptions',
- 'l2tpv3': 'NetdevL2TPv3Options',
- 'socket': 'NetdevSocketOptions',
- 'vde': 'NetdevVdeOptions',
- 'bridge': 'NetdevBridgeOptions',
- 'netmap': 'NetdevNetmapOptions',
- 'vhost-user': 'NetdevVhostUserOptions' } }
-
##
# @NetFilterDirection:
#
--
2.18.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Thomas Huth <thuth@redhat.com> writes:
> 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 <thuth@redhat.com>
Took me a minute to see the actual difference.
Here's Netdev:
{ 'union': 'Netdev',
'base': { 'id': 'str', 'type': 'NetClientDriver' },
'discriminator': 'type',
'data': {
'nic': 'NetLegacyNicOptions',
'user': 'NetdevUserOptions',
'tap': 'NetdevTapOptions',
'l2tpv3': 'NetdevL2TPv3Options',
'socket': 'NetdevSocketOptions',
'vde': 'NetdevVdeOptions',
'bridge': 'NetdevBridgeOptions',
'hubport': 'NetdevHubPortOptions',
'netmap': 'NetdevNetmapOptions',
'vhost-user': 'NetdevVhostUserOptions' } }
{ 'enum': 'NetClientDriver',
'data': [ 'none', 'nic', 'user', 'tap', 'l2tpv3', 'socket', 'vde',
'bridge', 'hubport', 'netmap', 'vhost-user' ] }
Here's NetLegacy:
{ 'struct': 'NetLegacy',
'data': {
'*id': 'str',
'opts': 'NetLegacyOptions' } }
{ 'union': 'NetLegacyOptions',
'base': { 'type': 'NetLegacyOptionsType' },
'discriminator': 'type',
'data': {
'nic': 'NetLegacyNicOptions',
'user': 'NetdevUserOptions',
'tap': 'NetdevTapOptions',
'l2tpv3': 'NetdevL2TPv3Options',
'socket': 'NetdevSocketOptions',
'vde': 'NetdevVdeOptions',
'bridge': 'NetdevBridgeOptions',
'netmap': 'NetdevNetmapOptions',
'vhost-user': 'NetdevVhostUserOptions' } }
{ 'enum': 'NetLegacyOptionsType',
'data': ['none', 'nic', 'user', 'tap', 'l2tpv3', 'socket', 'vde',
'bridge', 'netmap', 'vhost-user'] }
Difference: NetLegacy wraps the union in @opts.
On 12/5/19 11:36 PM, 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 <thuth@redhat.com>
> ---
Initial focus on the QAPI change:
> +++ b/qapi/net.json
> @@ -467,7 +467,7 @@
> # 'l2tpv3' - since 2.1
> ##
> { 'union': 'Netdev',
> - 'base': { 'id': 'str', 'type': 'NetClientDriver' },
> + 'base': { '*id': 'str', 'type': 'NetClientDriver' },
Making id optional here...
> 'discriminator': 'type',
> 'data': {
> 'nic': 'NetLegacyNicOptions',
> @@ -481,55 +481,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
> -#
> -# 'vlan': dropped in 3.0
> -# 'name': dropped in 5.0
> -##
> -{ 'struct': 'NetLegacy',
> - 'data': {
> - '*id': 'str',
> - 'opts': 'NetLegacyOptions' } }
to match how it was here. Should 'id' have been made mandatory in 1/2,
when deleting 'name' (after all, id was optional only when name was in use)?
> -
> -##
> -# @NetLegacyOptionsType:
> -#
> -# Since: 1.2
> -##
> -{ 'enum': 'NetLegacyOptionsType',
> - 'data': ['none', 'nic', 'user', 'tap', 'l2tpv3', 'socket', 'vde',
> - 'bridge', 'netmap', 'vhost-user'] }
Comparing this to the branches of Netdev:
We are losing 'none', while gaining 'hubport'. The gain is not
problematic, and I guess you are declaring that the use of 'none' has
been deprecated long enough to not be a problem.
> -
> -##
> -# @NetLegacyOptions:
> -#
> -# Like Netdev, but for use only by the legacy command line options
> -#
> -# Since: 1.2
> -##
> -{ 'union': 'NetLegacyOptions',
> - 'base': { 'type': 'NetLegacyOptionsType' },
> - 'discriminator': 'type',
> - 'data': {
> - 'nic': 'NetLegacyNicOptions',
Should we rename this to NetdevNicOptions, now that we are getting rid
of other NetLegacy names?
> - 'user': 'NetdevUserOptions',
> - 'tap': 'NetdevTapOptions',
> - 'l2tpv3': 'NetdevL2TPv3Options',
> - 'socket': 'NetdevSocketOptions',
> - 'vde': 'NetdevVdeOptions',
> - 'bridge': 'NetdevBridgeOptions',
> - 'netmap': 'NetdevNetmapOptions',
> - 'vhost-user': 'NetdevVhostUserOptions' } }
But I concur that all branches of the Netdev union have the same types
as what you are removing here from NetLegacyOptions, so the
consolidation looks sane.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 06/12/2019 16.14, Eric Blake wrote:
> On 12/5/19 11:36 PM, 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 <thuth@redhat.com>
>> ---
>
> Initial focus on the QAPI change:
>
>> +++ b/qapi/net.json
>> @@ -467,7 +467,7 @@
>> # 'l2tpv3' - since 2.1
>> ##
>> { 'union': 'Netdev',
>> - 'base': { 'id': 'str', 'type': 'NetClientDriver' },
>> + 'base': { '*id': 'str', 'type': 'NetClientDriver' },
>
> Making id optional here...
>
>> 'discriminator': 'type',
>> 'data': {
>> 'nic': 'NetLegacyNicOptions',
>> @@ -481,55 +481,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
>> -#
>> -# 'vlan': dropped in 3.0
>> -# 'name': dropped in 5.0
>> -##
>> -{ 'struct': 'NetLegacy',
>> - 'data': {
>> - '*id': 'str',
>> - 'opts': 'NetLegacyOptions' } }
>
> to match how it was here. Should 'id' have been made mandatory in 1/2,
> when deleting 'name' (after all, id was optional only when name was in
> use)?
No, since "id" is still not mandatory for "-net". In case it is missing,
the code creates an id internally (see assign_name() in net/net.c).
>> -
>> -##
>> -# @NetLegacyOptionsType:
>> -#
>> -# Since: 1.2
>> -##
>> -{ 'enum': 'NetLegacyOptionsType',
>> - 'data': ['none', 'nic', 'user', 'tap', 'l2tpv3', 'socket', 'vde',
>> - 'bridge', 'netmap', 'vhost-user'] }
>
> Comparing this to the branches of Netdev:
>
> We are losing 'none', while gaining 'hubport'. The gain is not
> problematic, and I guess you are declaring that the use of 'none' has
> been deprecated long enough to not be a problem.
'none' still continues to work, it's also a member of NetClientDriver
and was handled later in the patch:
+ if (netdev->type == NET_CLIENT_DRIVER_NONE) {
return 0; /* nothing to do */
'hubport' is blocked in the code now instead:
+ 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)");
>> -
>> -##
>> -# @NetLegacyOptions:
>> -#
>> -# Like Netdev, but for use only by the legacy command line options
>> -#
>> -# Since: 1.2
>> -##
>> -{ 'union': 'NetLegacyOptions',
>> - 'base': { 'type': 'NetLegacyOptionsType' },
>> - 'discriminator': 'type',
>> - 'data': {
>> - 'nic': 'NetLegacyNicOptions',
>
> Should we rename this to NetdevNicOptions, now that we are getting rid
> of other NetLegacy names?
I still consider "-net nic" as a legacy option that we should remove one
day in the future, so I'd rather keep that name.
>
> But I concur that all branches of the Netdev union have the same types
> as what you are removing here from NetLegacyOptions, so the
> consolidation looks sane.
Ok, thanks for your review!
Thomas
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2026 Red Hat, Inc.