Re: [Qemu-devel] [PATCH v13 11/12] migration: Remove not needed semaphore and quit

[Juan Quintela <quintela@redhat.com>]

"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> * Juan Quintela (quintela@redhat.com) wrote:
>> We know quit closing the QIO.
>
> This patch does two different things; one of which I think I understand.
>
> The 'quit' has been removed - I think that makes sense because the
> multifd threads terminate when either they come to the end of a stream
> or hit an error; there's no case of asking them to quit explicitly.
> The 'sem' was basically for kicking the recv-thread to quit; which again
> isn't needed.
>
> Now, what about the object_unref on p->c ?
> If I've got this right multifd_recv_terminate_threads is only called in
> the error case; but doesn't multifd_load_cleanup also get called in that
> case - it does the unref and p->c = NULL as well.

Adding another comment O:-)

In normal exit case: we don't care.

In error case, we do a close of the p->c.  Doing a close, means that the
channel threads that are waiting on qio_channel_read_all_eof() will stop
the waiting and return an error, so we are safe.

        ret = qio_channel_read_all_eof(p->c, (void *)p->packet,
                                       p->packet_len, &local_err);
        if (ret == 0) {   /* EOF */
            break;
        }
        if (ret == -1) {   /* Error */
            break;
        }

We have to wait for three things:
- we receive a synchronization packet
- we receive EOF and finish
- we got somehow an error and have to quit.

Old versions have a semaphore, and we have three places where we set
that semaphore "we are ready", when we receive one error, when we have
data waiting to be read (it had a qio whatcher) or we just got a normal
exit.

Waiting on two things make code more complex that it used to be (and
whatchers are really lame, because we know there are data ready, but not
how much, dealing with non-whole packets/pages is a real mess).  So we
moved to:
-  quit is gone: we just close the channel, that makes the *_read_eof()
   to end.  Notice that we don't care if the close is due to one error
   or because we have finished.  It is just done.
- Now reception channel only have to wait on _read_eof(), the three
  cases that we talked before are handled correctly.

I understand the confusion, this only makes sense when you came from the
previous version of the patchset.

Later, Juan.