[Qemu-devel] [PATCH] net: Mark 'vlan' parameter as deprecated

[Qemu-devel] [PATCH] net: Mark 'vlan' parameter as deprecated

[Thomas Huth]

The 'vlan' parameter is a continuous source of confusion for the users,
many people mix it up with the more common term VLAN (the link layer
packet encapsulation), and even if they realize that the QEMU 'vlan' is
rather some kind of network hub emulation, there is still a high risk
that they configure their QEMU networking in a wrong way with this
parameter (e.g. by hooking NICs together, so they get a 'loopback'
between one and the other NIC).
Thus at one point in time, we should finally get rid of the 'vlan'
feature in QEMU. Let's do a first step in this direction by declaring
the 'vlan' parameter as deprecated and informing the users to use the
'netdev' parameter instead.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 net/net.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/net.c b/net/net.c
index 939fe31..fb7af3a 100644
--- a/net/net.c
+++ b/net/net.c
@@ -970,6 +970,7 @@ static int net_client_init1(const void *object, bool is_netdev, Error **errp)
     const Netdev *netdev;
     const char *name;
     NetClientState *peer = NULL;
+    static bool vlan_warned;
 
     if (is_netdev) {
         netdev = object;
@@ -1050,6 +1051,11 @@ static int net_client_init1(const void *object, bool is_netdev, Error **errp)
             !opts->u.nic.data->has_netdev) {
             peer = net_hub_add_port(net->has_vlan ? net->vlan : 0, NULL);
         }
+
+        if (net->has_vlan && !vlan_warned) {
+            error_report("'vlan' is deprecated. Please use 'netdev' instead.");
+            vlan_warned = true;
+        }
     }
 
     if (net_client_init_fun[netdev->type](netdev, name, peer, errp) < 0) {
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH] net: Mark 'vlan' parameter as deprecated

[Jason Wang]

On 2017年01月24日 17:42, Thomas Huth wrote:
> The 'vlan' parameter is a continuous source of confusion for the users,
> many people mix it up with the more common term VLAN (the link layer
> packet encapsulation), and even if they realize that the QEMU 'vlan' is
> rather some kind of network hub emulation, there is still a high risk
> that they configure their QEMU networking in a wrong way with this
> parameter (e.g. by hooking NICs together, so they get a 'loopback'
> between one and the other NIC).
> Thus at one point in time, we should finally get rid of the 'vlan'
> feature in QEMU. Let's do a first step in this direction by declaring
> the 'vlan' parameter as deprecated and informing the users to use the
> 'netdev' parameter instead.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   net/net.c | 6 ++++++
>   1 file changed, 6 insertions(+)
>
> diff --git a/net/net.c b/net/net.c
> index 939fe31..fb7af3a 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -970,6 +970,7 @@ static int net_client_init1(const void *object, bool is_netdev, Error **errp)
>       const Netdev *netdev;
>       const char *name;
>       NetClientState *peer = NULL;
> +    static bool vlan_warned;
>   
>       if (is_netdev) {
>           netdev = object;
> @@ -1050,6 +1051,11 @@ static int net_client_init1(const void *object, bool is_netdev, Error **errp)
>               !opts->u.nic.data->has_netdev) {
>               peer = net_hub_add_port(net->has_vlan ? net->vlan : 0, NULL);
>           }
> +
> +        if (net->has_vlan && !vlan_warned) {
> +            error_report("'vlan' is deprecated. Please use 'netdev' instead.");
> +            vlan_warned = true;
> +        }
>       }
>   
>       if (net_client_init_fun[netdev->type](netdev, name, peer, errp) < 0) {

Looks good, but do really want only warn once? Consider we have monitor 
command e.g "host_net_add".

Thanks

Re: [Qemu-devel] [PATCH] net: Mark 'vlan' parameter as deprecated

[Thomas Huth]

On 25.01.2017 08:50, Jason Wang wrote:
> 
> 
> On 2017年01月24日 17:42, Thomas Huth wrote:
>> The 'vlan' parameter is a continuous source of confusion for the users,
>> many people mix it up with the more common term VLAN (the link layer
>> packet encapsulation), and even if they realize that the QEMU 'vlan' is
>> rather some kind of network hub emulation, there is still a high risk
>> that they configure their QEMU networking in a wrong way with this
>> parameter (e.g. by hooking NICs together, so they get a 'loopback'
>> between one and the other NIC).
>> Thus at one point in time, we should finally get rid of the 'vlan'
>> feature in QEMU. Let's do a first step in this direction by declaring
>> the 'vlan' parameter as deprecated and informing the users to use the
>> 'netdev' parameter instead.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>   net/net.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/net/net.c b/net/net.c
>> index 939fe31..fb7af3a 100644
>> --- a/net/net.c
>> +++ b/net/net.c
>> @@ -970,6 +970,7 @@ static int net_client_init1(const void *object,
>> bool is_netdev, Error **errp)
>>       const Netdev *netdev;
>>       const char *name;
>>       NetClientState *peer = NULL;
>> +    static bool vlan_warned;
>>         if (is_netdev) {
>>           netdev = object;
>> @@ -1050,6 +1051,11 @@ static int net_client_init1(const void *object,
>> bool is_netdev, Error **errp)
>>               !opts->u.nic.data->has_netdev) {
>>               peer = net_hub_add_port(net->has_vlan ? net->vlan : 0,
>> NULL);
>>           }
>> +
>> +        if (net->has_vlan && !vlan_warned) {
>> +            error_report("'vlan' is deprecated. Please use 'netdev'
>> instead.");
>> +            vlan_warned = true;
>> +        }
>>       }
>>         if (net_client_init_fun[netdev->type](netdev, name, peer,
>> errp) < 0) {
> 
> Looks good, but do really want only warn once? Consider we have monitor
> command e.g "host_net_add".

I don't mind ... I can remove the "vlan_warned" check, but then you'll
get a lot of error messages at once if you start QEMU with multiple
"-net" parameters that use "vlan=...". Is that ok for you? If yes, I'll
send an updated v2 of my patch without that "vlan_warned" check.

 Thomas

Re: [Qemu-devel] [PATCH] net: Mark 'vlan' parameter as deprecated

[Jason Wang]

On 2017年01月25日 15:54, Thomas Huth wrote:
> On 25.01.2017 08:50, Jason Wang wrote:
>>
>> On 2017年01月24日 17:42, Thomas Huth wrote:
>>> The 'vlan' parameter is a continuous source of confusion for the users,
>>> many people mix it up with the more common term VLAN (the link layer
>>> packet encapsulation), and even if they realize that the QEMU 'vlan' is
>>> rather some kind of network hub emulation, there is still a high risk
>>> that they configure their QEMU networking in a wrong way with this
>>> parameter (e.g. by hooking NICs together, so they get a 'loopback'
>>> between one and the other NIC).
>>> Thus at one point in time, we should finally get rid of the 'vlan'
>>> feature in QEMU. Let's do a first step in this direction by declaring
>>> the 'vlan' parameter as deprecated and informing the users to use the
>>> 'netdev' parameter instead.
>>>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>> ---
>>>    net/net.c | 6 ++++++
>>>    1 file changed, 6 insertions(+)
>>>
>>> diff --git a/net/net.c b/net/net.c
>>> index 939fe31..fb7af3a 100644
>>> --- a/net/net.c
>>> +++ b/net/net.c
>>> @@ -970,6 +970,7 @@ static int net_client_init1(const void *object,
>>> bool is_netdev, Error **errp)
>>>        const Netdev *netdev;
>>>        const char *name;
>>>        NetClientState *peer = NULL;
>>> +    static bool vlan_warned;
>>>          if (is_netdev) {
>>>            netdev = object;
>>> @@ -1050,6 +1051,11 @@ static int net_client_init1(const void *object,
>>> bool is_netdev, Error **errp)
>>>                !opts->u.nic.data->has_netdev) {
>>>                peer = net_hub_add_port(net->has_vlan ? net->vlan : 0,
>>> NULL);
>>>            }
>>> +
>>> +        if (net->has_vlan && !vlan_warned) {
>>> +            error_report("'vlan' is deprecated. Please use 'netdev'
>>> instead.");
>>> +            vlan_warned = true;
>>> +        }
>>>        }
>>>          if (net_client_init_fun[netdev->type](netdev, name, peer,
>>> errp) < 0) {
>> Looks good, but do really want only warn once? Consider we have monitor
>> command e.g "host_net_add".
> I don't mind ... I can remove the "vlan_warned" check, but then you'll
> get a lot of error messages at once if you start QEMU with multiple
> "-net" parameters that use "vlan=...". Is that ok for you? If yes, I'll
> send an updated v2 of my patch without that "vlan_warned" check.
>
>   Thomas
>

You're right, this version looks better, applied to -net.

Thanks