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

[Peter Maydell]

On 5 February 2018 at 10:26, Marcel Apfelbaum <address@hidden> 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