Re: [Qemu-devel] [Avocado-devel] [PATCH] net/slirp: Tell the users when they are using deprecated options

[Jason Wang <jasowang@redhat.com>]

On 01/13/2016 10:58 AM, Jason Wang wrote:
>
> On 01/12/2016 07:45 PM, Cleber Rosa wrote:
>> ----- Original Message -----
>>> From: "Thomas Huth" <thuth@redhat.com>
>>> To: "Jason Wang" <jasowang@redhat.com>, "Jan Kiszka" <jan.kiszka@siemens.com>, qemu-devel@nongnu.org
>>> Cc: "Paolo Bonzini" <pbonzini@redhat.com>, avocado-devel@redhat.com, "Markus Armbruster" <armbru@redhat.com>
>>> Sent: Tuesday, January 12, 2016 5:26:36 AM
>>> Subject: Re: [Qemu-devel] [PATCH] net/slirp: Tell the users when they are using deprecated options
>>>
>>> On 12.01.2016 04:42, Jason Wang wrote:
>>>> On 01/11/2016 11:55 PM, Thomas Huth wrote:
>>>>> On 15.12.2015 09:35, Thomas Huth wrote:
>>>>>> We don't want to support the legacy -tftp, -bootp, -smb and
>>>>>> -net channel options forever. So let's start telling the users
>>>>>> that they will go away in a future version.
>>>>>>
>>>>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>>>>> ---
>>>>>>  net/slirp.c | 3 +++
>>>>>>  os-posix.c  | 3 +++
>>>>>>  vl.c        | 6 ++++++
>>>>>>  3 files changed, 12 insertions(+)
>>>>>>
>>>>>> diff --git a/net/slirp.c b/net/slirp.c
>>>>>> index f505570..65e3766 100644
>>>>>> --- a/net/slirp.c
>>>>>> +++ b/net/slirp.c
>>>>>> @@ -784,6 +784,9 @@ int net_slirp_parse_legacy(QemuOptsList *opts_list,
>>>>>> const char *optarg, int *ret
>>>>>>          return 0;
>>>>>>      }
>>>>>>  
>>>>>> +    error_report("The -net channel option is deprecated and "
>>>>>> +                 "will be removed in a future version.");
>>>>>> +
>>>>>>      /* handle legacy -net channel,port:chr */
>>>>>>      optarg += strlen("channel,");
>>>>>>  
>>>>>> diff --git a/os-posix.c b/os-posix.c
>>>>>> index e4da406..3f62f7c 100644
>>>>>> --- a/os-posix.c
>>>>>> +++ b/os-posix.c
>>>>>> @@ -40,6 +40,7 @@
>>>>>>  #include "net/slirp.h"
>>>>>>  #include "qemu-options.h"
>>>>>>  #include "qemu/rcu.h"
>>>>>> +#include "qemu/error-report.h"
>>>>>>  
>>>>>>  #ifdef CONFIG_LINUX
>>>>>>  #include <sys/prctl.h>
>>>>>> @@ -139,6 +140,8 @@ void os_parse_cmd_args(int index, const char *optarg)
>>>>>>      switch (index) {
>>>>>>  #ifdef CONFIG_SLIRP
>>>>>>      case QEMU_OPTION_smb:
>>>>>> +        error_report("The -smb option is deprecated and "
>>>>>> +                     "will be removed in a future version.");
>>>>>>          if (net_slirp_smb(optarg) < 0)
>>>>>>              exit(1);
>>>>>>          break;
>>>>>> diff --git a/vl.c b/vl.c
>>>>>> index 4211ff1..fa829c0 100644
>>>>>> --- a/vl.c
>>>>>> +++ b/vl.c
>>>>>> @@ -3314,12 +3314,18 @@ int main(int argc, char **argv, char **envp)
>>>>>>  #endif
>>>>>>  #ifdef CONFIG_SLIRP
>>>>>>              case QEMU_OPTION_tftp:
>>>>>> +                error_report("The -tftp option is deprecated and "
>>>>>> +                             "will be removed in a future version.");
>>>>>>                  legacy_tftp_prefix = optarg;
>>>>>>                  break;
>>>>>>              case QEMU_OPTION_bootp:
>>>>>> +                error_report("The -bootp option is deprecated and "
>>>>>> +                             "will be removed in a future version.");
>>>>>>                  legacy_bootp_filename = optarg;
>>>>>>                  break;
>>>>>>              case QEMU_OPTION_redir:
>>>>>> +                error_report("The -redir option is deprecated and "
>>>>>> +                             "will be removed in a future version.");
>>>>>>                  if (net_slirp_redir(optarg) < 0)
>>>>>>                      exit(1);
>>>>>>                  break;
>>>>> *ping*
>>>>>
>>>>> Any comments on these old options? I hope Paolo does not want to keep
>>>>> them, too, forever ;-)
>>>> I vaguely remember autotest use those options in the past for guest os
>>>> installation. May need input from autotest guys.
>>> Looking at
>>>  https://github.com/avocado-framework/avocado-vt/blob/master/virttest/qemu_vm.py
>>> it seems like that latest incarnation of autotest is doing it
>>> right already:
>>> ...
>>>         def add_tftp(devices, filename):
>>>             # If the new syntax is supported, don't add -tftp
>>>             if "[,tftp=" in devices.get_help_text():
>>>                 return ""
>>>             else:
>>>                 return " -tftp '%s'" % filename
>>> ...
>>>         def add_net(devices, vlan, nettype, ifname=None, tftp=None,
>>>                     bootfile=None, hostfwd=[], netdev_id=None,
>>>                     netdev_extra_params=None, tapfds=None, script=None,
>>>                     downscript=None, vhost=None, queues=None, vhostfds=None,
>>>                     add_queues=None, helper=None, add_tapfd=None,
>>>                     add_vhostfd=None, vhostforce=None):
>>> ...
>>>            elif mode == "user":
>>>                 if tftp and "[,tftp=" in devices.get_help_text():
>>>                     cmd += ",tftp='%s'" % tftp
>>>                     cmd_nd = cmd
>>> ... etc ...
>>>
>>> So I think that avocado should be fine already.
>> It should.
>>
>> Avocado-VT can usually handle whatever version of QEMU is thrown at it
>> because it usually checks for specific features and command syntax.
>>
>> So, if a feature is removed and Avocado-VT is not handling that properly,
>> then it's a bug on "our" side.
>>
>> Cleber.
> Sounds very cool.
>
> Apply this to my -net.
>
> Thanks everyone.

Rethinking about this.  I'm not quite sure we can remove those
especially "-net user". Google qemu "-net user" gives about 15,900
results (and the first link is qemu wiki). Maybe we can replace "will be
removed in a future version." with something like "was suggested to use
-netdev user,XXX" instead?

>
>>>  Thomas
>>>
>>>
>>>
> _______________________________________________
> Avocado-devel mailing list
> Avocado-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/avocado-devel