Re: [Qemu-devel] [PATCH v3 00/23] Refactor and cleanup migration code

[Anthony Liguori]

I peeled off the first 4 patches to apply.  Once I finish testing I'll push 
them.

Most of the rest of the series looks good.  I really don't like making 
MigrationState nullable with MIG_STATE_NONE.  I don't think it makes sense with 
multiple MigrationStates.

The direct use of globals is not something I'm happy about, but I'd be inclined 
to hold my nose and apply if it weren't for MIG_STATE_NONE.

Regards,

Anthony Liguori

On 09/23/2011 07:56 AM, Juan Quintela wrote:
> Hi
>
> v4:
>
> rebase on top of new qemu and new migration-errors series
>
>
> v3:
>         this patch applies on top of my previous "migration error"
>         patches.  All error handling has been moved to that series,
>         except for "propagate error correctly", without this
>         refactoring, it is quite complicated to apply it.
>
> Please, review.
>
> Later, Juan.
>
> v3:
> - more checkpatch.pl happines
> - split error handling in a previous series
> - make Anthony happy.  current_migration is still a pointer, but points to
>    a static variable.  We can change current_migration when we integrate
>    kemari.
>
> v2:
> - make Jan^Wcheckpatch.pl happy
> - Yoshiaki Tamura suggestions:
>    - include its two patches to clean things
>    - MAX_THROTTLE define
>    - migration_state enum
> - I removed spurious differences between migration-{tcp,unix}
> - better error propagation, after this patch:
>     migrate -d "tcp:name_don_exist:port"
>     migrate -d "tcp:name:port_dont_exist"
>     migrate -d "exec: prog_dont_exist"
>     migrate -d "exec: gzip>  /path/dont/exist"
>   fail as expected.  Last two used to enter an infinite loop.
>
> The fixes part should be backported to 0.14, waiting for the review to do that.
>
> Later, Juan.
>
> v1:
> This series:
> - Fold MigrationState into FdMigrationState (and then rename)
> - Factorize migration statec creation in a single place
> - Make use of MIG_STATE_*, setup through helpers and make them local
> - remove relase&  cancel callbacks (where used only one in same
>    file than defined)
> - get_status() is no more, just access directly to .state
> - current_migration use cleanup, and make variable static
> - max_throotle is gone, now inside current_migration
> - change get_migration_status() to migration_has_finished()
>    and actualize single user.
>
> Please review.
>
> Later, Juan.
>
> Juan Quintela (23):
>    migration: Make *start_outgoing_migration return FdMigrationState
>    migration: Use FdMigrationState instead of MigrationState when
>      possible
>    migration: Fold MigrationState into FdMigrationState
>    migration: Rename FdMigrationState MigrationState
>    migration: Refactor MigrationState creation
>    migration: Make all posible migration functions static
>    migration: move migrate_create_state to do_migrate
>    migration: Introduce MIG_STATE_NONE
>    migration: Refactor and simplify error checking in
>      migrate_fd_put_ready
>    migration: Introduce migrate_fd_completed() for consistency
>    migration: Our release callback was just free
>    migration: Remove get_status() accessor
>    migration: Remove migration cancel() callback
>    migration: Move exported functions to the end of the file
>    migration: use global variable directly
>    migration: another case of global variable assigned to local one
>    migration: make sure we always have a migration state
>    migration: Use bandwidth_limit directly
>    migration: Export a function that tells if the migration has finished
>      correctly
>    migration: Make state definitions local
>    migration: Don't use callback on file defining it
>    migration: propagate error correctly
>    migration: make migration-{tcp,unix} consistent
>
>   migration-exec.c |   39 +----
>   migration-fd.c   |   42 ++-----
>   migration-tcp.c  |   76 ++++------
>   migration-unix.c |  112 ++++++---------
>   migration.c      |  404 ++++++++++++++++++++++++++---------------------------
>   migration.h      |   85 ++----------
>   ui/spice-core.c  |    4 +-
>   7 files changed, 303 insertions(+), 459 deletions(-)