Re: [Qemu-devel] [PATCH 14/22] migration: Remove get_status() accessor

[Yoshiaki Tamura]

2011/2/23 Juan Quintela <address@hidden>:
> It is only used inside migration.c, and fields on that struct are
> accessed all around the place on that file.
>
> Signed-off-by: Juan Quintela <address@hidden>
> ---
>  migration.c |   16 +++++-----------
>  migration.h |    1 -
>  2 files changed, 5 insertions(+), 12 deletions(-)
>
> diff --git a/migration.c b/migration.c
> index dfe6a96..2b873fa 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -90,7 +90,7 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject 
> **ret_data)
>     int ret;
>
>     if (current_migration &&
> -        current_migration->get_status(current_migration) == 
> MIG_STATE_ACTIVE) {
> +        current_migration->state == MIG_STATE_ACTIVE) {
>         monitor_printf(mon, "migration already in progress\n");
>         return -1;
>     }
> @@ -135,7 +135,7 @@ int do_migrate_cancel(Monitor *mon, const QDict *qdict, 
> QObject **ret_data)
>  {
>     MigrationState *s = current_migration;
>
> -    if (s && s->get_status(s) == MIG_STATE_ACTIVE)
> +    if (s && s->state == MIG_STATE_ACTIVE)
>         s->cancel(s);
>
>     return 0;
> @@ -234,7 +234,7 @@ void do_info_migrate(Monitor *mon, QObject **ret_data)
>     if (current_migration) {
>         MigrationState *s = current_migration;
>
> -        switch (s->get_status(current_migration)) {
> +        switch (s->state) {
>         case MIG_STATE_NONE:
>             /* no migration has happened ever */
>             break;
> @@ -375,7 +375,7 @@ static void migrate_fd_put_ready(void *opaque)
>         } else {
>             migrate_fd_completed(s);
>         }
> -        if (s->get_status(s) != MIG_STATE_COMPLETED) {
> +        if (s->state != MIG_STATE_COMPLETED) {
>             if (old_vm_running) {
>                 vm_start();
>             }
> @@ -383,11 +383,6 @@ static void migrate_fd_put_ready(void *opaque)
>     }
>  }
>
> -static int migrate_fd_get_status(MigrationState *s)
> -{
> -    return s->state;
> -}
> -
>  static void migrate_fd_cancel(MigrationState *s)
>  {
>     if (s->state != MIG_STATE_ACTIVE)
> @@ -442,7 +437,7 @@ void remove_migration_state_change_notifier(Notifier 
> *notify)
>  int get_migration_state(void)
>  {
>     if (current_migration) {
> -        return migrate_fd_get_status(current_migration);
> +        return current_migration->state;
>     } else {
>         return MIG_STATE_ERROR;
>     }
> @@ -477,7 +472,6 @@ static MigrationState *migrate_create_state(Monitor *mon, 
> int64_t bandwidth_limi
>     MigrationState *s = qemu_mallocz(sizeof(*s));
>
>     s->cancel = migrate_fd_cancel;
> -    s->get_status = migrate_fd_get_status;
>     s->blk = blk;
>     s->shared = inc;
>     s->mon = NULL;
> diff --git a/migration.h b/migration.h
> index 5455d8b..58a6e06 100644
> --- a/migration.h
> +++ b/migration.h
> @@ -37,7 +37,6 @@ struct MigrationState
>     int (*close)(MigrationState*);
>     int (*write)(MigrationState*, const void *, size_t);
>     void (*cancel)(MigrationState *s);
> -    int (*get_status)(MigrationState *s);
>     void *opaque;
>     int blk;
>     int shared;

I agree to access s->state directly inside of migration.c, but I
disagree to remove get_status() accessor right away.  We don't
have strong motivations for doing that AFAIK.

Yoshi

> --
> 1.7.4
>
>
>