[Qemu-devel] Re: [PATCH 16/28] migration: use global variable directly

[Juan Quintela]

Anthony Liguori <address@hidden> wrote:
> On 02/23/2011 03:47 PM, Juan Quintela wrote:
>> We are setting a pointer to a local variable in the previous line, just use
>> the global variable directly.  We remove the ->file test because it is 
>> already
>> done inside qemu_file_set_rate_limit() function.
>>    
>
> I think this is bad form generally speaking.  Globals are not
> something to be embraced but rather to be isolated as much as humanly
> possible.

current_migration is a global variable.

And just doing:

s = current_migration;

foo(s);

helps nothing.  We are not going to happen more than one migration at
the same time anytime soon.  Notice that everything that is outside of
migration.c  already receives a pointer to it, i.e. not use the global
one.  So, I claim this is a very special case, not the normal case about
global variables.

Later, Juan.

> Regards,
>
> Anthony Liguori
>
>> Signed-off-by: Juan Quintela<address@hidden>
>> ---
>>   migration.c |    6 ++----
>>   1 files changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/migration.c b/migration.c
>> index caf5044..21f5a9a 100644
>> --- a/migration.c
>> +++ b/migration.c
>> @@ -457,7 +457,6 @@ int do_migrate_cancel(Monitor *mon, const QDict *qdict, 
>> QObject **ret_data)
>>   int do_migrate_set_speed(Monitor *mon, const QDict *qdict, QObject 
>> **ret_data)
>>   {
>>       int64_t d;
>> -    MigrationState *s;
>>
>>       d = qdict_get_int(qdict, "value");
>>       if (d<  0) {
>> @@ -465,9 +464,8 @@ int do_migrate_set_speed(Monitor *mon, const QDict 
>> *qdict, QObject **ret_data)
>>       }
>>       max_throttle = d;
>>
>> -    s = current_migration;
>> -    if (s&&  s->file) {
>> -        qemu_file_set_rate_limit(s->file, max_throttle);
>> +    if (current_migration) {
>> +        qemu_file_set_rate_limit(current_migration->file, max_throttle);
>>       }
>>
>>       return 0;
>>