Re: [Qemu-devel] [PATCH v4 16/47] Return path: Source handling of return path

[Paolo Bonzini]

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