Re: [Qemu-devel] [PATCH 00/35] Migration thread (20121211)

[Paolo Bonzini]

Il 11/12/2012 13:46, Juan Quintela ha scritto:
> Please test & comment & review, Juan.

Please, not this way.

This hardly takes into account all of the comments I sent to this series.

For example on 21 September I wrote about patch 4 that "usleep is broken
under IIRC Solaris (it uses signals and conflicts with qemu-timer.c).
Please use g_usleep instead."  It still doesn't do.

At the beginning of November I went through another review of this code.
 I found another small problem in patch 4 (it calls migrate_fd_close
twice).  I commented that patch 6 has a stale commit message, and
incorrectly uses a global.  It still does all of these things.

At the time, I also found that ratelimiting is currently broken
(including in the 1.3 release), and you haven't reviewed my patches to
fix that, nor included them in this series.

You are also ignoring other people's comments too.  Peter mentioned on
21 October "Maybe worth adding a comment explaining why we have the two
lists, to save people having to ferret around in the revision history
later?".  Nope.

But this is all fine.  The worst part is that you *knew* I had worked on
the migration thread code, fixing all the above and more, and redoing
the second half of your series in a way that actually removed the memory
copies and serialized memory contents outside the BQL (no doubt there
are other cleanups that may be in these series and not in mine; I grant
that).  Vinod had tested the branch successfully and Orit is already
using it as a base for further improvements.

I was going to ask you today about how to move on with the migration
thread work.  Instead I found out that we duplicated effort on fixing
the conflicts, and the review comments have fallen on the floor once more.

So, I'm not going to review this series.  I pushed my version of the
patches to github on the migration-thread-20121211 branch.  Please
consider at least reviewing it.

Paolo