diff mbox

net: Inform the user about deprecated -net options

Message ID 1450179992-15959-1-git-send-email-thuth@redhat.com
State New
Headers show

Commit Message

Thomas Huth Dec. 15, 2015, 11:46 a.m. UTC
We likely don't want to maintain the legacy -net options forever,
so let's start informing the users that this option will be
removed in a future version. However, there are two cases where
we should not spill out a message yet: First is the "-net nic"
option which might still be required to configure on-board NICs
that can not be handled via "-netdev" yet, and second is the
"-net user" default option that is created automatically when
no other networking option has been specified.

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

Comments

Paolo Bonzini Dec. 15, 2015, 12:51 p.m. UTC | #1
On 15/12/2015 12:46, Thomas Huth wrote:
> We likely don't want to maintain the legacy -net options forever,
> so let's start informing the users that this option will be
> removed in a future version. However, there are two cases where
> we should not spill out a message yet: First is the "-net nic"
> option which might still be required to configure on-board NICs
> that can not be handled via "-netdev" yet, and second is the
> "-net user" default option that is created automatically when
> no other networking option has been specified.
> 
> 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 ade6051..2593961 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -1006,6 +1006,12 @@ static int net_client_init1(const void *object, int is_netdev, Error **errp)
>              return -1;
>          }
>  
> +        if (opts->type != NET_CLIENT_OPTIONS_KIND_NIC &&
> +            (!default_net || opts->type != NET_CLIENT_OPTIONS_KIND_USER)) {
> +            error_report("Deprecated net option - "
> +                         "this will be removed in a future version!");
> +        }

Honestly, I still do not believe that they will be removed.  They are
little more than syntactic sugar at this point.

Paolo

>          if (!net_client_init_fun[opts->type]) {
>              error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "type",
>                         "a net backend type (maybe it is not compiled "
>
Thomas Huth Dec. 15, 2015, 4:01 p.m. UTC | #2
On 15/12/15 13:51, Paolo Bonzini wrote:
> 
> On 15/12/2015 12:46, Thomas Huth wrote:
>> We likely don't want to maintain the legacy -net options forever,
>> so let's start informing the users that this option will be
>> removed in a future version. However, there are two cases where
>> we should not spill out a message yet: First is the "-net nic"
>> option which might still be required to configure on-board NICs
>> that can not be handled via "-netdev" yet, and second is the
>> "-net user" default option that is created automatically when
>> no other networking option has been specified.
>>
>> 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 ade6051..2593961 100644
>> --- a/net/net.c
>> +++ b/net/net.c
>> @@ -1006,6 +1006,12 @@ static int net_client_init1(const void *object, int is_netdev, Error **errp)
>>              return -1;
>>          }
>>  
>> +        if (opts->type != NET_CLIENT_OPTIONS_KIND_NIC &&
>> +            (!default_net || opts->type != NET_CLIENT_OPTIONS_KIND_USER)) {
>> +            error_report("Deprecated net option - "
>> +                         "this will be removed in a future version!");
>> +        }
> 
> Honestly, I still do not believe that they will be removed.  They are
> little more than syntactic sugar at this point.

As far as I can tell, that "-net" vs. "-netdev" dualism causes quite often confusion
for the users. Some options work with "-net", some only work with "-netdev",
and the ones that work with both often also behave slightly differently (see
[1] for example). I've already had to deal with a bug ticket in this area, and
it's just cumbersome to always find out and explain the differences.

One other example is the "-net nic,model=?" help text. It is inaccurate for a
couple of machines - and if somebody tries to fix problems like this, you're
often told something like "oh, it's the legacy -net option, simply forget about
that" [2].

And if you additionally ever had to deal with all that vlan code and duplicated
option parsing stuff in the net/ code, then you certainly do not think anymore
that this is just a little bit more than "syntactic sugar".

I'm fine if we keep the "-net" options for a couple of more versions of QEMU,
but we should be prepared to be able to remove it quickly once it is getting into
the way again too much. So we better start nagging the users about "-net" being
deprecated now, than discovering later that we have to deal with this legacy
stuff for longer than we would like to.

 Thomas


[1] http://www.linuxquestions.org/questions/linux-virtualization-and-cloud-90/cannot-set-mac-address-with-qemu-4175490716/
[2] https://lists.gnu.org/archive/html/qemu-ppc/2013-10/msg00088.html
Peter Maydell Dec. 15, 2015, 4:17 p.m. UTC | #3
On 15 December 2015 at 12:51, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Honestly, I still do not believe that they will be removed.  They are
> little more than syntactic sugar at this point.

If they're just syntactic sugar, what is the new-style command
line mechanism for configuring boards with embedded network
controller devices? Last time I looked at this that was just
impossible...and as long as it is, you can't get rid of any
of the backend -net config either, because it has to be either
all -net or all new-style and not a mix (I think).

thanks
-- PMM
Paolo Bonzini Dec. 15, 2015, 4:21 p.m. UTC | #4
On 15/12/2015 17:01, Thomas Huth wrote:
> Some options work with "-net", some only work with "-netdev",
> and the ones that work with both often also behave slightly differently (see
> [1] for example).

This example is about -net nic, which you're keeping.  What are the
options that differ between them for network backends?

> One other example is the "-net nic,model=?" help text. It is inaccurate for a
> couple of machines - and if somebody tries to fix problems like this, you're
> often told something like "oh, it's the legacy -net option, simply forget about
> that" [2].

Nobody mentioned legacy in that thread...  Alex just said *he* would not
bother, but if you could come up with a better way to do it, it would
surely be accepted.

For example you could print all DEVICE_CATEGORY_NETWORK devices that
support device_add.

> And if you additionally ever had to deal with all that vlan code and duplicated
> option parsing stuff in the net/ code, then you certainly do not think anymore
> that this is just a little bit more than "syntactic sugar".

In fact there isn't much shared code in the is_netdev=0 and is_netdev=1
cases.  Perhaps you could just make a shared function with just

    if (net_client_init_fun[opts->type](opts, name, peer, errp) < 0) {
        /* FIXME drop when all init functions store an Error */
        if (errp && !*errp) {
            error_setg(errp, QERR_DEVICE_INIT_FAILED,
                       NetClientOptionsKind_lookup[opts->type]);
        }
        return -1;
    }


and inline all the rest of net_client_init1, net_visit, net_client_init
into two functions netdev_add and net_legacy_add.  Then -net handling
(including HMP) can be moved into a separate file which no one looks at.

> I'm fine if we keep the "-net" options for a couple of more versions of QEMU,
> but we should be prepared to be able to remove it quickly once it is getting into
> the way again too much. So we better start nagging the users about "-net" being
> deprecated now, than discovering later that we have to deal with this legacy
> stuff for longer than we would like to.

The thing is, people are still running QEMU from the command line.

"-net nic -net bridge,br=virbr0" is still much less of a mouthful than
"-netdev bridge,br=virbr0,id=br -device rtl8139,netdev=br" if all I want
is something I can ssh into.

It's easy to deprecate things.  It's hard to convince users that it's
worth, and you haven't convinced this user. :)

Paolo
Thomas Huth Dec. 15, 2015, 4:33 p.m. UTC | #5
On 15/12/15 17:17, Peter Maydell wrote:
> On 15 December 2015 at 12:51, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> Honestly, I still do not believe that they will be removed.  They are
>> little more than syntactic sugar at this point.
> 
> If they're just syntactic sugar, what is the new-style command
> line mechanism for configuring boards with embedded network
> controller devices? Last time I looked at this that was just
> impossible...and as long as it is, you can't get rid of any
> of the backend -net config either, because it has to be either
> all -net or all new-style and not a mix (I think).

No, you can use "-netdev user,id=mynet -net nic,netdev=mynet" for
example to wire an embedded network controller to a netdev device, so
that's not a problem anymore.

 Thomas
Peter Maydell Dec. 15, 2015, 4:46 p.m. UTC | #6
On 15 December 2015 at 16:33, Thomas Huth <thuth@redhat.com> wrote:
> On 15/12/15 17:17, Peter Maydell wrote:
>> If they're just syntactic sugar, what is the new-style command
>> line mechanism for configuring boards with embedded network
>> controller devices? Last time I looked at this that was just
>> impossible...and as long as it is, you can't get rid of any
>> of the backend -net config either, because it has to be either
>> all -net or all new-style and not a mix (I think).
>
> No, you can use "-netdev user,id=mynet -net nic,netdev=mynet" for
> example to wire an embedded network controller to a netdev device, so
> that's not a problem anymore.

Oh good. We should probably document this on
http://wiki.qemu.org/Documentation/Networking somewhere.

thanks
-- PMM
Eric Blake Dec. 15, 2015, 4:46 p.m. UTC | #7
On 12/15/2015 09:21 AM, Paolo Bonzini wrote:

> 
> In fact there isn't much shared code in the is_netdev=0 and is_netdev=1
> cases.  Perhaps you could just make a shared function with just
> 
>     if (net_client_init_fun[opts->type](opts, name, peer, errp) < 0) {
>         /* FIXME drop when all init functions store an Error */
>         if (errp && !*errp) {
>             error_setg(errp, QERR_DEVICE_INIT_FAILED,
>                        NetClientOptionsKind_lookup[opts->type]);
>         }
>         return -1;
>     }
> 
> 
> and inline all the rest of net_client_init1, net_visit, net_client_init
> into two functions netdev_add and net_legacy_add.  Then -net handling
> (including HMP) can be moved into a separate file which no one looks at.

There's also some pending patches from qapi refactoring and from
Zoltan's work towards audio unification (which spilled over into netdev
refactoring), which may impact things here.  For my latest version, see:
http://repo.or.cz/qemu/ericb.git/shortlog/refs/heads/qapi
Thomas Huth Dec. 15, 2015, 5:15 p.m. UTC | #8
On 15/12/15 17:21, Paolo Bonzini wrote:
> 
> 
> On 15/12/2015 17:01, Thomas Huth wrote:
>> Some options work with "-net", some only work with "-netdev",
>> and the ones that work with both often also behave slightly differently (see
>> [1] for example).
> 
> This example is about -net nic, which you're keeping.  What are the
> options that differ between them for network backends?

The vlan option only exists for the "-net" backends. And I somehow
thought that all the vhost related stuff would only work with the
"-netdev", but I can not spot that in the code anymore, so I guess I
simply got that wrong.

>> One other example is the "-net nic,model=?" help text. It is inaccurate for a
>> couple of machines - and if somebody tries to fix problems like this, you're
>> often told something like "oh, it's the legacy -net option, simply forget about
>> that" [2].
> 
> Nobody mentioned legacy in that thread...  Alex just said *he* would not
> bother, but if you could come up with a better way to do it, it would
> surely be accepted.

Ok, maybe should have cited this one here instead:

https://lists.gnu.org/archive/html/qemu-ppc/2013-11/msg00040.html

Markus said: "My advice would be to let -net nic rot in peace."

> For example you could print all DEVICE_CATEGORY_NETWORK devices that
> support device_add.

I think that would not work since spapr-vlan is not hot-pluggable.

>> And if you additionally ever had to deal with all that vlan code and duplicated
>> option parsing stuff in the net/ code, then you certainly do not think anymore
>> that this is just a little bit more than "syntactic sugar".
> 
> In fact there isn't much shared code in the is_netdev=0 and is_netdev=1
> cases.  Perhaps you could just make a shared function with just
> 
>     if (net_client_init_fun[opts->type](opts, name, peer, errp) < 0) {
>         /* FIXME drop when all init functions store an Error */
>         if (errp && !*errp) {
>             error_setg(errp, QERR_DEVICE_INIT_FAILED,
>                        NetClientOptionsKind_lookup[opts->type]);
>         }
>         return -1;
>     }
> 
> 
> and inline all the rest of net_client_init1, net_visit, net_client_init
> into two functions netdev_add and net_legacy_add.  Then -net handling
> (including HMP) can be moved into a separate file which no one looks at.

Well, moving the code into a file "which no one looks at" does not
really solve the problem. When you want to modify the net code, you can
not simply ignore this since it is still there and needs to be kept
alive when it breaks or does not compile anymore.

>> I'm fine if we keep the "-net" options for a couple of more versions of QEMU,
>> but we should be prepared to be able to remove it quickly once it is getting into
>> the way again too much. So we better start nagging the users about "-net" being
>> deprecated now, than discovering later that we have to deal with this legacy
>> stuff for longer than we would like to.
> 
> The thing is, people are still running QEMU from the command line.
> 
> "-net nic -net bridge,br=virbr0" is still much less of a mouthful than
> "-netdev bridge,br=virbr0,id=br -device rtl8139,netdev=br" if all I want
> is something I can ssh into.
> 
> It's easy to deprecate things.  It's hard to convince users that it's
> worth, and you haven't convinced this user. :)

Ok, if you want to maintain that legacy stuff forever - fine. I thought
it would be a good idea to slowly get rid off it, but honestly, if
there's so much resistance ... I've also got other stuff to do, so
simply forget about this.

 Thomas
Paolo Bonzini Dec. 15, 2015, 5:31 p.m. UTC | #9
On 15/12/2015 18:15, Thomas Huth wrote:
> On 15/12/15 17:21, Paolo Bonzini wrote:
>> On 15/12/2015 17:01, Thomas Huth wrote:
>>> Some options work with "-net", some only work with "-netdev",
>>> and the ones that work with both often also behave slightly differently (see
>>> [1] for example).
>>
>> This example is about -net nic, which you're keeping.  What are the
>> options that differ between them for network backends?
> 
> The vlan option only exists for the "-net" backends.

Yup, vlans (implicit hubs) are the main difference between -net and
-netdev.  That's pretty much the only one, and it's not specific to
single backends.

> And I somehow
> thought that all the vhost related stuff would only work with the
> "-netdev", but I can not spot that in the code anymore, so I guess I
> simply got that wrong.

vhost probably doesn't start unless you use -netdev.  It's just ignored.
 But I may also be remembering wrong.

> Ok, maybe should have cited this one here instead:
> https://lists.gnu.org/archive/html/qemu-ppc/2013-11/msg00040.html
> 
> Markus said: "My advice would be to let -net nic rot in peace."

That would have meant not supporting "-net nic,model=ibmveth" to begin
with.  Too late, so a good solution would always be accepted.  We deal
with "legacy" stuff all the time, and sometime even improve it.

To some extent HMP is considered legacy for example, but no one in their
right minds would eliminate it.

>> For example you could print all DEVICE_CATEGORY_NETWORK devices that
>> support device_add.
> 
> I think that would not work since spapr-vlan is not hot-pluggable.

cannot_instantiate_with_device_add_yet which actually refers to -device,
not really device_add, so it would work.

The only issue would be that the default name is spapr-vlan rather than
ibmveth.  That would be not too bad as a backwards-incompatible change.

>> The thing is, people are still running QEMU from the command line.
>>
>> "-net nic -net bridge,br=virbr0" is still much less of a mouthful than
>> "-netdev bridge,br=virbr0,id=br -device rtl8139,netdev=br" if all I want
>> is something I can ssh into.
>>
>> It's easy to deprecate things.  It's hard to convince users that it's
>> worth, and you haven't convinced this user. :)
> 
> Ok, if you want to maintain that legacy stuff forever - fine.

I don't want to, but I think we have to.  It would be a good idea to get
rid of it, but unfortunately it's not a feasible idea IMHO.

> I thought it would be a good idea to slowly get rid off it, but honestly, if
> there's so much resistance ... I've also got other stuff to do, so
> simply forget about this.

I'm sorry.  It's just that _I_ would be a bit pissed off by it, even
though I type 500-characters QEMU command lines daily, therefore I think
end users would complain even more.  I would live with it, but for
regular users I don't think we would do them a great service.

It's the same as all the other shortcut options (-hda, -drive with
if!=none, -vga).  Many of them enjoy similar amounts of cruft, but they
cannot really go away. :(

Paolo
Thomas Huth Dec. 15, 2015, 5:50 p.m. UTC | #10
On 15/12/15 18:31, Paolo Bonzini wrote:
> 
> On 15/12/2015 18:15, Thomas Huth wrote:
>> On 15/12/15 17:21, Paolo Bonzini wrote:
[...]
>>> For example you could print all DEVICE_CATEGORY_NETWORK devices that
>>> support device_add.
>>
>> I think that would not work since spapr-vlan is not hot-pluggable.
> 
> cannot_instantiate_with_device_add_yet which actually refers to -device,
> not really device_add, so it would work.

I just tried:

(qemu) device_add spapr-vlan
Bus 'spapr-vio' does not support hotplugging

... did I miss something?

>>> The thing is, people are still running QEMU from the command line.
>>>
>>> "-net nic -net bridge,br=virbr0" is still much less of a mouthful than
>>> "-netdev bridge,br=virbr0,id=br -device rtl8139,netdev=br" if all I want
>>> is something I can ssh into.
>>>
>>> It's easy to deprecate things.  It's hard to convince users that it's
>>> worth, and you haven't convinced this user. :)
>>
>> Ok, if you want to maintain that legacy stuff forever - fine.
> 
> I don't want to, but I think we have to.  It would be a good idea to get
> rid of it, but unfortunately it's not a feasible idea IMHO.

If the code gets unmaintainable because there is too much legacy cruft
in it, that's IMHO even worse. At one point in time, you just have to
get rid of such stuff ... but well, maybe the time for the "-net" option
just has not come yet.

 Thomas
Paolo Bonzini Dec. 15, 2015, 6:08 p.m. UTC | #11
On 15/12/2015 18:50, Thomas Huth wrote:
> On 15/12/15 18:31, Paolo Bonzini wrote:
>>
>> On 15/12/2015 18:15, Thomas Huth wrote:
>>> On 15/12/15 17:21, Paolo Bonzini wrote:
> [...]
>>>> For example you could print all DEVICE_CATEGORY_NETWORK devices that
>>>> support device_add.
>>>
>>> I think that would not work since spapr-vlan is not hot-pluggable.
>>
>> cannot_instantiate_with_device_add_yet which actually refers to -device,
>> not really device_add, so it would work.
> 
> I just tried:
> 
> (qemu) device_add spapr-vlan
> Bus 'spapr-vio' does not support hotplugging
> 
> ... did I miss something?

When I said "support device_add" I really meant "does not have
cannot_instantiate_with_device_add_yet", but spapr-vlan should pass that
test.

>>>> The thing is, people are still running QEMU from the command line.
>>>>
>>>> "-net nic -net bridge,br=virbr0" is still much less of a mouthful than
>>>> "-netdev bridge,br=virbr0,id=br -device rtl8139,netdev=br" if all I want
>>>> is something I can ssh into.
>>>>
>>>> It's easy to deprecate things.  It's hard to convince users that it's
>>>> worth, and you haven't convinced this user. :)
>>>
>>> Ok, if you want to maintain that legacy stuff forever - fine.
>>
>> I don't want to, but I think we have to.  It would be a good idea to get
>> rid of it, but unfortunately it's not a feasible idea IMHO.
> 
> If the code gets unmaintainable because there is too much legacy cruft
> in it, that's IMHO even worse. At one point in time, you just have to
> get rid of such stuff ... but well, maybe the time for the "-net" option
> just has not come yet.

Yes, indeed.  I don't think it's unmaintainable.  It used to be much
worse, and it could be improved.

Paolo
Thomas Huth Dec. 16, 2015, 7:20 a.m. UTC | #12
On 15/12/15 19:08, Paolo Bonzini wrote:
> 
> 
> On 15/12/2015 18:50, Thomas Huth wrote:
>> On 15/12/15 18:31, Paolo Bonzini wrote:
>>>
>>> On 15/12/2015 18:15, Thomas Huth wrote:
>>>> On 15/12/15 17:21, Paolo Bonzini wrote:
>> [...]
>>>>> For example you could print all DEVICE_CATEGORY_NETWORK devices that
>>>>> support device_add.
>>>>
>>>> I think that would not work since spapr-vlan is not hot-pluggable.
>>>
>>> cannot_instantiate_with_device_add_yet which actually refers to -device,
>>> not really device_add, so it would work.
>>
>> I just tried:
>>
>> (qemu) device_add spapr-vlan
>> Bus 'spapr-vio' does not support hotplugging
>>
>> ... did I miss something?
> 
> When I said "support device_add" I really meant "does not have
> cannot_instantiate_with_device_add_yet", but spapr-vlan should pass that
> test.

Ah, ok, that makes more sense... anyway, "-device ?" also lists some
devices like "ne2k_isa", ""usb-bt-dongle" and "vmxnet3" ... I somewhat
doubt that we want to have these in the list of "-net nic" supported
devices, too.

... hmmm, by the way, why the heck do we have vmxnet3 on powerpc? Does
that make sense at all?

>>>>> The thing is, people are still running QEMU from the command line.
>>>>>
>>>>> "-net nic -net bridge,br=virbr0" is still much less of a mouthful than
>>>>> "-netdev bridge,br=virbr0,id=br -device rtl8139,netdev=br" if all I want
>>>>> is something I can ssh into.
>>>>>
>>>>> It's easy to deprecate things.  It's hard to convince users that it's
>>>>> worth, and you haven't convinced this user. :)

Just another idea before we drop this topic again completely: What if
we'd extend "-netdev" to be easier to use, too. For example, if you'd
just specify "-netdev bridge,br=virbr0", without using an id and without
specifying a "-device", you would get the netdev hooked up to the
board's default NIC automatically. That would be even less to type than
your example with "-net" since you would not need the "-net nic"
parameter in that case... Would it then be ok to deprecate the "-net"
option?

 Thomas
Paolo Bonzini Dec. 16, 2015, 8:50 a.m. UTC | #13
On 16/12/2015 08:20, Thomas Huth wrote:
> Ah, ok, that makes more sense... anyway, "-device ?" also lists some
> devices like "ne2k_isa", ""usb-bt-dongle" and "vmxnet3" ... I somewhat
> doubt that we want to have these in the list of "-net nic" supported
> devices, too.

Why not?

> ... hmmm, by the way, why the heck do we have vmxnet3 on powerpc? Does
> that make sense at all?

By default all PCI devices are included in all targets, that's the
simple explanation. :)

>>>>>> The thing is, people are still running QEMU from the command line.
>>>>>>
>>>>>> "-net nic -net bridge,br=virbr0" is still much less of a mouthful than
>>>>>> "-netdev bridge,br=virbr0,id=br -device rtl8139,netdev=br" if all I want
>>>>>> is something I can ssh into.
>>>>>>
>>>>>> It's easy to deprecate things.  It's hard to convince users that it's
>>>>>> worth, and you haven't convinced this user. :)
> 
> Just another idea before we drop this topic again completely: What if
> we'd extend "-netdev" to be easier to use, too. For example, if you'd
> just specify "-netdev bridge,br=virbr0", without using an id and without
> specifying a "-device", you would get the netdev hooked up to the
> board's default NIC automatically. That would be even less to type than
> your example with "-net" since you would not need the "-net nic"
> parameter in that case... Would it then be ok to deprecate the "-net"
> option?

Actually that would be the worst of both worlds. :)  The point of
-netdev is exactly to have no magic, to be a direct connection between
the command line and the devices.  It makes sense, it's just not too
user friendly.

I really think that if you move -net to net/netlegacy.c it wouldn't look
bad at all.

Paolo
diff mbox

Patch

diff --git a/net/net.c b/net/net.c
index ade6051..2593961 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1006,6 +1006,12 @@  static int net_client_init1(const void *object, int is_netdev, Error **errp)
             return -1;
         }
 
+        if (opts->type != NET_CLIENT_OPTIONS_KIND_NIC &&
+            (!default_net || opts->type != NET_CLIENT_OPTIONS_KIND_USER)) {
+            error_report("Deprecated net option - "
+                         "this will be removed in a future version!");
+        }
+
         if (!net_client_init_fun[opts->type]) {
             error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "type",
                        "a net backend type (maybe it is not compiled "