Re: [Qemu-devel] [PULL v4 00/11] rdma: migration support

[Michael S. Tsirkin]

On Thu, Apr 18, 2013 at 09:49:37AM -0400, Michael R. Hines wrote:
> On 04/18/2013 03:00 AM, Michael S. Tsirkin wrote:
> >On Wed, Apr 17, 2013 at 07:07:09PM -0400, address@hidden wrote:
> >>From: "Michael R. Hines" <address@hidden>
> >>
> >>The following changes since commit e0a83fc2c1582dc8d4453849852ebe6c258b7c3a:
> >>
> >>   qom: do nothing on unparent of object without parent (2013-04-16 
> >> 16:10:21 -0500)
> >>
> >>are available in the git repository at:
> >>
> >>   address@hidden:hinesmr/qemu.git rdma_patch_v4
> >It is customary to include performance numbers
> >showing the gain from the patch in the cover letter.
> >Github and wiki links are nice but don't require us
> >to chase this information please.
> 
> Acknowledged.
> 
> >
> >>for you to fetch changes up to 60bfc321613cd300af2ec0f7142e8ac1a5a8bdfd:
> >>
> >>   rdma: add documentation (2013-04-17 19:04:32 -0400)
> >This is v4 and no changelog is provided.
> >This makes review very hard - we are supposed to
> >re-review everything.
> 
> Sorry - again I was scripting. I will stop scripting.
> 
> >I looked at the documentation patch and not all of my
> >past comments on documentation were addressed.
> >Please address and log changes.
> 
> I go very carefully through all of the reviews and make it a point
> to include all of the comments that were agreed upon.
> 
> Exactly which comment did I forget?
> 
> Thanks,
> - Michael

I tried to point some of them out in the mail I sent.
I think even if author does not agree with reviewer, it's often best to
partially address the review by adding comment in code or improving
documentation.  If a reviewer was confused, so will the reader and the
user.

> >>----------------------------------------------------------------
> >>Michael R. Hines (11):
> >>       rdma: export yield_until_fd_readable()
> >>       rdma: introduce qemu_ram_foreach_block()
> >>       rdma: introduce qemu_file_mode_is_not_valid()
> >>       rdma: export ram_handle_compressed()
> >>       rdma: export qemu_fflush()
> >>       rdma: new QEMUFileOps hooks
> >>       rdma: introduce capability for chunk registration
> >>       rdma: core logic
> >>       rdma: send pc.ram
> >>       rdma: print out throughput while debugging
> >>       rdma: add documentation
> >>
> >>  Makefile.objs                 |    1 +
> >>  arch_init.c                   |   59 +-
> >>  configure                     |   29 +
> >>  docs/rdma.txt                 |  360 ++++++
> >>  exec.c                        |    9 +
> >>  include/block/coroutine.h     |    6 +
> >>  include/exec/cpu-common.h     |    5 +
> >>  include/migration/migration.h |   23 +
> >>  include/migration/qemu-file.h |   31 +
> >>  migration-rdma.c              | 2667 
> >> +++++++++++++++++++++++++++++++++++++++++
> >>  migration.c                   |   26 +-
> >>  qapi-schema.json              |    8 +-
> >>  qemu-coroutine-io.c           |   23 +
> >>  savevm.c                      |  107 +-
> >>  14 files changed, 3305 insertions(+), 49 deletions(-)
> >>  create mode 100644 docs/rdma.txt
> >>  create mode 100644 migration-rdma.c
> >>-- 
> >>1.7.10.4