From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:58631) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RBjyb-0007Gc-Oi for qemu-devel@nongnu.org; Thu, 06 Oct 2011 05:10:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RBjya-0006cC-Dd for qemu-devel@nongnu.org; Thu, 06 Oct 2011 05:10:49 -0400 Received: from mx1.redhat.com ([209.132.183.28]:55816) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RBjya-0006by-6M for qemu-devel@nongnu.org; Thu, 06 Oct 2011 05:10:48 -0400 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p969AkpX018044 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 6 Oct 2011 05:10:46 -0400 Received: from dhcp-1-120.tlv.redhat.com (dhcp-1-120.tlv.redhat.com [10.35.1.120]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id p969AjqP029623 for ; Thu, 6 Oct 2011 05:10:46 -0400 Message-ID: <4E8D7063.7090404@redhat.com> Date: Thu, 06 Oct 2011 11:09:55 +0200 From: Orit Wasserman MIME-Version: 1.0 References: <4E8B182D.9050603@codemonkey.ws> <4E8CB7F2.7000405@codemonkey.ws> In-Reply-To: Content-Type: multipart/alternative; boundary="------------090008010701050300050403" Subject: Re: [Qemu-devel] [PATCH 17/23] migration: make sure we always have a migration state List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org This is a multi-part message in MIME format. --------------090008010701050300050403 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit On 10/05/2011 10:25 PM, Juan Quintela wrote: > Anthony Liguori wrote: >> On 10/04/2011 09:49 AM, Juan Quintela wrote: >>> Anthony Liguori wrote: >>>> On 09/23/2011 07:57 AM, Juan Quintela wrote: > [ much more stuff ] > >>> It avoids s==NULL checks, >> In favor of s->state == MIG_STATE_NONE. >> >>> and it also avoids having to have new >>> variables (max_throotle) just because we don't have a migration state >>> handy. >> The intention of the global variable is to set a default for new >> sessions. I can imagine a world where if you had parallel migrations, >> you could either toggle the default (the global variable) or the >> specific parameter within a single migration session. >> >> But it's not about features. It's a general reluctances to heavily >> modify the code to rely on a global instead of passing the state >> around. Here's a pretty good short write up of why this is a Bad >> Thing. >> >> http://stackoverflow.com/questions/1392315/problems-with-singleton-pattern >> >> Ultimately, MIG_STATE_NONE shouldn't be needed because we should not >> be relying on a singleton accessor to get the MigrationState. A >> better cleanup would be to further pass MigrationState between >> functions and eliminate the need for a global at all. > I understand the singleton problem, but the reason to put STATE_NONE is > not for that O:-) (it just happens that we only have a migration now). > > Why I want it? > > Just now, the only thing that we are "setting" for a migration before it > starts is the "bandwidth". I see the future when migration becomes > something like: > > migration_set_speed .... > migration_set_target .... > migration_set_ > migrate > > as you can see, we are "preparing" migration, and we don't have a > "STATE" now that describes this state, migration not started, but we > want to prepare it. > > Perhaps a better name that STATE_NONE is in order. I got NONE in the > sense that "migration" has not been attemted yet. How about STATE_SETUP or STATE_PREPARE ? Cheers, Orit > Later, Juan. > --------------090008010701050300050403 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit On 10/05/2011 10:25 PM, Juan Quintela wrote: 
Anthony Liguori <anthony@codemonkey.ws> wrote:

On 10/04/2011 09:49 AM, Juan Quintela wrote:

Anthony Liguori<anthony@codemonkey.ws>  wrote:

On 09/23/2011 07:57 AM, Juan Quintela wrote:

[ much more stuff ]


It avoids s==NULL checks,

In favor of s->state == MIG_STATE_NONE.


and it also avoids having to have new
variables (max_throotle) just because we don't have a migration state
handy.

The intention of the global variable is to set a default for new
sessions.  I can imagine a world where if you had parallel migrations,
you could either toggle the default (the global variable) or the
specific parameter within a single migration session.

But it's not about features.  It's a general reluctances to heavily
modify the code to rely on a global instead of passing the state
around.  Here's a pretty good short write up of why this is a Bad
Thing.

http://stackoverflow.com/questions/1392315/problems-with-singleton-pattern

Ultimately, MIG_STATE_NONE shouldn't be needed because we should not
be relying on a singleton accessor to get the MigrationState.  A
better cleanup would be to further pass MigrationState between
functions and eliminate the need for a global at all.

I understand the singleton problem, but the reason to put STATE_NONE is
not for that O:-) (it just happens that we only have a migration now).

Why I want it?

Just now, the only thing that we are "setting" for a migration before it
starts is the "bandwidth".  I see the future when migration becomes
something like:

migration_set_speed ....
migration_set_target ....
migration_set_<whatever else>
migrate

as you can see, we are "preparing" migration, and we don't have a
"STATE" now that describes this state, migration not started, but we
want to prepare it.

Perhaps a better name that STATE_NONE is in order.  I got NONE in the
sense that "migration" has not been attemted yet.
How about STATE_SETUP or STATE_PREPARE ?
Cheers,
Orit
Later, Juan.


--------------090008010701050300050403--