[PATCH v2 1/2] net: Drop the legacy "name" parameter from the -net option

Thomas Huth posted 2 patches 5 years, 6 months ago
Maintainers: Eric Blake <eblake@redhat.com>, Markus Armbruster <armbru@redhat.com>, Jason Wang <jasowang@redhat.com>
There is a newer version of this series
[PATCH v2 1/2] net: Drop the legacy "name" parameter from the -net option
Posted by Thomas Huth 5 years, 6 months ago
It's been deprecated since QEMU v3.1, so it's time to finally
remove it. The "id" parameter can simply be used instead.

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 docs/system/deprecated.rst | 15 +++++++++------
 net/net.c                  | 10 +---------
 qapi/net.json              |  3 ---
 3 files changed, 10 insertions(+), 18 deletions(-)

diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index 3142fac386..1b87c8f0db 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -47,12 +47,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.
 
-``-net ...,name=``\ *name* (since 3.1)
-''''''''''''''''''''''''''''''''''''''
-
-The ``name`` parameter of the ``-net`` option is a synonym
-for the ``id`` parameter, which should now be used instead.
-
 ``-smp`` (invalid topologies) (since 3.1)
 '''''''''''''''''''''''''''''''''''''''''
 
@@ -470,6 +464,15 @@ 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.
 
+System emulator command line arguments
+--------------------------------------
+
+``-net ...,name=``\ *name* (removed in 5.1)
+'''''''''''''''''''''''''''''''''''''''''''
+
+The ``name`` parameter of the ``-net`` option was a synonym
+for the ``id`` parameter, which should now be used instead.
+
 QEMU Machine Protocol (QMP) commands
 ------------------------------------
 
diff --git a/net/net.c b/net/net.c
index 38778e831d..a24da66e66 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 cebb1b52e3..fc7c95f6d8 100644
--- a/qapi/net.json
+++ b/qapi/net.json
@@ -474,8 +474,6 @@
 #
 # @id: identifier for monitor commands
 #
-# @name: identifier for monitor commands, ignored if @id is present
-#
 # @opts: device type specific properties (legacy)
 #
 # Since: 1.2
@@ -483,7 +481,6 @@
 { 'struct': 'NetLegacy',
   'data': {
     '*id':   'str',
-    '*name': 'str',
     'opts':  'NetLegacyOptions' } }
 
 ##
-- 
2.18.1


Re: [PATCH v2 1/2] net: Drop the legacy "name" parameter from the -net option
Posted by Eric Blake 5 years, 6 months ago
On 5/12/20 7:31 AM, 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.
> 
> Reviewed-by: Eric Blake <eblake@redhat.com>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   docs/system/deprecated.rst | 15 +++++++++------
>   net/net.c                  | 10 +---------
>   qapi/net.json              |  3 ---
>   3 files changed, 10 insertions(+), 18 deletions(-)
> 

> +++ b/qapi/net.json
> @@ -474,8 +474,6 @@
>   #
>   # @id: identifier for monitor commands
>   #
> -# @name: identifier for monitor commands, ignored if @id is present
> -#
>   # @opts: device type specific properties (legacy)
>   #
>   # Since: 1.2
> @@ -483,7 +481,6 @@
>   { 'struct': 'NetLegacy',
>     'data': {
>       '*id':   'str',
> -    '*name': 'str',
>       'opts':  'NetLegacyOptions' } }

Why is 'id' left optional? I'd expect it to be mandatory, now.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


Re: [PATCH v2 1/2] net: Drop the legacy "name" parameter from the -net option
Posted by Thomas Huth 5 years, 6 months ago
On 12/05/2020 16.26, Eric Blake wrote:
> On 5/12/20 7:31 AM, 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.
>>
>> Reviewed-by: Eric Blake <eblake@redhat.com>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>   docs/system/deprecated.rst | 15 +++++++++------
>>   net/net.c                  | 10 +---------
>>   qapi/net.json              |  3 ---
>>   3 files changed, 10 insertions(+), 18 deletions(-)
>>
> 
>> +++ b/qapi/net.json
>> @@ -474,8 +474,6 @@
>>   #
>>   # @id: identifier for monitor commands
>>   #
>> -# @name: identifier for monitor commands, ignored if @id is present
>> -#
>>   # @opts: device type specific properties (legacy)
>>   #
>>   # Since: 1.2
>> @@ -483,7 +481,6 @@
>>   { 'struct': 'NetLegacy',
>>     'data': {
>>       '*id':   'str',
>> -    '*name': 'str',
>>       'opts':  'NetLegacyOptions' } }
> 
> Why is 'id' left optional? I'd expect it to be mandatory, now.

That's because it is still optional, indeed. It is currently perfectly
valid to run "qemu-system-xyz -net user -net nic" without specifying any
"id". If I remove the "*" here, the old syntax breaks. I don't think
that we want this here, so "*id" has to stay optional.
(FWIW, the ID is then created internally, see assign_name() in net/net.c)

 Thomas