From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60422) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zz52q-0004Pw-8D for qemu-devel@nongnu.org; Wed, 18 Nov 2015 10:53:17 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zz52k-0002yc-QL for qemu-devel@nongnu.org; Wed, 18 Nov 2015 10:53:16 -0500 Received: from mx1.redhat.com ([209.132.183.28]:39143) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zz52k-0002yO-Ib for qemu-devel@nongnu.org; Wed, 18 Nov 2015 10:53:10 -0500 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (Postfix) with ESMTPS id 1D31B8D3B5 for ; Wed, 18 Nov 2015 15:53:10 +0000 (UTC) Date: Wed, 18 Nov 2015 15:53:06 +0000 From: "Daniel P. Berrange" Message-ID: <20151118155306.GL27591@redhat.com> References: <1447779624-21625-1-git-send-email-berrange@redhat.com> <1447779624-21625-3-git-send-email-berrange@redhat.com> <564BA88C.1010403@redhat.com> <20151118100821.GB27591@redhat.com> <564C9CDC.3040604@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <564C9CDC.3040604@redhat.com> Subject: Re: [Qemu-devel] [PATCH v2 2/5] sockets: remove use of QemuOpts from socket_listen Reply-To: "Daniel P. Berrange" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: Paolo Bonzini , qemu-devel@nongnu.org, Gerd Hoffmann 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 > >>> + * 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 :|