On Thu, Feb 08, 2018 at 12:59:02PM +0000, Peter Maydell wrote: > On 5 February 2018 at 10:26, Marcel Apfelbaum <mar...@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
