* [PATCH 0/2] net: Drop legacy "name" from -net and remove NetLegacy @ 2019-12-06 5:36 Thomas Huth 2019-12-06 5:36 ` [PATCH 1/2] net: Drop the legacy "name" parameter from the -net option Thomas Huth ` (2 more replies) 0 siblings, 3 replies; 10+ messages in thread From: Thomas Huth @ 2019-12-06 5:36 UTC (permalink / raw) To: qemu-devel, Jason Wang, Eric Blake, Markus Armbruster; +Cc: libvir-list It's time to remove the deprecated "name" parameter from -net. Please have a closer look at the second patch ... I think it should be fine, but I'm not 100% sure whether it's ok for all cases to drop NetLegacy, so please double-check. Thomas Huth (2): net: Drop the legacy "name" parameter from the -net option net: Drop the NetLegacy structure, always use Netdev instead net/net.c | 84 +++++++------------------------------------- qapi/net.json | 53 +--------------------------- qemu-deprecated.texi | 12 ++++--- 3 files changed, 20 insertions(+), 129 deletions(-) -- 2.18.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] net: Drop the legacy "name" parameter from the -net option 2019-12-06 5:36 [PATCH 0/2] net: Drop legacy "name" from -net and remove NetLegacy Thomas Huth @ 2019-12-06 5:36 ` Thomas Huth 2019-12-06 15:07 ` Eric Blake 2019-12-10 11:06 ` Markus Armbruster 2019-12-06 5:36 ` [PATCH 2/2] net: Drop the NetLegacy structure, always use Netdev instead Thomas Huth 2020-01-29 12:42 ` [PATCH 0/2] net: Drop legacy "name" from -net and remove NetLegacy Thomas Huth 2 siblings, 2 replies; 10+ messages in thread From: Thomas Huth @ 2019-12-06 5:36 UTC (permalink / raw) To: qemu-devel, Jason Wang, Eric Blake, Markus Armbruster; +Cc: libvir-list It's been deprecated since QEMU v3.1, so it's time to finally remove it. The "id" parameter can simply be used instead. Signed-off-by: Thomas Huth <thuth@redhat.com> --- net/net.c | 10 +--------- qapi/net.json | 4 +--- qemu-deprecated.texi | 12 +++++++----- 3 files changed, 9 insertions(+), 17 deletions(-) diff --git a/net/net.c b/net/net.c index 84aa6d8d00..ee4a76eb3e 100644 --- a/net/net.c +++ b/net/net.c @@ -969,12 +969,10 @@ static int net_client_init1(const void *object, bool is_netdev, Error **errp) { Netdev legacy = {0}; const Netdev *netdev; - const char *name; NetClientState *peer = NULL; if (is_netdev) { netdev = object; - name = netdev->id; if (netdev->type == NET_CLIENT_DRIVER_NIC || !net_client_init_fun[netdev->type]) { @@ -987,12 +985,6 @@ static int net_client_init1(const void *object, bool is_netdev, Error **errp) const NetLegacyOptions *opts = net->opts; legacy.id = net->id; netdev = &legacy; - /* missing optional values have been initialized to "all bits zero" */ - name = net->has_id ? net->id : net->name; - - if (net->has_name) { - warn_report("The 'name' parameter is deprecated, use 'id' instead"); - } /* Map the old options to the new flat type */ switch (opts->type) { @@ -1052,7 +1044,7 @@ static int net_client_init1(const void *object, bool is_netdev, Error **errp) } } - if (net_client_init_fun[netdev->type](netdev, name, peer, errp) < 0) { + if (net_client_init_fun[netdev->type](netdev, netdev->id, peer, errp) < 0) { /* FIXME drop when all init functions store an Error */ if (errp && !*errp) { error_setg(errp, QERR_DEVICE_INIT_FAILED, diff --git a/qapi/net.json b/qapi/net.json index 335295be50..ff280ccd16 100644 --- a/qapi/net.json +++ b/qapi/net.json @@ -488,18 +488,16 @@ # # @id: identifier for monitor commands # -# @name: identifier for monitor commands, ignored if @id is present -# # @opts: device type specific properties (legacy) # # Since: 1.2 # # 'vlan': dropped in 3.0 +# 'name': dropped in 5.0 ## { 'struct': 'NetLegacy', 'data': { '*id': 'str', - '*name': 'str', 'opts': 'NetLegacyOptions' } } ## diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi index 2850f9a520..2f9efb45ba 100644 --- a/qemu-deprecated.texi +++ b/qemu-deprecated.texi @@ -42,11 +42,6 @@ The 'file' driver for drives is no longer appropriate for character or host devices and will only accept regular files (S_IFREG). The correct driver for these file types is 'host_cdrom' or 'host_device' as appropriate. -@subsection -net ...,name=@var{name} (since 3.1) - -The @option{name} parameter of the @option{-net} option is a synonym -for the @option{id} parameter, which should now be used instead. - @subsection -smp (invalid topologies) (since 3.1) CPU topology properties should describe whole machine topology including @@ -371,6 +366,13 @@ What follows is a record of recently removed, formerly deprecated features that serves as a record for users who have encountered trouble after a recent upgrade. +@section System emulator command line arguments + +@subsection -net ...,name=@var{name} (removed in 5.0) + +The @option{name} parameter of the @option{-net} option was a synonym +for the @option{id} parameter, which should now be used instead. + @section Human Monitor Protocol (HMP) commands @subsection The hub_id parameter of 'hostfwd_add' / 'hostfwd_remove' (removed in 5.0) -- 2.18.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] net: Drop the legacy "name" parameter from the -net option 2019-12-06 5:36 ` [PATCH 1/2] net: Drop the legacy "name" parameter from the -net option Thomas Huth @ 2019-12-06 15:07 ` Eric Blake 2019-12-10 11:06 ` Markus Armbruster 1 sibling, 0 replies; 10+ messages in thread From: Eric Blake @ 2019-12-06 15:07 UTC (permalink / raw) To: Thomas Huth, qemu-devel, Jason Wang, Markus Armbruster; +Cc: libvir-list On 12/5/19 11:36 PM, Thomas Huth wrote: > It's been deprecated since QEMU v3.1, so it's time to finally > remove it. The "id" parameter can simply be used instead. > > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > net/net.c | 10 +--------- > qapi/net.json | 4 +--- > qemu-deprecated.texi | 12 +++++++----- > 3 files changed, 9 insertions(+), 17 deletions(-) > Reviewed-by: Eric Blake <eblake@redhat.com> A quick grep of 'name.*netdev' and 'netdev.*name' in libvirt did not seem to find any uses of the deprecated syntax. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] net: Drop the legacy "name" parameter from the -net option 2019-12-06 5:36 ` [PATCH 1/2] net: Drop the legacy "name" parameter from the -net option Thomas Huth 2019-12-06 15:07 ` Eric Blake @ 2019-12-10 11:06 ` Markus Armbruster 2019-12-10 11:28 ` Thomas Huth 1 sibling, 1 reply; 10+ messages in thread From: Markus Armbruster @ 2019-12-10 11:06 UTC (permalink / raw) To: Thomas Huth; +Cc: libvir-list, Jason Wang, qemu-devel Thomas Huth <thuth@redhat.com> writes: > It's been deprecated since QEMU v3.1, so it's time to finally > remove it. The "id" parameter can simply be used instead. > > Signed-off-by: Thomas Huth <thuth@redhat.com> [...] > diff --git a/qapi/net.json b/qapi/net.json > index 335295be50..ff280ccd16 100644 > --- a/qapi/net.json > +++ b/qapi/net.json > @@ -488,18 +488,16 @@ > # > # @id: identifier for monitor commands > # > -# @name: identifier for monitor commands, ignored if @id is present > -# > # @opts: device type specific properties (legacy) > # > # Since: 1.2 > # > # 'vlan': dropped in 3.0 > +# 'name': dropped in 5.0 > ## Uh, when did we start to add "dropped in" to our doc comments? We should either do this systematically, or not at all. If the former, we have quite a few "dropped in" to add belatedly. I vote for "not at all". > { 'struct': 'NetLegacy', > 'data': { > '*id': 'str', > - '*name': 'str', > 'opts': 'NetLegacyOptions' } } > > ## [...] History: $ git-log -S"dropped in" -- qapi qapi-schema.json commit ffaee83bcb28913b8b854aeab78b1a1f2115712d Author: Markus Armbruster <armbru@redhat.com> Date: Tue Jul 9 17:20:53 2019 +0200 qapi: Move query-target from misc.json to machine.json Move query-target and its return type TargetInfo from misc.json to machine.json, where they are covered by MAINTAINERS section "Machine core". Also move its implementation from arch_init.c to hw/core/machine-qmp-cmds, where it is likewise covered. All users of SysEmuTarget are now in machine.json. Move it there from common.json. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20190709152053.16670-3-armbru@redhat.com> commit 416756cc049006ab8a05fe39e5f2e6af25cad9d2 Author: Thomas Huth <thuth@redhat.com> Date: Tue Aug 21 13:27:48 2018 +0200 Record history of ppcemb target in common.json We recently removed the long deprecated "ppcemb" target. This adds a comment in common.json about the SysEmuTarget type, recording when it was removed. Suggested-by: Eric Blake <eblake@redhat.com> Signed-off-by: David Gibson <david@gibson.dropbear.id.au> commit 4088b5536436090207dcf6d15e47908f74b2d8f2 Author: Thomas Huth <thuth@redhat.com> Date: Tue May 15 18:26:20 2018 +0200 qapi/net.json: Fix the version number of the "vlan" removal "vlan" will be dropped in 2.13, not in 2.12. And while we're at it, use the better wording "dropped in" instead of "removed with" (also for the "dump" removal). Reported-by: Stefan Hajnoczi <stefanha@redhat.com> Reported-by: Eric Blake <eblake@redhat.com> Signed-off-by: Thomas Huth <thuth@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru> commit 608cfed66a6adeac136b0c09cd62d081062256f3 Author: Markus Armbruster <armbru@redhat.com> Date: Thu Aug 24 21:14:00 2017 +0200 qapi-schema: Collect UI stuff in qapi/ui.json UI stuff is remote desktop stuff (Spice, VNC) and input stuff (mouse, keyboard). Cc: Gerd Hoffmann <kraxel@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <1503602048-12268-9-git-send-email-armbru@redhat.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Gerd Hoffmann <kraxel@redhat.com> commit 912092b8e47f31c3db25e088af8460d9e752da29 Author: Gerd Hoffmann <kraxel@redhat.com> Date: Thu Jul 27 12:47:20 2017 +0200 ui: drop altgr and altgr_r QKeyCodes The right alt key (alt_r aka KEY_RIGHTALT) is used for AltGr. The altgr and altgr_r keys simply don't exist. Drop them. Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-id: 20170727104720.30061-1-kraxel@redhat.com ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] net: Drop the legacy "name" parameter from the -net option 2019-12-10 11:06 ` Markus Armbruster @ 2019-12-10 11:28 ` Thomas Huth 0 siblings, 0 replies; 10+ messages in thread From: Thomas Huth @ 2019-12-10 11:28 UTC (permalink / raw) To: Markus Armbruster; +Cc: libvir-list, Jason Wang, qemu-devel On 10/12/2019 12.06, Markus Armbruster wrote: > Thomas Huth <thuth@redhat.com> writes: > >> It's been deprecated since QEMU v3.1, so it's time to finally >> remove it. The "id" parameter can simply be used instead. >> >> Signed-off-by: Thomas Huth <thuth@redhat.com> > [...] >> diff --git a/qapi/net.json b/qapi/net.json >> index 335295be50..ff280ccd16 100644 >> --- a/qapi/net.json >> +++ b/qapi/net.json >> @@ -488,18 +488,16 @@ >> # >> # @id: identifier for monitor commands >> # >> -# @name: identifier for monitor commands, ignored if @id is present >> -# >> # @opts: device type specific properties (legacy) >> # >> # Since: 1.2 >> # >> # 'vlan': dropped in 3.0 >> +# 'name': dropped in 5.0 >> ## > > Uh, when did we start to add "dropped in" to our doc comments? > > We should either do this systematically, or not at all. If the former, > we have quite a few "dropped in" to add belatedly. IIRC this has been suggested by Eric, see e.g.: https://patchwork.kernel.org/patch/10227335/#21501161 Anyway, it should not matter much for this patch here, since the line gets removed again in the 2nd patch anyway. Thomas ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] net: Drop the NetLegacy structure, always use Netdev instead 2019-12-06 5:36 [PATCH 0/2] net: Drop legacy "name" from -net and remove NetLegacy Thomas Huth 2019-12-06 5:36 ` [PATCH 1/2] net: Drop the legacy "name" parameter from the -net option Thomas Huth @ 2019-12-06 5:36 ` Thomas Huth 2019-12-06 15:14 ` Eric Blake 2019-12-10 11:09 ` Markus Armbruster 2020-01-29 12:42 ` [PATCH 0/2] net: Drop legacy "name" from -net and remove NetLegacy Thomas Huth 2 siblings, 2 replies; 10+ messages in thread From: Thomas Huth @ 2019-12-06 5:36 UTC (permalink / raw) To: qemu-devel, Jason Wang, Eric Blake, Markus Armbruster; +Cc: libvir-list 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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] net: Drop the NetLegacy structure, always use Netdev instead 2019-12-06 5:36 ` [PATCH 2/2] net: Drop the NetLegacy structure, always use Netdev instead Thomas Huth @ 2019-12-06 15:14 ` Eric Blake 2019-12-09 11:33 ` Thomas Huth 2019-12-10 11:09 ` Markus Armbruster 1 sibling, 1 reply; 10+ messages in thread From: Eric Blake @ 2019-12-06 15:14 UTC (permalink / raw) To: Thomas Huth, qemu-devel, Jason Wang, Markus Armbruster; +Cc: libvir-list 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] net: Drop the NetLegacy structure, always use Netdev instead 2019-12-06 15:14 ` Eric Blake @ 2019-12-09 11:33 ` Thomas Huth 0 siblings, 0 replies; 10+ messages in thread From: Thomas Huth @ 2019-12-09 11:33 UTC (permalink / raw) To: Eric Blake, qemu-devel, Jason Wang, Markus Armbruster; +Cc: 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] net: Drop the NetLegacy structure, always use Netdev instead 2019-12-06 5:36 ` [PATCH 2/2] net: Drop the NetLegacy structure, always use Netdev instead Thomas Huth 2019-12-06 15:14 ` Eric Blake @ 2019-12-10 11:09 ` Markus Armbruster 1 sibling, 0 replies; 10+ messages in thread From: Markus Armbruster @ 2019-12-10 11:09 UTC (permalink / raw) To: Thomas Huth; +Cc: libvir-list, Jason Wang, qemu-devel 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. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/2] net: Drop legacy "name" from -net and remove NetLegacy 2019-12-06 5:36 [PATCH 0/2] net: Drop legacy "name" from -net and remove NetLegacy Thomas Huth 2019-12-06 5:36 ` [PATCH 1/2] net: Drop the legacy "name" parameter from the -net option Thomas Huth 2019-12-06 5:36 ` [PATCH 2/2] net: Drop the NetLegacy structure, always use Netdev instead Thomas Huth @ 2020-01-29 12:42 ` Thomas Huth 2 siblings, 0 replies; 10+ messages in thread From: Thomas Huth @ 2020-01-29 12:42 UTC (permalink / raw) To: qemu-devel, Jason Wang On 06/12/2019 06.36, Thomas Huth wrote: > It's time to remove the deprecated "name" parameter from -net. > > Please have a closer look at the second patch ... I think it should be > fine, but I'm not 100% sure whether it's ok for all cases to drop NetLegacy, > so please double-check. > > Thomas Huth (2): > net: Drop the legacy "name" parameter from the -net option > net: Drop the NetLegacy structure, always use Netdev instead > > net/net.c | 84 +++++++------------------------------------- > qapi/net.json | 53 +--------------------------- > qemu-deprecated.texi | 12 ++++--- > 3 files changed, 20 insertions(+), 129 deletions(-) Ping? Thomas ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-01-29 12:43 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-12-06 5:36 [PATCH 0/2] net: Drop legacy "name" from -net and remove NetLegacy Thomas Huth 2019-12-06 5:36 ` [PATCH 1/2] net: Drop the legacy "name" parameter from the -net option Thomas Huth 2019-12-06 15:07 ` Eric Blake 2019-12-10 11:06 ` Markus Armbruster 2019-12-10 11:28 ` Thomas Huth 2019-12-06 5:36 ` [PATCH 2/2] net: Drop the NetLegacy structure, always use Netdev instead Thomas Huth 2019-12-06 15:14 ` Eric Blake 2019-12-09 11:33 ` Thomas Huth 2019-12-10 11:09 ` Markus Armbruster 2020-01-29 12:42 ` [PATCH 0/2] net: Drop legacy "name" from -net and remove NetLegacy Thomas Huth
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).