From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:39839) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RB6J4-0006VI-78 for qemu-devel@nongnu.org; Tue, 04 Oct 2011 10:49:22 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RB6J2-0003SJ-Rj for qemu-devel@nongnu.org; Tue, 04 Oct 2011 10:49:18 -0400 Received: from mx1.redhat.com ([209.132.183.28]:50003) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RB6J2-0003SC-Ho for qemu-devel@nongnu.org; Tue, 04 Oct 2011 10:49:16 -0400 From: Juan Quintela In-Reply-To: <4E8B182D.9050603@codemonkey.ws> (Anthony Liguori's message of "Tue, 04 Oct 2011 09:29:01 -0500") References: <4E8B182D.9050603@codemonkey.ws> Date: Tue, 04 Oct 2011 16:49:12 +0200 Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Subject: Re: [Qemu-devel] [PATCH 17/23] migration: make sure we always have a migration state Reply-To: quintela@redhat.com List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: qemu-devel@nongnu.org Anthony Liguori wrote: > On 09/23/2011 07:57 AM, Juan Quintela wrote: >> This cleans up a lot the code as we don't have to check anymore if >> the variable is NULL or not. >> >> We don't make it static, because when we integrate fault tolerance, we >> can have several migrations at once. >> >> Signed-off-by: Juan Quintela >> --- >> migration.c | 126 +++++++++++++++++++++++++--------------------------------- >> 1 files changed, 54 insertions(+), 72 deletions(-) >> >> diff --git a/migration.c b/migration.c >> index 1cc21f0..c382383 100644 >> --- a/migration.c >> +++ b/migration.c >> @@ -34,7 +34,13 @@ >> /* Migration speed throttling */ >> static int64_t max_throttle = (32<< 20); >> >> -static MigrationState *current_migration; >> +/* When we add fault tolerance, we could have several >> + migrations at once. For now we don't need to add >> + dynamic creation of migration */ >> +static MigrationState current_migration_static = { >> + .state = MIG_STATE_NONE, >> +}; >> +static MigrationState *current_migration =¤t_migration_static; > > This doesn't make sense to me. We can have two migration states that > are both in the MIG_STATE_NONE? What does that actually mean? > > I don't see the point in making migration state nullable via the state > member other than avoiding the if (s == NULL) check. It avoids s==NULL checks, and it also avoids having to have new variables (max_throotle) just because we don't have a migration state handy. Obviously the problem can't be the size (the variable is smaller that all the code tests for NULL). For me, it is easier to understand that we have an state (migration never attempted) with the same states that the other migrations states. Later, Juan.