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

[Michael R. Hines]

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