qemu-devel
[Top][All Lists]
Advanced

[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: Thu, 20 Sep 2018 08:07:29 +0200
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux)

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.

> Signed-off-by: Thomas Huth <address@hidden>
> ---
>  Makefile             |  2 +-
>  net/net.c            |  4 ++++
>  net/slirp.c          |  2 ++
>  qemu-deprecated.texi | 13 +++++++++++++
>  4 files changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index fe623e4..f42e176 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -978,7 +978,7 @@ txt: qemu-doc.txt docs/interop/qemu-qmp-ref.txt 
> docs/interop/qemu-ga-ref.txt
>  
>  qemu-doc.html qemu-doc.info qemu-doc.pdf qemu-doc.txt: \
>       qemu-img.texi qemu-nbd.texi qemu-options.texi qemu-option-trace.texi \
> -     qemu-monitor.texi qemu-img-cmds.texi qemu-ga.texi \
> +     qemu-deprecated.texi qemu-monitor.texi qemu-img-cmds.texi qemu-ga.texi \
>       qemu-monitor-info.texi docs/qemu-block-drivers.texi \
>       docs/qemu-cpu-models.texi
>  

Missed in commit 44c67847e32 by yours truly.

> diff --git a/net/net.c b/net/net.c
> index 2a31339..cdcd5cf 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -984,6 +984,10 @@ static int net_client_init1(const void *object, bool 
> is_netdev, Error **errp)
>          /* 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) {
>          case NET_LEGACY_OPTIONS_TYPE_NONE:
> diff --git a/net/slirp.c b/net/slirp.c
> index c18060f..073bc69 100644
> --- a/net/slirp.c
> +++ b/net/slirp.c
> @@ -404,6 +404,8 @@ static SlirpState *slirp_lookup(Monitor *mon, const char 
> *hub_id,
>                  monitor_printf(mon, "unrecognized (hub-id, stackname) 
> pair\n");
>                  return NULL;
>              }
> +            warn_report("Using 'hub-id' is deprecated. Specify the netdev id 
> "
> +                        "directly instead");

Comma instead of period, please.

>          } else {
>              nc = qemu_find_netdev(name);
>              if (!nc) {
> diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
> index 8a2e399..eed7132 100644
> --- a/qemu-deprecated.texi
> +++ b/qemu-deprecated.texi
> @@ -83,6 +83,11 @@ 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.
>  
> address@hidden -net ...,address@hidden (since 3.1)
> +
> +The @option{name} parameter of the @option{-net} option is an old synonym
> +for the @option{id} parameter which should now be used instead.
> +

Suggest to scratch "old".  Comma before "which".

>  @section QEMU Machine Protocol (QMP) commands
>  
>  @subsection block-dirty-bitmap-add "autoload" parameter (since 2.12.0)
> @@ -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}.

> +
>  @section System emulator devices
>  
>  @subsection ivshmem (since 2.6.0)



reply via email to

[Prev in Thread] Current Thread [Next in Thread]