Re: [Qemu-devel] [PATCH 17/23] migration: make sure we always have a migration state

[Orit Wasserman <owasserm@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2098 bytes --]

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.
>



[-- Attachment #2: Type: text/html, Size: 3305 bytes --]