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

[Michael S. Tsirkin]

On Thu, Apr 18, 2013 at 05:28:14PM -0400, Michael R. Hines wrote:
> On 04/18/2013 04:12 PM, Michael S. Tsirkin wrote:
> >On Thu, Apr 18, 2013 at 03:17:39PM -0400, Michael R. Hines wrote:
> >>On 04/18/2013 09:50 AM, Michael S. Tsirkin wrote:
> >>>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.
> >>I think the one I missed was "ulimit -l" + ibv_reg_mr() + cancel migration.
> >>
> >>I have a crappy description of this listed in TODO, but I can added
> >>it to the "Before Running" section.
> >>
> >>Would that be sufficient?
> >No, I listed other missing things.
> >
> You could be helpful by clearly pointing out whatever it is that
> you're concerned about.
> 
> I'm not gonna go flip through every email of the last 200+ email exchanges.
> 
> - Michael
> 
> 
> - Michael
> 

You didn't address all comments to v4 that I sent only today.

-- 
MST