[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] Re: [PATCH 16/28] migration: use global variable directly
From: |
Juan Quintela |
Subject: |
[Qemu-devel] Re: [PATCH 16/28] migration: use global variable directly |
Date: |
Wed, 23 Feb 2011 23:46:45 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/23.2 (gnu/linux) |
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;
>>
- [Qemu-devel] [PATCH 10/28] migration: Refactor and simplify error checking in migrate_fd_put_ready, (continued)
- [Qemu-devel] [PATCH 10/28] migration: Refactor and simplify error checking in migrate_fd_put_ready, Juan Quintela, 2011/02/23
- [Qemu-devel] [PATCH 09/28] migration: Introduce MIG_STATE_NONE, Juan Quintela, 2011/02/23
- [Qemu-devel] [PATCH 12/28] migration: Our release callback was just free, Juan Quintela, 2011/02/23
- [Qemu-devel] [PATCH 13/28] migration: Remove get_status() accessor, Juan Quintela, 2011/02/23
- [Qemu-devel] [PATCH 14/28] migration: Remove migration cancel() callback, Juan Quintela, 2011/02/23
- [Qemu-devel] [PATCH 08/28] migration: Check that migration is active before cancel it, Juan Quintela, 2011/02/23
- [Qemu-devel] [PATCH 06/28] migration: Make all posible migration functions static, Juan Quintela, 2011/02/23
- [Qemu-devel] [PATCH 03/28] migration: Fold MigrationState into FdMigrationState, Juan Quintela, 2011/02/23
- [Qemu-devel] [PATCH 16/28] migration: use global variable directly, Juan Quintela, 2011/02/23
[Qemu-devel] [PATCH 17/28] migration: another case of global variable assigned to local one, Juan Quintela, 2011/02/23
[Qemu-devel] [PATCH 18/28] migration: convert current_migration from pointer to struct, Juan Quintela, 2011/02/23
[Qemu-devel] [PATCH 19/28] migration: Use bandwidth_limit directly, Juan Quintela, 2011/02/23
[Qemu-devel] [PATCH 07/28] migration: move migrate_create_state to do_migrate, Juan Quintela, 2011/02/23