From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:52044) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SwytL-0005ev-QZ for qemu-devel@nongnu.org; Thu, 02 Aug 2012 13:08:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SwytK-00071A-Nh for qemu-devel@nongnu.org; Thu, 02 Aug 2012 13:08:55 -0400 Received: from mx1.redhat.com ([209.132.183.28]:54087) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SwytK-00070x-FQ for qemu-devel@nongnu.org; Thu, 02 Aug 2012 13:08:54 -0400 Date: Thu, 2 Aug 2012 14:08:48 -0300 From: Luiz Capitulino Message-ID: <20120802140848.7a2f826d@doriath.home> In-Reply-To: <20120802165411.GA16157@illuin> References: <1343869374-23417-1-git-send-email-lcapitulino@redhat.com> <1343869374-23417-18-git-send-email-lcapitulino@redhat.com> <20120802165411.GA16157@illuin> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 17/34] qerror: drop QERR_SOCKET_CONNECT_IN_PROGRESS List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Michael Roth Cc: kwolf@redhat.com, aliguori@us.ibm.com, qemu-devel@nongnu.org, armbru@redhat.com, pbonzini@redhat.com, eblake@redhat.com On Thu, 2 Aug 2012 11:54:11 -0500 Michael Roth wrote: > On Wed, Aug 01, 2012 at 10:02:37PM -0300, Luiz Capitulino wrote: > > This error is currently returned by inet_connect_opts(), however > > it causes the follow spurious message on HMP: > > > > (qemu) migrate tcp:0:4444 > > migrate: Connection can not be completed immediately > > (qemu) > > > > But migration succeeds. > > I think the core issue is that inet_connect_opts() passes back the > QERR_SOCKET_CONNECT_IN_PROGRESS via Error (which is fine), but that > we have users that erroneous pass this error up the stack, when really, > when specifying blocking=on as one of the options, they should be > expecting and doing specific handling for this error. You're right here. > So if we fix that (by simply using a local Error when doing the call and > using error_propagate() for non QSCIP errors), I think we can basically > drop patches 14-17 by fixing the callers in that manner and just giving QSCIP > it's own error class. I don't think QSCIP errors is something we should report to QMP clients, at least not for the use-case this patch is about, hence we should not have a specific error class for this. As pointed out by Markus in his review, keeping the in_progress flag introduced by patch 14/34 should be enough to drop patches 15 and 16. > Relying on the errno result was something these socket errors were > specifically meant to fix, since errno is set multiple times > throughout the function and extracting an errno reliably requires > callers to examine all the possible error paths and errno setters. So I > think it's a regression to go back to the old behavior, and these were > issues found in inet_connect() when we attempted to generalize it's > usage for non-blocking connections. I'm not completely sure I agree because the new error format doesn't allow callers to programatically know the cause of an failure. That's what errno is for, though. But I'll drop the patch that changes inet_connect() to return errno, so it's not worth it to discuss this specific case.