From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:55884) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1USui9-0004LT-NS for qemu-devel@nongnu.org; Thu, 18 Apr 2013 15:41:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1USui6-0003fv-Tn for qemu-devel@nongnu.org; Thu, 18 Apr 2013 15:41:37 -0400 Received: from e7.ny.us.ibm.com ([32.97.182.137]:50603) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1USui6-0003fo-Q2 for qemu-devel@nongnu.org; Thu, 18 Apr 2013 15:41:34 -0400 Received: from /spool/local by e7.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 18 Apr 2013 15:41:34 -0400 Received: from d01relay01.pok.ibm.com (d01relay01.pok.ibm.com [9.56.227.233]) by d01dlp01.pok.ibm.com (Postfix) with ESMTP id 9C94738C8067 for ; Thu, 18 Apr 2013 15:41:31 -0400 (EDT) Received: from d01av05.pok.ibm.com (d01av05.pok.ibm.com [9.56.224.195]) by d01relay01.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r3IJfV5J286100 for ; Thu, 18 Apr 2013 15:41:31 -0400 Received: from d01av05.pok.ibm.com (loopback [127.0.0.1]) by d01av05.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id r3IJfV9J011362 for ; Thu, 18 Apr 2013 15:41:31 -0400 Message-ID: <51704C6A.70303@linux.vnet.ibm.com> Date: Thu, 18 Apr 2013 15:41:30 -0400 From: "Michael R. Hines" MIME-Version: 1.0 References: <1366240040-10730-1-git-send-email-mrhines@linux.vnet.ibm.com> <1366240040-10730-9-git-send-email-mrhines@linux.vnet.ibm.com> <516FB285.2040701@redhat.com> <516FFB11.90505@linux.vnet.ibm.com> <51701690.5070306@redhat.com> In-Reply-To: <51701690.5070306@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PULL v4 08/11] rdma: core logic List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Orit Wasserman Cc: aliguori@us.ibm.com, quintela@redhat.com, mst@redhat.com, qemu-devel@nongnu.org, abali@us.ibm.com, mrhines@us.ibm.com, gokul@us.ibm.com, pbonzini@redhat.com On 04/18/2013 11:51 AM, Orit Wasserman wrote: > On 04/18/2013 04:54 PM, Michael R. Hines wrote: >> On 04/18/2013 04:44 AM, Orit Wasserman wrote: >>> Hi Michael, >>> >>> I don't see you addressed any of the comment I had in v3 >>> (especially the error handling) >>> >>> please, fix those >> I did fix them, but not all of your comments were correct, >> because I was passing errp in many places were errp >> did not supposed to belong there. > If you decide not to fix some comment you just need > to reply to the comment and explain the reason, > this makes the reviewing process easier. > Ok, so here's a more specific question while I have your ears, that is a problem I'm having with error_setg(): Currently, the migration works by calling "qemu_set_handler_fd2()", which comes along and calls "accept_incoming_migration" on the destination side. The problem with this from an error-handling perspective is that there is no global datastructure on the destination side, similar to "MigrationState". Thus, there is nowhere to propagate errors to, even if we want to do so - and that's why I deleted most of the error_setg() comments that you had because they were mostly focus on delivering an error without having any part of the call stack to actually *process* the error. So, without overly-complicating error propagation on the destination side, what solution would you recommend for both TCP and RDMA? >> So, first I *removed* errp from being proliferated all over >> the entire file, which was not necessary. >> >> Then, in the places where errp is required by migration.c, >> I added new uses of errp. > That is fine as long as external rdma API use errp and the > internal always return some error code. > > Also I noticed that the errors are very general for example > in qemu_rdma_connect we set the same error always it will be more > helpful to have different errors for each case. We need > errors that can help the user to understand what went wrong. > > Orit >> There are some places, as Paolo mentioned where errp was >> written twice, which I must fix, however. >>