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

[Juan Quintela <quintela@redhat.com>]

Anthony Liguori <anthony@codemonkey.ws> 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<quintela@redhat.com>
>> ---
>>   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 =&current_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.