On Wed, Nov 18, 2015 at 08:44:28AM -0700, Eric Blake wrote: > On 11/18/2015 03:08 AM, Daniel P. Berrange wrote: > > On Tue, Nov 17, 2015 at 03:22:04PM -0700, Eric Blake wrote: > >> On 11/17/2015 10:00 AM, Daniel P. Berrange wrote: > >>> The socket_listen method accepts a QAPI SocketAddress object > >>> which it then turns into QemuOpts before calling the > >>> inet_listen_opts/unix_listen_opts helper methods. By > >>> converting the latter to use QAPI SocketAddress directly, > >>> the QemuOpts conversion step can be eliminated > >>> > >>> This also fixes the problem where ipv4=off && ipv6=off > >>> would be treated the same as ipv4=on && ipv6=on > >>> > > >>> + * ipv4 ipv6 family > >>> + * - - PF_UNSPEC > > This says I have no preference, so pick the one that works. > > >>> + * - f PF_INET > >>> + * - t PF_INET6 > >>> + * f - PF_INET6 > >>> + * f f <error> > >>> + * f t PF_INET6 > >>> + * t - PF_INET > >>> + * t f PF_INET > >> > >> These I understand, > > Generally to mean "I specifically requested this" or "I specifically > don't want that", where there is no collision. > > >> > >>> + * t t PF_INET6 > >> > >> but why is this one PF_INET6 instead of PF_UNSPEC? > > > > If you use PF_UNSPEC, then the addresses we listen on will be automatically > > deteremined by results of the DNS lookup. ie if DNS only returns an IPv4 > > address, it won't listen on IPv6. When the user has said ipv6=on, then > > they expect to get an error if it was not possible to listen on IPv6. So > > we must use PF_INET6 here to ensure that error, otherwise ipv6=on & ipv4=on > > would be no different from ipv6=- & ipv4=-. > > But if I'm specifically requesting that both families be used, either > that should be an error (we can't honor two families at once) or it > should be allowed (use the family that makes sense), not a synonym for > ipv6-only.
Yes, you are right that this code means ipv6=t & ipv4=t is essentially equivalent to ipv6=t & ipv4=-, but that is a limitation of getaddrinfo(). To address this semantic flaw, when ipv6=t, then we need better handling of the IPV6_V6ONLY flag to take account of ipv4= setting, and then actually verify whether ipv4 really was enabled when we asked for it. This is a pre-existing bug that my patch is not making worse. I will have a think about fixing it separately. And yes, we so badly need a unit test to validate all this logic, which I also want to look into... Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
