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