Re: [Qemu-devel] [PULL 0/4] RDMA patches

["Michael S. Tsirkin" <mst@redhat.com>]

On Thu, Feb 08, 2018 at 12:59:02PM +0000, Peter Maydell wrote:
> On 5 February 2018 at 10:26, Marcel Apfelbaum <marcel@redhat.com> wrote:
> > The following changes since commit f24ee107a07f093bd7ed475dd48d7ba57ea3d8fe:
> >
> >   Merge remote-tracking branch 'remotes/kraxel/tags/ui-20180202-pull-request' into staging (2018-02-02 18:54:11 +0000)
> >
> > are available in the git repository at:
> >
> >   https://github.com/marcel-apf/qemu tags/rdma-pull-request
> >
> > for you to fetch changes up to f172ba1b02724fb66dabd69cd553cfa625b413e5:
> >
> >   MAINTAINERS: add entry for hw/rdma (2018-02-05 11:53:00 +0200)
> >
> > ----------------------------------------------------------------
> > PVRDMA implementation
> >
> > ----------------------------------------------------------------
> > Marcel Apfelbaum (3):
> >       mem: add share parameter to memory-backend-ram
> >       docs: add pvrdma device documentation.
> >       MAINTAINERS: add entry for hw/rdma
> >
> > Yuval Shaia (1):
> >       pvrdma: initial implementation
> 
> Hi. The technical details of this pullreq are all fine (pgp
> key, format, etc), and it passes my build tests. But I gave
> this pullreq a bit of a closer inspection than I normally
> would, since it's your first, and there are a few things I
> thought worth bringing up:
> 
> (1) I notice that some of the new files in this pullreq are licensed
> as "GPL, version 2", rather than "version 2 or any later version".
> Did you really mean that? Per 'LICENSE', we have a strong preference
> for 2-or-later for new code.
> 
> (2) Some new files have no copyright or license comment at the
> top of them. Can you fix that, please?
> 
> (3) Some of the new headers use kernel-internals __u32 etc types.
> This isn't portable. ('HACKING' has some suggestions for types you
> might want instead.)
> 
> (4) One of your patches doesn't have any reviewed-by tags.
> We don't always manage to review everything, but it is
> nicer if we can get review, especially for patches from
> new submaintainers.
> 
> (5) This is an absolutely enormous diffstat for a single commit:
>  26 files changed, 5149 insertions(+), 4 deletions(-)
> 
> thanks
> -- PMM

And one of the reasons is that it pulls in some unneeded stuff.
E.g.  vmw_pvrdma-abi.h should be pulled into standard-headers
from Linux, rather than copy-pasted.

-- 
MST