[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [QEMU PATCH] net: Deprecate the old way of using a lega
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [QEMU PATCH] net: Deprecate the old way of using a legacy net via "name" instead of "id" |
Date: |
Fri, 21 Sep 2018 10:08:51 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) |
Thomas Huth <address@hidden> writes:
> On 2018-09-20 08:07, Markus Armbruster wrote:
>> Thomas Huth <address@hidden> writes:
>>
>>> In early times, network backends were specified by a "vlan" and "name"
>>> tuple. With the introduction of netdevs, the "name" was replaced by an
>>> "id" (which is supposed to be unique), but the "name" parameter stayed
>>> as an alias which could be used instead of "id". Unfortunately, we miss
>>> the duplication check for "name":
>>>
>>> $ qemu-system-x86_64 -net user,name=n1 -net user,name=n1
>>>
>>> ... starts without an error, while "id" correctly complains:
>>>
>>> $ qemu-system-x86_64 -net user,id=n1 -net user,id=n1
>>> qemu-system-x86_64: -net user,id=n1: Duplicate ID 'n1' for net
>>>
>>> Instead of trying to fix the code for the legacy "name" parameter, let's
>>> rather get rid of this old interface and deprecate the "name" parameter
>>> now - this will also be less confusing for the users in the long run.
>>>
>>> While we're at it, also deprecate the old syntax for the hostfwd_add
>>> and hostfwd_remove commands that still work with this legacy "hub"
>>> plus "name" tuple. It is enough to specify the netdev id there instead.
>>>
>>> Also add a missing dependency to the Makefile to make sure that the
>>> docs get correctly regenerated when qemu-deprecated.texi is changed.
>>
>> Sure sounds like three patches to me.
>
> Since the changes are really small and slightly related, one compact
> patch sounded fine for me, too. But ok, I just discovered yet another
> unrelated thing that should be added to the deprecation chapter
> ("enforce-config-section"), so I'll turn this into a small patch series
> instead.
Thanks.
>>> @@ -99,6 +104,14 @@ The ``query-cpus'' command is replaced by the
>>> ``query-cpus-fast'' command.
>>> The ``arch'' output member of the ``query-cpus-fast'' command is
>>> replaced by the ``target'' output member.
>>>
>>> address@hidden System emulator human monitor commands
>>> +
>>> address@hidden The hub_id parameter of 'hostfwd_add' / 'hostfwd_remove'
>>> (since 3.1)
>>> +
>>> +The @option{hub_id} parameter of the 'hostfwd_add' and 'hostfwd_remove' HMP
>>> +commands is redundant. It is enough to specify the netdev ID of the backend
>>> +that should be changed.
>>
>> Hmm. What's redundant is the [hub_id name] alternative in
>>
>> .params = "[hub_id name]|[netdev_id] ..."
>>
>> The [netdev_id] alternative was added in January (commit 93653066445).
>> Suggest to word it more like other sections do:
>>
>> Parameters @option{hub_id} and @option{name} have been replaced by
>> @option{netdev_id}.
>
> Ok. The funny thing is that IIUC "name" and "netdev_id" are the very
> same identifier nowadays (I was not aware of this in the past), so the
> descriptions could also have been written as "[[hub_id] netdev_id]"
> instead. ==> This "name" vs. "id" thing is really confusing, it's really
> a good idea to remove it soon.
Much appreciated.