| Message ID | 1300819180-8121-1-git-send-email-peter.maydell@linaro.org |
|---|---|
| State | Accepted |
| Commit | f68b9d672b90dedc79aeb9b44607f484dbe46a6b |
| Headers | show |
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
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 > > >
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
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)
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(-)