From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from [140.186.70.92] (port=52214 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1PsNVR-0005sK-FW
	for qemu-devel@nongnu.org; Wed, 23 Feb 2011 17:48:28 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <quintela@redhat.com>) id 1PsNVH-0000Lm-KS
	for qemu-devel@nongnu.org; Wed, 23 Feb 2011 17:48:20 -0500
Received: from mx1.redhat.com ([209.132.183.28]:38339)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <quintela@redhat.com>) id 1PsNVH-0000LV-8W
	for qemu-devel@nongnu.org; Wed, 23 Feb 2011 17:48:15 -0500
From: Juan Quintela <quintela@redhat.com>
In-Reply-To: <4D6587C7.6030604@codemonkey.ws> (Anthony Liguori's message of
	"Wed, 23 Feb 2011 16:18:47 -0600")
References: <cover.1298492768.git.quintela@redhat.com>
	<8b675b6ffee29ec15f2b73a9fd47e334429f5c5c.1298492768.git.quintela@redhat.com>
	<4D6587C7.6030604@codemonkey.ws>
Date: Wed, 23 Feb 2011 23:46:45 +0100
Message-ID: <m3mxlm2wtm.fsf@trasno.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Subject: [Qemu-devel] Re: [PATCH 16/28] migration: use global variable
	directly
Reply-To: quintela@redhat.com
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Anthony Liguori <anthony@codemonkey.ws>
Cc: qemu-devel@nongnu.org

Anthony Liguori <anthony@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<quintela@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;
>>