Re: [PATCH 1/2] migration: move wait-unplug loop to its own function

[Juan Quintela]

Laurent Vivier <lvivier@redhat.com> wrote:
> The loop is used in migration_thread() and bg_migration_thread(),
> so we can move it to its own function and call it from these both places.
>
> Moreover, in migration_thread() we have a wrong state transition from
> SETUP to ACTIVE while state could be WAIT_UNPLUG. This is correctly
> managed in bg_migration_thread() so use this code instead.
>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>

If you have to repost:


> +/*
> + * if failover devices are present, wait they are completely
> + * unplugged
> + */
> +
> +static void qemu_savevm_wait_unplug(MigrationState *s, int old_state,
> +                                    int new_state)

old_state and new state are always the same. SETUP -> ACTIVE.  I think
we can hardcode them.


> +{
> +    if (qemu_savevm_state_guest_unplug_pending()) {
> +        migrate_set_state(&s->state, old_state, 
> MIGRATION_STATUS_WAIT_UNPLUG);
> +
> +        while (s->state == MIGRATION_STATUS_WAIT_UNPLUG &&
> +               qemu_savevm_state_guest_unplug_pending()) {
> +            qemu_sem_timedwait(&s->wait_unplug_sem, 250);

I still don't understand why are we using a semaphore when we just want
a timer :-(

Yes, this is independent of this patch.