Il 03/10/2014 19:47, Dr. David Alan Gilbert (git) ha scritto:
> +/* Source side RP state */
> +struct MigrationRetPathState {
> + uint32_t latest_ack;
> + QemuThread rp_thread;
> + bool error;
Should the QemuFile be in here?
> +};
> +
Also please do not abbrev words, and add a typedef that matches the
struct if it is useful. If it is not, just embed the struct without
giving the type a name (struct { } rp_state).
> +static bool migration_already_active(MigrationState *ms)
> +{
> + switch (ms->state) {
> + case MIG_STATE_ACTIVE:
> + case MIG_STATE_SETUP:
> + return true;
> +
> + default:
> + return false;
> +
> + }
> +}
Should CANCELLING also be considered active? It is on the source->dest
path.
>
> +static void await_outgoing_return_path_close(MigrationState *ms)
> +{
> + /*
> + * If this is a normal exit then the destination will send a SHUT and the
> + * rp_thread will exit, however if there's an error we need to cause
> + * it to exit, which we can do by a shutdown.
> + * (canceling must also shutdown to stop us getting stuck here if
> + * the destination died at just the wrong place)
> + */
> + if (qemu_file_get_error(ms->file) && ms->return_path) {
> + qemu_file_shutdown(ms->return_path);
> + }
As mentioned early, I think it's simpler to let these function handle
themselves the case where there is no return path, and call them
unconditionally.
Paolo
