From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43)
	id 1Kx1TM-0008Kc-4Z
	for qemu-devel@nongnu.org; Mon, 03 Nov 2008 10:36:08 -0500
Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43)
	id 1Kx1TK-0008Jv-Br
	for qemu-devel@nongnu.org; Mon, 03 Nov 2008 10:36:07 -0500
Received: from [199.232.76.173] (port=34485 helo=monty-python.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1Kx1TK-0008Jq-13
	for qemu-devel@nongnu.org; Mon, 03 Nov 2008 10:36:06 -0500
Received: from mx2.redhat.com ([66.187.237.31]:59097)
	by monty-python.gnu.org with esmtp (Exim 4.60)
	(envelope-from <kraxel@redhat.com>) id 1Kx1TJ-0008DX-KC
	for qemu-devel@nongnu.org; Mon, 03 Nov 2008 10:36:05 -0500
Message-ID: <490F1A5E.9000509@redhat.com>
Date: Mon, 03 Nov 2008 16:35:58 +0100
From: Gerd Hoffmann <kraxel@redhat.com>
MIME-Version: 1.0
Subject: Re: [Qemu-devel] [PATCH 2/4] sockets: helper functions for qemu.
References: <1225457254-1000-1-git-send-email-kraxel@redhat.com>
	<1225457254-1000-3-git-send-email-kraxel@redhat.com>
	<490B424C.7060404@codemonkey.ws>
In-Reply-To: <490B424C.7060404@codemonkey.ws>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Reply-To: qemu-devel@nongnu.org
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.gnu.org/pipermail/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Anthony Liguori <anthony@codemonkey.ws>
Cc: qemu-devel@nongnu.org

Anthony Liguori wrote:
> I like this patch, but we already have a qemu_socket.h.  Please remove
> qemu_socket.h if you're going to introduce qemu-sockets.h.

Will fix.

>> +
>> +listen:
>> +    if (0 != listen(slisten,1)) {
> 
> Please try to avoid this style of if().

Yea, old habit from the days where gcc didn't warn on "if (a = -1)".
Pointless these days.  Trying to get rid of it ...

>> +    { "ipv4", 0, QEMU_OPTION_ipv4 },
>> +    { "ipv6", 0, QEMU_OPTION_ipv6 },
> 
> I don't like the idea of aliasing these options.  Please just stick with
> one set of options.

> And do we really need to have options for this?

Yes, we do, unfortunaly.

> Can't we just do the
> right thing?  I can't believe that every application has to have an ipv6
> switch to be ipv6 enabled.

98% of the users should never ever need that.  Unfortunaly there are
always corner cases where you can't get it right automatically.  You can
easily check the ipv6 setup on the local machine, but you still don't
know how the network is setup and whenever reaching machine $foo via
ipv6 actually works.  Of course you can just try and in case it doesn't
work fallback to ipv4.  Which is what the code does btw.  But that
involves a noticeable delay, waiting for the ipv6 connect attempt time out.

Will make that a per-connection option as discussed further down this
thread.

Respin of the patches will follow later today or tomorrow.

cheers,
  Gerd