* [Qemu-devel] [PATCH 0/2] Small fixes for qemu-sockets ipv4/ipv6 logic.
@ 2014-02-28 10:16 Paolo Bonzini
2014-02-28 10:16 ` [Qemu-devel] [PATCH 1/2] socket: treat ipv4=on,ipv6=on uniformly Paolo Bonzini
2014-02-28 10:16 ` [Qemu-devel] [PATCH 2/2] socket: handle ipv4/ipv6 in socket_dgram Paolo Bonzini
0 siblings, 2 replies; 7+ messages in thread
From: Paolo Bonzini @ 2014-02-28 10:16 UTC (permalink / raw)
To: qemu-devel; +Cc: kraxel, stefanha
While reviewing the L2TPv3 submission, I wanted to tell Anton that
IPv4/IPv6 is already done in qemu-sockets.c. Turns out it is not
exactly the case; these patches fixes the two issues I spotted.
Paolo Bonzini (2):
socket: treat ipv4=on,ipv6=on uniformly
socket: handle ipv4/ipv6 in socket_dgram
util/qemu-sockets.c | 39 ++++++++++++++++++++++++++-------------
1 file changed, 26 insertions(+), 13 deletions(-)
--
1.8.5.3
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH 1/2] socket: treat ipv4=on,ipv6=on uniformly
2014-02-28 10:16 [Qemu-devel] [PATCH 0/2] Small fixes for qemu-sockets ipv4/ipv6 logic Paolo Bonzini
@ 2014-02-28 10:16 ` Paolo Bonzini
2014-02-28 11:56 ` [Qemu-devel] [PATCH 1/2] socket: treat ipv4=on, ipv6=on uniformly Markus Armbruster
2014-02-28 12:23 ` Gerd Hoffmann
2014-02-28 10:16 ` [Qemu-devel] [PATCH 2/2] socket: handle ipv4/ipv6 in socket_dgram Paolo Bonzini
1 sibling, 2 replies; 7+ messages in thread
From: Paolo Bonzini @ 2014-02-28 10:16 UTC (permalink / raw)
To: qemu-devel; +Cc: kraxel, stefanha
In some cases, "ipv4=on,ipv6=on" means "try both kinds of address";
in others, it means "try IPv6 only" just by virtue of how the code
is structured.
Fix this to make things more consistent, and adjust coding style too.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
util/qemu-sockets.c | 33 ++++++++++++++++++++-------------
1 file changed, 20 insertions(+), 13 deletions(-)
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 8818d7c..7a9065b 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -127,10 +127,13 @@ int inet_listen_opts(QemuOpts *opts, int port_offset, Error **errp)
addr = qemu_opt_get(opts, "host");
to = qemu_opt_get_number(opts, "to", 0);
- if (qemu_opt_get_bool(opts, "ipv4", 0))
- ai.ai_family = PF_INET;
- if (qemu_opt_get_bool(opts, "ipv6", 0))
- ai.ai_family = PF_INET6;
+ if (qemu_opt_get_bool(opts, "ipv4", 0) != qemu_opt_get_bool(opts, "ipv6", 0)) {
+ if (qemu_opt_get_bool(opts, "ipv4", 0)) {
+ ai.ai_family = PF_INET;
+ } else {
+ ai.ai_family = PF_INET6;
+ }
+ }
/* lookup */
if (port_offset)
@@ -319,11 +322,12 @@ static struct addrinfo *inet_parse_connect_opts(QemuOpts *opts, Error **errp)
return NULL;
}
- if (qemu_opt_get_bool(opts, "ipv4", 0)) {
- ai.ai_family = PF_INET;
- }
- if (qemu_opt_get_bool(opts, "ipv6", 0)) {
- ai.ai_family = PF_INET6;
+ if (qemu_opt_get_bool(opts, "ipv4", 0) != qemu_opt_get_bool(opts, "ipv6", 0)) {
+ if (qemu_opt_get_bool(opts, "ipv4", 0)) {
+ ai.ai_family = PF_INET;
+ } else {
+ ai.ai_family = PF_INET6;
+ }
}
/* lookup */
@@ -418,10 +422,13 @@ int inet_dgram_opts(QemuOpts *opts, Error **errp)
return -1;
}
- if (qemu_opt_get_bool(opts, "ipv4", 0))
- ai.ai_family = PF_INET;
- if (qemu_opt_get_bool(opts, "ipv6", 0))
- ai.ai_family = PF_INET6;
+ if (qemu_opt_get_bool(opts, "ipv4", 0) != qemu_opt_get_bool(opts, "ipv6", 0)) {
+ if (qemu_opt_get_bool(opts, "ipv4", 0)) {
+ ai.ai_family = PF_INET;
+ } else {
+ ai.ai_family = PF_INET6;
+ }
+ }
if (0 != (rc = getaddrinfo(addr, port, &ai, &peer))) {
error_setg(errp, "address resolution failed for %s:%s: %s", addr, port,
--
1.8.5.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH 2/2] socket: handle ipv4/ipv6 in socket_dgram
2014-02-28 10:16 [Qemu-devel] [PATCH 0/2] Small fixes for qemu-sockets ipv4/ipv6 logic Paolo Bonzini
2014-02-28 10:16 ` [Qemu-devel] [PATCH 1/2] socket: treat ipv4=on,ipv6=on uniformly Paolo Bonzini
@ 2014-02-28 10:16 ` Paolo Bonzini
2014-02-28 13:05 ` Gerd Hoffmann
1 sibling, 1 reply; 7+ messages in thread
From: Paolo Bonzini @ 2014-02-28 10:16 UTC (permalink / raw)
To: qemu-devel; +Cc: kraxel, stefanha
We were forgetting to set the ipv4 and ipv6 QemuOpts properties, so
socket_dgram was always trying _both_ ipv4 and ipv6. Forward the
properties correctly.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
util/qemu-sockets.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 7a9065b..7391aee 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -958,15 +958,22 @@ int socket_dgram(SocketAddress *remote, SocketAddress *local, Error **errp)
opts = qemu_opts_create(&socket_optslist, NULL, 0, &error_abort);
switch (remote->kind) {
- case SOCKET_ADDRESS_KIND_INET:
+ case SOCKET_ADDRESS_KIND_INET: {
+ bool ipv4 = remote->inet->ipv4 || !remote->inet->has_ipv4;
+ bool ipv6 = remote->inet->ipv6 || !remote->inet->has_ipv6;
qemu_opt_set(opts, "host", remote->inet->host);
qemu_opt_set(opts, "port", remote->inet->port);
if (local) {
qemu_opt_set(opts, "localaddr", local->inet->host);
qemu_opt_set(opts, "localport", local->inet->port);
}
+ if (!ipv4 || !ipv6) {
+ qemu_opt_set_bool(opts, "ipv4", ipv4);
+ qemu_opt_set_bool(opts, "ipv6", ipv6);
+ }
fd = inet_dgram_opts(opts, errp);
break;
+ }
default:
error_setg(errp, "socket type unsupported for datagram");
--
1.8.5.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] socket: treat ipv4=on, ipv6=on uniformly
2014-02-28 10:16 ` [Qemu-devel] [PATCH 1/2] socket: treat ipv4=on,ipv6=on uniformly Paolo Bonzini
@ 2014-02-28 11:56 ` Markus Armbruster
2014-02-28 12:23 ` Gerd Hoffmann
1 sibling, 0 replies; 7+ messages in thread
From: Markus Armbruster @ 2014-02-28 11:56 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, stefanha, kraxel
Paolo Bonzini <pbonzini@redhat.com> writes:
> In some cases, "ipv4=on,ipv6=on" means "try both kinds of address";
> in others, it means "try IPv6 only" just by virtue of how the code
> is structured.
>
> Fix this to make things more consistent, and adjust coding style too.
Begs the question which one you're picking as the consistent
interpretation, "both" or "v6 only".
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] socket: treat ipv4=on, ipv6=on uniformly
2014-02-28 10:16 ` [Qemu-devel] [PATCH 1/2] socket: treat ipv4=on,ipv6=on uniformly Paolo Bonzini
2014-02-28 11:56 ` [Qemu-devel] [PATCH 1/2] socket: treat ipv4=on, ipv6=on uniformly Markus Armbruster
@ 2014-02-28 12:23 ` Gerd Hoffmann
2014-02-28 12:44 ` Paolo Bonzini
1 sibling, 1 reply; 7+ messages in thread
From: Gerd Hoffmann @ 2014-02-28 12:23 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, stefanha
On Fr, 2014-02-28 at 11:16 +0100, Paolo Bonzini wrote:
> In some cases, "ipv4=on,ipv6=on" means "try both kinds of address";
> in others, it means "try IPv6 only" just by virtue of how the code
> is structured.
> @@ -127,10 +127,13 @@ int inet_listen_opts(QemuOpts *opts, int port_offset, Error **errp)
> - if (qemu_opt_get_bool(opts, "ipv4", 0))
> - ai.ai_family = PF_INET;
> - if (qemu_opt_get_bool(opts, "ipv6", 0))
> - ai.ai_family = PF_INET6;
> + if (qemu_opt_get_bool(opts, "ipv4", 0) != qemu_opt_get_bool(opts, "ipv6", 0)) {
> + if (qemu_opt_get_bool(opts, "ipv4", 0)) {
> + ai.ai_family = PF_INET;
> + } else {
> + ai.ai_family = PF_INET6;
> + }
> + }
This is wrong for the listening side.
ipv6 sockets can listen on both ipv4 and ipv6. qemu configures ipv6
sockets to do that, unconditionally.
So ipv4=yes,ipv6=no works correctly.
ipv4=yes,ipv6=yes works too.
ipv4=no,ipv6=yes doesn't work, but to fix that you have to set the
IPV6_V6ONLY socket option according to the ipv4 option.
Canevat: Listening on both ipv4+ipv6 with a single file handle works for
the wildcard address only. Specifying -- say --
host=localhost,ipv4=yes,ipv6=yes, then expect qemu to listen on both
127.0.0.1 and ::1 doesn't work.
This can only be fixed by allowing multiple listening filehandles.
Which is a non-trivial change as this also affects the public api (which
will have to report a list of listening addresses instead of a single
address).
> @@ -319,11 +322,12 @@ static struct addrinfo *inet_parse_connect_opts(QemuOpts *opts, Error **errp)
> - if (qemu_opt_get_bool(opts, "ipv4", 0)) {
> - ai.ai_family = PF_INET;
> - }
> - if (qemu_opt_get_bool(opts, "ipv6", 0)) {
> - ai.ai_family = PF_INET6;
> + if (qemu_opt_get_bool(opts, "ipv4", 0) != qemu_opt_get_bool(opts, "ipv6", 0)) {
> + if (qemu_opt_get_bool(opts, "ipv4", 0)) {
> + ai.ai_family = PF_INET;
> + } else {
> + ai.ai_family = PF_INET6;
> + }
For connect this change is fine I think.
> @@ -418,10 +422,13 @@ int inet_dgram_opts(QemuOpts *opts, Error **errp)
> - if (qemu_opt_get_bool(opts, "ipv4", 0))
> - ai.ai_family = PF_INET;
> - if (qemu_opt_get_bool(opts, "ipv6", 0))
> - ai.ai_family = PF_INET6;
> + if (qemu_opt_get_bool(opts, "ipv4", 0) != qemu_opt_get_bool(opts, "ipv6", 0)) {
> + if (qemu_opt_get_bool(opts, "ipv4", 0)) {
> + ai.ai_family = PF_INET;
> + } else {
> + ai.ai_family = PF_INET6;
> + }
> + }
Not fully sure, but I think this is fine too.
cheers,
Gerd
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] socket: treat ipv4=on, ipv6=on uniformly
2014-02-28 12:23 ` Gerd Hoffmann
@ 2014-02-28 12:44 ` Paolo Bonzini
0 siblings, 0 replies; 7+ messages in thread
From: Paolo Bonzini @ 2014-02-28 12:44 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: qemu-devel, stefanha
Il 28/02/2014 13:23, Gerd Hoffmann ha scritto:
> On Fr, 2014-02-28 at 11:16 +0100, Paolo Bonzini wrote:
>> In some cases, "ipv4=on,ipv6=on" means "try both kinds of address";
>> in others, it means "try IPv6 only" just by virtue of how the code
>> is structured.
>
>> @@ -127,10 +127,13 @@ int inet_listen_opts(QemuOpts *opts, int port_offset, Error **errp)
>
>> - if (qemu_opt_get_bool(opts, "ipv4", 0))
>> - ai.ai_family = PF_INET;
>> - if (qemu_opt_get_bool(opts, "ipv6", 0))
>> - ai.ai_family = PF_INET6;
>> + if (qemu_opt_get_bool(opts, "ipv4", 0) != qemu_opt_get_bool(opts, "ipv6", 0)) {
>> + if (qemu_opt_get_bool(opts, "ipv4", 0)) {
>> + ai.ai_family = PF_INET;
>> + } else {
>> + ai.ai_family = PF_INET6;
>> + }
>> + }
>
> This is wrong for the listening side.
>
> ipv6 sockets can listen on both ipv4 and ipv6. qemu configures ipv6
> sockets to do that, unconditionally.
>
> So ipv4=yes,ipv6=no works correctly.
> ipv4=yes,ipv6=yes works too.
> ipv4=no,ipv6=yes doesn't work, but to fix that you have to set the
> IPV6_V6ONLY socket option according to the ipv4 option.
>
> Canevat: Listening on both ipv4+ipv6 with a single file handle works for
> the wildcard address only. Specifying -- say --
> host=localhost,ipv4=yes,ipv6=yes, then expect qemu to listen on both
> 127.0.0.1 and ::1 doesn't work.
>
> This can only be fixed by allowing multiple listening filehandles.
> Which is a non-trivial change as this also affects the public api (which
> will have to report a list of listening addresses instead of a single
> address).
Thanks for teaching me!
Do you think patch 2 is okay alone?
Paolo
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] socket: handle ipv4/ipv6 in socket_dgram
2014-02-28 10:16 ` [Qemu-devel] [PATCH 2/2] socket: handle ipv4/ipv6 in socket_dgram Paolo Bonzini
@ 2014-02-28 13:05 ` Gerd Hoffmann
0 siblings, 0 replies; 7+ messages in thread
From: Gerd Hoffmann @ 2014-02-28 13:05 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, stefanha
On Fr, 2014-02-28 at 11:16 +0100, Paolo Bonzini wrote:
> - case SOCKET_ADDRESS_KIND_INET:
> + case SOCKET_ADDRESS_KIND_INET: {
> + bool ipv4 = remote->inet->ipv4 || !remote->inet->has_ipv4;
> + bool ipv6 = remote->inet->ipv6 || !remote->inet->has_ipv6;
> qemu_opt_set(opts, "host", remote->inet->host);
> qemu_opt_set(opts, "port", remote->inet->port);
> if (local) {
> qemu_opt_set(opts, "localaddr", local->inet->host);
> qemu_opt_set(opts, "localport", local->inet->port);
> }
> + if (!ipv4 || !ipv6) {
> + qemu_opt_set_bool(opts, "ipv4", ipv4);
> + qemu_opt_set_bool(opts, "ipv6", ipv6);
> + }
I'd go for a simple
if (remote->inet->has_ipv4) {
qemu_opt_set_bool(opts, "ipv4", remote->inet->ipv4)
}
cheers,
Gerd
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-02-28 13:06 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-28 10:16 [Qemu-devel] [PATCH 0/2] Small fixes for qemu-sockets ipv4/ipv6 logic Paolo Bonzini
2014-02-28 10:16 ` [Qemu-devel] [PATCH 1/2] socket: treat ipv4=on,ipv6=on uniformly Paolo Bonzini
2014-02-28 11:56 ` [Qemu-devel] [PATCH 1/2] socket: treat ipv4=on, ipv6=on uniformly Markus Armbruster
2014-02-28 12:23 ` Gerd Hoffmann
2014-02-28 12:44 ` Paolo Bonzini
2014-02-28 10:16 ` [Qemu-devel] [PATCH 2/2] socket: handle ipv4/ipv6 in socket_dgram Paolo Bonzini
2014-02-28 13:05 ` Gerd Hoffmann
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.