From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45367) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ur7Lr-0000qd-CV for qemu-devel@nongnu.org; Mon, 24 Jun 2013 10:02:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Ur7Ll-00030E-Q4 for qemu-devel@nongnu.org; Mon, 24 Jun 2013 10:02:39 -0400 Received: from mx1.redhat.com ([209.132.183.28]:12258) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ur7Ll-00030A-HD for qemu-devel@nongnu.org; Mon, 24 Jun 2013 10:02:33 -0400 Message-ID: <51C85172.7020400@redhat.com> Date: Mon, 24 Jun 2013 16:02:26 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1372081590-24506-1-git-send-email-mrhines@linux.vnet.ibm.com> <1372081590-24506-15-git-send-email-mrhines@linux.vnet.ibm.com> <51C84EDD.4000508@redhat.com> <51C84FC3.3060302@linux.vnet.ibm.com> In-Reply-To: <51C84FC3.3060302@linux.vnet.ibm.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v10 14/14, resend] rdma: add setup time accounting to QMP statistics List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael R. Hines" Cc: aliguori@us.ibm.com, quintela@redhat.com, qemu-devel@nongnu.org, owasserm@redhat.com, abali@us.ibm.com, mrhines@us.ibm.com, gokul@us.ibm.com, chegu_vinod@hp.com, knoel@redhat.com Il 24/06/2013 15:55, Michael R. Hines ha scritto: >>> >>> Reviewed-by: Paolo Bonzini >> Please stop inventing Reviewed-by tags, or I will stop reviewing your >> patches. >> >> Paolo > > Inventing? I don't understand. > > I accumulated all of those tags from everybody - copy and pasted > directly from everyone when they gave them to me.\ > > What exactly is wrong here? I didn't review this patch in particular, or at least it was changed very heavily since I last reviewed it. I certainly haven't reviewed this: >>> +static void migrate_set_state(MigrationState *s, int old_state, int new_state) >>> { >>> - if (__sync_val_compare_and_swap(&s->state, MIG_STATE_ACTIVE, >>> + if (__sync_val_compare_and_swap(&s->state, old_state, >>> new_state) == new_state) { and I wouldn't have approved it in fact. In this shape, the patch should be split in at least 3 or 4 pieces. In v9 I asked to not apply patch 14 of v9 (I don't remember if you had already the R-b there); in v10 you rewrote patch 14 and I pointed out that the Reviewed-by tag was coming out of thin air; now you resent the series and left the wrong tag. That's why I started to be a bit grumpy. By now you must have learnt that I want changes outside migration-rdma.c to be as a) minimal b) separate c) well-described as possible. What is _exactly_ the reason why you're changing the state machine? What is the change exactly, in fact? Paolo