Anthony Liguori <anth...@codemonkey.ws> 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<quint...@redhat.com>
>> ---
>> 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;
>>
