| Message ID | 1393582579-10366-2-git-send-email-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
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".
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
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
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,
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(-)