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

[Dr. David Alan Gilbert]

* Paolo Bonzini (address@hidden) wrote:
> 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?

Yes, done.

> > +};
> > +
> 
> 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).

Done.

> 
> > +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.

Hmm, possibly - but my intention here was just to round up all of the
places that already checked for ACTIVE+SETUP so that I could add 
POSTCOPY_ACTIVE;
only one of those places also checked for CANCELLING, so I left it out.

> > +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.

I still need to think about that.

Dave

--
Dr. David Alan Gilbert / address@hidden / Manchester, UK