* [Qemu-devel] [PATCH] net: Silence 'has no peer' messages in testing mode
@ 2018-05-03 5:50 Thomas Huth
2018-05-03 11:47 ` Markus Armbruster
0 siblings, 1 reply; 6+ messages in thread
From: Thomas Huth @ 2018-05-03 5:50 UTC (permalink / raw)
To: qemu-devel, Jason Wang; +Cc: Markus Armbruster
When running qtests with -nodefaults, we are not interested in
these 'XYZ has no peer' messages.
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
net/net.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/net/net.c b/net/net.c
index 29f8398..58bf85e 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1427,12 +1427,13 @@ void net_check_clients(void)
net_hub_check_clients();
- QTAILQ_FOREACH(nc, &net_clients, next) {
- if (!nc->peer) {
- warn_report("%s %s has no peer",
- nc->info->type == NET_CLIENT_DRIVER_NIC
- ? "nic" : "netdev",
- nc->name);
+ if (!qtest_enabled() || nd_table[0].used) {
+ QTAILQ_FOREACH(nc, &net_clients, next) {
+ if (!nc->peer) {
+ warn_report("%s %s has no peer",
+ nc->info->type == NET_CLIENT_DRIVER_NIC
+ ? "nic" : "netdev", nc->name);
+ }
}
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] net: Silence 'has no peer' messages in testing mode
2018-05-03 5:50 [Qemu-devel] [PATCH] net: Silence 'has no peer' messages in testing mode Thomas Huth
@ 2018-05-03 11:47 ` Markus Armbruster
2018-05-04 14:55 ` Thomas Huth
0 siblings, 1 reply; 6+ messages in thread
From: Markus Armbruster @ 2018-05-03 11:47 UTC (permalink / raw)
To: Thomas Huth; +Cc: qemu-devel, Jason Wang
Thomas Huth <thuth@redhat.com> writes:
> When running qtests with -nodefaults, we are not interested in
> these 'XYZ has no peer' messages.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> net/net.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/net/net.c b/net/net.c
> index 29f8398..58bf85e 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -1427,12 +1427,13 @@ void net_check_clients(void)
>
> net_hub_check_clients();
>
> - QTAILQ_FOREACH(nc, &net_clients, next) {
> - if (!nc->peer) {
> - warn_report("%s %s has no peer",
> - nc->info->type == NET_CLIENT_DRIVER_NIC
> - ? "nic" : "netdev",
> - nc->name);
> + if (!qtest_enabled() || nd_table[0].used) {
I understand the !qtest_enabled part, but not the nd_table[0].used
part. Can you explain?
> + QTAILQ_FOREACH(nc, &net_clients, next) {
> + if (!nc->peer) {
> + warn_report("%s %s has no peer",
> + nc->info->type == NET_CLIENT_DRIVER_NIC
> + ? "nic" : "netdev", nc->name);
> + }
> }
> }
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] net: Silence 'has no peer' messages in testing mode
2018-05-03 11:47 ` Markus Armbruster
@ 2018-05-04 14:55 ` Thomas Huth
2018-05-07 7:14 ` Markus Armbruster
0 siblings, 1 reply; 6+ messages in thread
From: Thomas Huth @ 2018-05-04 14:55 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel, Jason Wang
On 03.05.2018 13:47, Markus Armbruster wrote:
> Thomas Huth <thuth@redhat.com> writes:
>
>> When running qtests with -nodefaults, we are not interested in
>> these 'XYZ has no peer' messages.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>> net/net.c | 13 +++++++------
>> 1 file changed, 7 insertions(+), 6 deletions(-)
>>
>> diff --git a/net/net.c b/net/net.c
>> index 29f8398..58bf85e 100644
>> --- a/net/net.c
>> +++ b/net/net.c
>> @@ -1427,12 +1427,13 @@ void net_check_clients(void)
>>
>> net_hub_check_clients();
>>
>> - QTAILQ_FOREACH(nc, &net_clients, next) {
>> - if (!nc->peer) {
>> - warn_report("%s %s has no peer",
>> - nc->info->type == NET_CLIENT_DRIVER_NIC
>> - ? "nic" : "netdev",
>> - nc->name);
>> + if (!qtest_enabled() || nd_table[0].used) {
>
> I understand the !qtest_enabled part, but not the nd_table[0].used
> part. Can you explain?
Sure: I want to silence the message in qtest mode with -nodefaults.
qtest mode enabled means qtest_enabled() returns true.
-nodefaults enabled means nd_table[0].used is set to false.
So silence the message if qtest_enabled() && !nd_table[0].used.
Negates to: Print the message if !qtest_enabled || nd_table[0].used.
>> + QTAILQ_FOREACH(nc, &net_clients, next) {
>> + if (!nc->peer) {
>> + warn_report("%s %s has no peer",
>> + nc->info->type == NET_CLIENT_DRIVER_NIC
>> + ? "nic" : "netdev", nc->name);
>> + }
>> }
>> }
Thomas
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] net: Silence 'has no peer' messages in testing mode
2018-05-04 14:55 ` Thomas Huth
@ 2018-05-07 7:14 ` Markus Armbruster
2018-05-08 13:19 ` Thomas Huth
0 siblings, 1 reply; 6+ messages in thread
From: Markus Armbruster @ 2018-05-07 7:14 UTC (permalink / raw)
To: Thomas Huth; +Cc: Jason Wang, qemu-devel
Thomas Huth <thuth@redhat.com> writes:
> On 03.05.2018 13:47, Markus Armbruster wrote:
>> Thomas Huth <thuth@redhat.com> writes:
>>
>>> When running qtests with -nodefaults, we are not interested in
>>> these 'XYZ has no peer' messages.
>>>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>> ---
>>> net/net.c | 13 +++++++------
>>> 1 file changed, 7 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/net/net.c b/net/net.c
>>> index 29f8398..58bf85e 100644
>>> --- a/net/net.c
>>> +++ b/net/net.c
>>> @@ -1427,12 +1427,13 @@ void net_check_clients(void)
>>>
>>> net_hub_check_clients();
>>>
>>> - QTAILQ_FOREACH(nc, &net_clients, next) {
>>> - if (!nc->peer) {
>>> - warn_report("%s %s has no peer",
>>> - nc->info->type == NET_CLIENT_DRIVER_NIC
>>> - ? "nic" : "netdev",
>>> - nc->name);
>>> + if (!qtest_enabled() || nd_table[0].used) {
>>
>> I understand the !qtest_enabled part, but not the nd_table[0].used
>> part. Can you explain?
>
> Sure: I want to silence the message in qtest mode with -nodefaults.
> qtest mode enabled means qtest_enabled() returns true.
> -nodefaults enabled means nd_table[0].used is set to false.
.used is initialized to false, and becomes true on successful
net_init_nic() / net_param_nic().
The connection to -nodefaults is this:
if (default_net) {
QemuOptsList *net = qemu_find_opts("net");
qemu_opts_set(net, NULL, "type", "nic", &error_abort);
#ifdef CONFIG_SLIRP
qemu_opts_set(net, NULL, "type", "user", &error_abort);
#endif
}
When default_net is true, we effectively add -net nic". It is true
unless the user specifies any of -nodefaults, -netdev, -nic, -net. So,
with the default nic, nd_table[0].used is true. It's also true with
non-default nics specified with convenience options.
> So silence the message if qtest_enabled() && !nd_table[0].used.
Thus, silence the message if qtesting and the user specified -nodefaults
and didn't add nics with convenience options.
Two (possibly confused) questions:
1. The user can add nics without convenience options:
$ upstream-qemu -display none -nodefaults -device e1000
upstream-qemu: warning: nic e1000.0 has no peer
Shouldn't we silence the warning then, too?
2. We already have code to silence the warning:
/* Don't warn about the default network setup that you get if
* no command line -net or -netdev 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
* (2) CONFIG_SLIRP not set, in which case the implicit "-net nic"
* sets up a nic that isn't connected to anything.
*/
if (!default_net) {
net_check_clients();
}
Is it a good idea to split the logic between net_check_clients() and
its caller?
> Negates to: Print the message if !qtest_enabled || nd_table[0].used.
>
>>> + QTAILQ_FOREACH(nc, &net_clients, next) {
>>> + if (!nc->peer) {
>>> + warn_report("%s %s has no peer",
>>> + nc->info->type == NET_CLIENT_DRIVER_NIC
>>> + ? "nic" : "netdev", nc->name);
>>> + }
>>> }
>>> }
>
> Thomas
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] net: Silence 'has no peer' messages in testing mode
2018-05-07 7:14 ` Markus Armbruster
@ 2018-05-08 13:19 ` Thomas Huth
2018-05-09 6:18 ` Markus Armbruster
0 siblings, 1 reply; 6+ messages in thread
From: Thomas Huth @ 2018-05-08 13:19 UTC (permalink / raw)
To: Markus Armbruster; +Cc: Jason Wang, qemu-devel
On 07.05.2018 09:14, Markus Armbruster wrote:
[...]
> Two (possibly confused) questions:
>
> 1. The user can add nics without convenience options:
>
> $ upstream-qemu -display none -nodefaults -device e1000
> upstream-qemu: warning: nic e1000.0 has no peer
>
> Shouldn't we silence the warning then, too?
No, since that is certainly a mis-configuration in that case. Why would
a user want to add a NIC without host backend?
> 2. We already have code to silence the warning:
>
> /* Don't warn about the default network setup that you get if
> * no command line -net or -netdev 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
> * (2) CONFIG_SLIRP not set, in which case the implicit "-net nic"
> * sets up a nic that isn't connected to anything.
> */
> if (!default_net) {
> net_check_clients();
> }
>
> Is it a good idea to split the logic between net_check_clients() and
> its caller?
Hmm, it's likely nicer to keep everything in one place. Since
"default_net" is only available in vl.c, I think the checks should go
there... so I'll rework my patch accordingly.
Thomas
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] net: Silence 'has no peer' messages in testing mode
2018-05-08 13:19 ` Thomas Huth
@ 2018-05-09 6:18 ` Markus Armbruster
0 siblings, 0 replies; 6+ messages in thread
From: Markus Armbruster @ 2018-05-09 6:18 UTC (permalink / raw)
To: Thomas Huth; +Cc: Jason Wang, qemu-devel
Thomas Huth <thuth@redhat.com> writes:
> On 07.05.2018 09:14, Markus Armbruster wrote:
> [...]
>> Two (possibly confused) questions:
>>
>> 1. The user can add nics without convenience options:
>>
>> $ upstream-qemu -display none -nodefaults -device e1000
>> upstream-qemu: warning: nic e1000.0 has no peer
>>
>> Shouldn't we silence the warning then, too?
>
> No, since that is certainly a mis-configuration in that case. Why would
> a user want to add a NIC without host backend?
Related:
$ upstream-qemu -display none -nodefaults -net nic
upstream-qemu: warning: vlan 0 is not connected to host network
The root of my confusion: I'm having difficulties making the connection
from the guard predicate (!qtest_enabled() || nd_table[0].used) to
"stupid default setup that isn't the test's fault".
In what scenarios exactly do we get unwanted "has no peer" messages?
Mandatory onboard NIC, perhaps?
Arguably, onboard NICs should default to a null backend that behaves
like "no carrier". That's exactly what physical hardware does. Could
even be useful outside tests once we support hot-plugging network
backends (we might already, I have no idea), just like physical hardware
supports plugging in network cables.
[...]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-05-09 6:18 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-03 5:50 [Qemu-devel] [PATCH] net: Silence 'has no peer' messages in testing mode Thomas Huth
2018-05-03 11:47 ` Markus Armbruster
2018-05-04 14:55 ` Thomas Huth
2018-05-07 7:14 ` Markus Armbruster
2018-05-08 13:19 ` Thomas Huth
2018-05-09 6:18 ` Markus Armbruster
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.