From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from [140.186.70.92] (port=36160 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1PsNOF-0001sa-TV
	for qemu-devel@nongnu.org; Wed, 23 Feb 2011 17:41:01 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <anthony@codemonkey.ws>) id 1PsN2e-0001uF-IK
	for qemu-devel@nongnu.org; Wed, 23 Feb 2011 17:18:41 -0500
Received: from mail-vx0-f173.google.com ([209.85.220.173]:45718)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <anthony@codemonkey.ws>) id 1PsN2e-0001u9-Fj
	for qemu-devel@nongnu.org; Wed, 23 Feb 2011 17:18:40 -0500
Received: by vxb41 with SMTP id 41so3201914vxb.4
	for <qemu-devel@nongnu.org>; Wed, 23 Feb 2011 14:18:39 -0800 (PST)
Message-ID: <4D6587C7.6030604@codemonkey.ws>
Date: Wed, 23 Feb 2011 16:18:47 -0600
From: Anthony Liguori <anthony@codemonkey.ws>
MIME-Version: 1.0
Subject: Re: [Qemu-devel] [PATCH 16/28] migration: use global variable directly
References: <cover.1298492768.git.quintela@redhat.com>
	<8b675b6ffee29ec15f2b73a9fd47e334429f5c5c.1298492768.git.quintela@redhat.com>
In-Reply-To: <8b675b6ffee29ec15f2b73a9fd47e334429f5c5c.1298492768.git.quintela@redhat.com>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
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: Juan Quintela <quintela@redhat.com>
Cc: qemu-devel@nongnu.org

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.

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