[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PULL 0/4] RDMA patches
From: |
Marcel Apfelbaum |
Subject: |
Re: [Qemu-devel] [PULL 0/4] RDMA patches |
Date: |
Thu, 8 Feb 2018 15:38:59 +0200 |
User-agent: |
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 |
Hi Peter,
On 08/02/2018 14:59, Peter Maydell wrote:
> 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:
Thanks for doing it!
>
> (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.
>
No real preference, I will modify the license.
> (2) Some new files have no copyright or license comment at the
> top of them. Can you fix that, please?
>
Sure.
> (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.)
>
We do not "use" the __u32 types, we just copied a kernel file
for structures used for communication between the guest driver
and the QEMU code. We had a look on how it is done and
we use the model that adds macros __u32 -> uint32_t,
so the "__types" do not really create such problems.
> (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.
>
The patch did receive several questions/comments and all
of them were addressed, but indeed no RB tag was given.
Since the patch was in the mailing list for over a month
and *was* reviewed, I thought is enough.
I will ping Eduardo, he had the latest comments for it.
> (5) This is an absolutely enormous diffstat for a single commit:
> 26 files changed, 5149 insertions(+), 4 deletions(-)
>
On the github where the project was developed we have thousands of commits,
so it can't be used.
It was reviewed closely by one reviewer and got a lot
of comments from others.
That being said, we will try to split it in a few patches
and send a new version.
Thanks for the comments,
Marcel
> thanks
> -- PMM
>
- [Qemu-devel] [PULL 0/4] RDMA patches, Marcel Apfelbaum, 2018/02/05
- [Qemu-devel] [PULL 1/4] mem: add share parameter to memory-backend-ram, Marcel Apfelbaum, 2018/02/05
- [Qemu-devel] [PULL 2/4] docs: add pvrdma device documentation., Marcel Apfelbaum, 2018/02/05
- [Qemu-devel] [PULL 4/4] MAINTAINERS: add entry for hw/rdma, Marcel Apfelbaum, 2018/02/05
- [Qemu-devel] [PULL 3/4] pvrdma: initial implementation, Marcel Apfelbaum, 2018/02/05
- Re: [Qemu-devel] [PULL 0/4] RDMA patches, Marcel Apfelbaum, 2018/02/07
- Re: [Qemu-devel] [PULL 0/4] RDMA patches, Peter Maydell, 2018/02/08
- Re: [Qemu-devel] [PULL 0/4] RDMA patches, Philippe Mathieu-Daudé, 2018/02/08
- Re: [Qemu-devel] [PULL 0/4] RDMA patches, Marcel Apfelbaum, 2018/02/08
Re: [Qemu-devel] [PULL 0/4] RDMA patches, Michael S. Tsirkin, 2018/02/08