diff mbox

[15/23] migration: use global variable directly

Message ID 8247c5c1b6078349a1c87e3524b3b7d051df9802.1316782367.git.quintela@redhat.com
State New
Headers show

Commit Message

Juan Quintela Sept. 23, 2011, 12:57 p.m. UTC
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.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

Comments

Anthony Liguori Oct. 4, 2011, 2:27 p.m. UTC | #1
On 09/23/2011 07:57 AM, 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.
>
> 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 9bb089a..d5e0eb0 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -469,7 +469,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) {
> @@ -477,9 +476,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);
>       }

How about we compromise and add a:

MigrationState *migrate_get_current(void);

I'm strongly opposed to propagating direct usage of a global.  If it's at least 
a function call, that's a bit nicer.

Regards,

Anthony Liguori

>
>       return 0;
Juan Quintela Oct. 4, 2011, 2:46 p.m. UTC | #2
Anthony Liguori <anthony@codemonkey.ws> wrote:

> How about we compromise and add a:
>
> MigrationState *migrate_get_current(void);

I can agree with this one.

> I'm strongly opposed to propagating direct usage of a global.  If it's
> at least a function call, that's a bit nicer.

What I am against is with trying to "hide" the fact that we are using a
global.  function accessor looks ok enough.
diff mbox

Patch

diff --git a/migration.c b/migration.c
index 9bb089a..d5e0eb0 100644
--- a/migration.c
+++ b/migration.c
@@ -469,7 +469,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) {
@@ -477,9 +476,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;