[Qemu-devel] [PATCH] net: Improve the warnings for dubious command line option combinations

[Qemu-devel] [PATCH] net: Improve the warnings for dubious command line option combinations

[Peter Maydell]

Improve the warnings we give if the user specified a combination of -net
options which don't make much sense:
 * Fix a bug where we would only complain about the first VLAN having
   no NIC or no host network connection; we now diagnose this situation
   for all VLANs
 * Don't warn about anything if the config is the implicit default
   "-net user -net nic" rather than one specified by the user (this will
   only kick in for boards with no NIC or if CONFIG_SLIRP is not set)
 * Diagnose the case where the user asked for NICs which the board
   didn't instantiate (for example where the user asked for two NICs
   but the board only supports one)

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
The motivation for this patch is that I thought it made more sense
to complain about unused NIC specifications in generic code than
force every board to do it; see discussion of the vexpress patch
http://patchwork.ozlabs.org/patch/85727/

 net.c |   34 +++++++++++++++++++++++++++++++++-
 1 files changed, 33 insertions(+), 1 deletions(-)

diff --git a/net.c b/net.c
index ddcca97..9d3aaf5 100644
--- a/net.c
+++ b/net.c
@@ -1305,12 +1305,30 @@ void net_check_clients(void)
 {
     VLANState *vlan;
     VLANClientState *vc;
-    int has_nic = 0, has_host_dev = 0;
+    int has_nic, has_host_dev;
+    int seen_nics = 0;
+
+    /* Don't warn about the default network setup that you get if
+     * no command line -net options are specified. There are two
+     * cases that we would otherwise complain about:
+     * (1) board doesn't support a NIC but the implicit "-net nic"
+     * requested one; we'd otherwise complain about more NICs being
+     * specified than we support, and also that the vlan set up by
+     * the implicit "-net user" didn't have any NICs connected to it
+     * (2) CONFIG_SLIRP not set: we'd otherwise complain about the
+     * implicit "-net nic" setting up a nic that wasn't connected to
+     * anything.
+     */
+    if (default_net) {
+        return;
+    }
 
     QTAILQ_FOREACH(vlan, &vlans, next) {
+        has_nic = has_host_dev = 0;
         QTAILQ_FOREACH(vc, &vlan->clients, next) {
             switch (vc->info->type) {
             case NET_CLIENT_TYPE_NIC:
+                seen_nics++;
                 has_nic = 1;
                 break;
             case NET_CLIENT_TYPE_SLIRP:
@@ -1330,12 +1348,26 @@ void net_check_clients(void)
                     vlan->id);
     }
     QTAILQ_FOREACH(vc, &non_vlan_clients, next) {
+        if (vc->info->type == NET_CLIENT_TYPE_NIC) {
+            seen_nics++;
+        }
         if (!vc->peer) {
             fprintf(stderr, "Warning: %s %s has no peer\n",
                     vc->info->type == NET_CLIENT_TYPE_NIC ? "nic" : "netdev",
                     vc->name);
         }
     }
+    if (seen_nics != nb_nics) {
+        /* Number of NICs requested by user on command line doesn't match
+         * the number the model actually registered with us.
+         * This will generally only happen for models of embedded boards
+         * with no PCI bus or similar. PCI based machines can instantiate
+         * all requested NICs as PCI devices but usually embedded boards
+         * only have a single NIC.
+         */
+        fprintf(stderr, "Warning: more nics requested than this machine "
+                "supports; some have been ignored\n");
+    }
 }
 
 static int net_init_client(QemuOpts *opts, void *dummy)
-- 
1.7.1

Re: [Qemu-devel] [PATCH] net: Improve the warnings for dubious command line option combinations

[Peter Maydell]

Ping? It would be nice if this could be committed, it's blocking
the versatile express support patch.

thanks
-- PMM

On 22 March 2011 18:39, Peter Maydell <peter.maydell@linaro.org> wrote:
> Improve the warnings we give if the user specified a combination of -net
> options which don't make much sense:
>  * Fix a bug where we would only complain about the first VLAN having
>   no NIC or no host network connection; we now diagnose this situation
>   for all VLANs
>  * Don't warn about anything if the config is the implicit default
>   "-net user -net nic" rather than one specified by the user (this will
>   only kick in for boards with no NIC or if CONFIG_SLIRP is not set)
>  * Diagnose the case where the user asked for NICs which the board
>   didn't instantiate (for example where the user asked for two NICs
>   but the board only supports one)
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> The motivation for this patch is that I thought it made more sense
> to complain about unused NIC specifications in generic code than
> force every board to do it; see discussion of the vexpress patch
> http://patchwork.ozlabs.org/patch/85727/
>
>  net.c |   34 +++++++++++++++++++++++++++++++++-
>  1 files changed, 33 insertions(+), 1 deletions(-)
>
> diff --git a/net.c b/net.c
> index ddcca97..9d3aaf5 100644
> --- a/net.c
> +++ b/net.c
> @@ -1305,12 +1305,30 @@ void net_check_clients(void)
>  {
>     VLANState *vlan;
>     VLANClientState *vc;
> -    int has_nic = 0, has_host_dev = 0;
> +    int has_nic, has_host_dev;
> +    int seen_nics = 0;
> +
> +    /* Don't warn about the default network setup that you get if
> +     * no command line -net options are specified. There are two
> +     * cases that we would otherwise complain about:
> +     * (1) board doesn't support a NIC but the implicit "-net nic"
> +     * requested one; we'd otherwise complain about more NICs being
> +     * specified than we support, and also that the vlan set up by
> +     * the implicit "-net user" didn't have any NICs connected to it
> +     * (2) CONFIG_SLIRP not set: we'd otherwise complain about the
> +     * implicit "-net nic" setting up a nic that wasn't connected to
> +     * anything.
> +     */
> +    if (default_net) {
> +        return;
> +    }
>
>     QTAILQ_FOREACH(vlan, &vlans, next) {
> +        has_nic = has_host_dev = 0;
>         QTAILQ_FOREACH(vc, &vlan->clients, next) {
>             switch (vc->info->type) {
>             case NET_CLIENT_TYPE_NIC:
> +                seen_nics++;
>                 has_nic = 1;
>                 break;
>             case NET_CLIENT_TYPE_SLIRP:
> @@ -1330,12 +1348,26 @@ void net_check_clients(void)
>                     vlan->id);
>     }
>     QTAILQ_FOREACH(vc, &non_vlan_clients, next) {
> +        if (vc->info->type == NET_CLIENT_TYPE_NIC) {
> +            seen_nics++;
> +        }
>         if (!vc->peer) {
>             fprintf(stderr, "Warning: %s %s has no peer\n",
>                     vc->info->type == NET_CLIENT_TYPE_NIC ? "nic" : "netdev",
>                     vc->name);
>         }
>     }
> +    if (seen_nics != nb_nics) {
> +        /* Number of NICs requested by user on command line doesn't match
> +         * the number the model actually registered with us.
> +         * This will generally only happen for models of embedded boards
> +         * with no PCI bus or similar. PCI based machines can instantiate
> +         * all requested NICs as PCI devices but usually embedded boards
> +         * only have a single NIC.
> +         */
> +        fprintf(stderr, "Warning: more nics requested than this machine "
> +                "supports; some have been ignored\n");
> +    }
>  }
>
>  static int net_init_client(QemuOpts *opts, void *dummy)
> --
> 1.7.1

Re: [Qemu-devel] [PATCH] net: Improve the warnings for dubious command line option combinations

[Aurelien Jarno]

On Tue, Mar 22, 2011 at 06:39:40PM +0000, Peter Maydell wrote:
> Improve the warnings we give if the user specified a combination of -net
> options which don't make much sense:
>  * Fix a bug where we would only complain about the first VLAN having
>    no NIC or no host network connection; we now diagnose this situation
>    for all VLANs
>  * Don't warn about anything if the config is the implicit default
>    "-net user -net nic" rather than one specified by the user (this will
>    only kick in for boards with no NIC or if CONFIG_SLIRP is not set)
>  * Diagnose the case where the user asked for NICs which the board
>    didn't instantiate (for example where the user asked for two NICs
>    but the board only supports one)
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> The motivation for this patch is that I thought it made more sense
> to complain about unused NIC specifications in generic code than
> force every board to do it; see discussion of the vexpress patch
> http://patchwork.ozlabs.org/patch/85727/
> 
>  net.c |   34 +++++++++++++++++++++++++++++++++-
>  1 files changed, 33 insertions(+), 1 deletions(-)

Thanks, applied. Note that I have removed the first point from the
patch, it has been already fixed in commit 
ac60cc18711a9786af9844d7e3d002276fbd85f3

> diff --git a/net.c b/net.c
> index ddcca97..9d3aaf5 100644
> --- a/net.c
> +++ b/net.c
> @@ -1305,12 +1305,30 @@ void net_check_clients(void)
>  {
>      VLANState *vlan;
>      VLANClientState *vc;
> -    int has_nic = 0, has_host_dev = 0;
> +    int has_nic, has_host_dev;
> +    int seen_nics = 0;
> +
> +    /* Don't warn about the default network setup that you get if
> +     * no command line -net options are specified. There are two
> +     * cases that we would otherwise complain about:
> +     * (1) board doesn't support a NIC but the implicit "-net nic"
> +     * requested one; we'd otherwise complain about more NICs being
> +     * specified than we support, and also that the vlan set up by
> +     * the implicit "-net user" didn't have any NICs connected to it
> +     * (2) CONFIG_SLIRP not set: we'd otherwise complain about the
> +     * implicit "-net nic" setting up a nic that wasn't connected to
> +     * anything.
> +     */
> +    if (default_net) {
> +        return;
> +    }
>  
>      QTAILQ_FOREACH(vlan, &vlans, next) {
> +        has_nic = has_host_dev = 0;
>          QTAILQ_FOREACH(vc, &vlan->clients, next) {
>              switch (vc->info->type) {
>              case NET_CLIENT_TYPE_NIC:
> +                seen_nics++;
>                  has_nic = 1;
>                  break;
>              case NET_CLIENT_TYPE_SLIRP:
> @@ -1330,12 +1348,26 @@ void net_check_clients(void)
>                      vlan->id);
>      }
>      QTAILQ_FOREACH(vc, &non_vlan_clients, next) {
> +        if (vc->info->type == NET_CLIENT_TYPE_NIC) {
> +            seen_nics++;
> +        }
>          if (!vc->peer) {
>              fprintf(stderr, "Warning: %s %s has no peer\n",
>                      vc->info->type == NET_CLIENT_TYPE_NIC ? "nic" : "netdev",
>                      vc->name);
>          }
>      }
> +    if (seen_nics != nb_nics) {
> +        /* Number of NICs requested by user on command line doesn't match
> +         * the number the model actually registered with us.
> +         * This will generally only happen for models of embedded boards
> +         * with no PCI bus or similar. PCI based machines can instantiate
> +         * all requested NICs as PCI devices but usually embedded boards
> +         * only have a single NIC.
> +         */
> +        fprintf(stderr, "Warning: more nics requested than this machine "
> +                "supports; some have been ignored\n");
> +    }
>  }
>  
>  static int net_init_client(QemuOpts *opts, void *dummy)
> -- 
> 1.7.1
> 
> 
> 

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

Re: [Qemu-devel] [PATCH] net: Improve the warnings for dubious command line option combinations

[Peter Maydell]

On 1 April 2011 21:55, Aurelien Jarno <aurelien@aurel32.net> wrote:
> On Tue, Mar 22, 2011 at 06:39:40PM +0000, Peter Maydell wrote:
>>  * Diagnose the case where the user asked for NICs which the board
>>    didn't instantiate (for example where the user asked for two NICs
>>    but the board only supports one)

> Thanks, applied.

Thanks. I believe that removes the only issue with the vexpress
board support patch so you could apply that now? :-)

-- PMM