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

 MAINTAINERS                   |   8 +
 Makefile.objs                 |   2 +
 backends/hostmem-file.c       |  25 +-
 backends/hostmem-ram.c        |   4 +-
 backends/hostmem.c            |  21 ++
 configure                     |   9 +-
 docs/pvrdma.txt               | 255 +++++++++++++
 exec.c                        |  26 +-
 hw/Makefile.objs              |   1 +
 hw/rdma/Makefile.objs         |   5 +
 hw/rdma/rdma_backend.c        | 818 ++++++++++++++++++++++++++++++++++++++++++
 hw/rdma/rdma_backend.h        |  99 +++++
 hw/rdma/rdma_backend_defs.h   |  62 ++++
 hw/rdma/rdma_rm.c             | 544 ++++++++++++++++++++++++++++
 hw/rdma/rdma_rm.h             |  69 ++++
 hw/rdma/rdma_rm_defs.h        | 104 ++++++
 hw/rdma/rdma_utils.c          |  52 +++
 hw/rdma/rdma_utils.h          |  43 +++
 hw/rdma/trace-events          |   5 +
 hw/rdma/vmw/pvrdma.h          | 122 +++++++
 hw/rdma/vmw/pvrdma_cmd.c      | 656 +++++++++++++++++++++++++++++++++
 hw/rdma/vmw/pvrdma_dev_api.h  | 602 +++++++++++++++++++++++++++++++
 hw/rdma/vmw/pvrdma_dev_ring.c | 140 ++++++++
 hw/rdma/vmw/pvrdma_dev_ring.h |  42 +++
 hw/rdma/vmw/pvrdma_ib_verbs.h | 433 ++++++++++++++++++++++
 hw/rdma/vmw/pvrdma_main.c     | 653 +++++++++++++++++++++++++++++++++
 hw/rdma/vmw/pvrdma_qp_ops.c   | 212 +++++++++++
 hw/rdma/vmw/pvrdma_qp_ops.h   |  27 ++
 hw/rdma/vmw/pvrdma_ring.h     | 134 +++++++
 hw/rdma/vmw/trace-events      |   5 +
 hw/rdma/vmw/vmw_pvrdma-abi.h  | 311 ++++++++++++++++
 include/exec/memory.h         |  23 ++
 include/exec/ram_addr.h       |   3 +-
 include/hw/pci/pci_ids.h      |   3 +
 include/qemu/osdep.h          |   2 +-
 include/sysemu/hostmem.h      |   2 +-
 include/sysemu/kvm.h          |   2 +-
 memory.c                      |  16 +-
 qemu-options.hx               |  10 +-
 target/s390x/kvm.c            |   4 +-
 util/oslib-posix.c            |   4 +-
 util/oslib-win32.c            |   2 +-
 42 files changed, 5506 insertions(+), 54 deletions(-)
 create mode 100644 docs/pvrdma.txt
 create mode 100644 hw/rdma/Makefile.objs
 create mode 100644 hw/rdma/rdma_backend.c
 create mode 100644 hw/rdma/rdma_backend.h
 create mode 100644 hw/rdma/rdma_backend_defs.h
 create mode 100644 hw/rdma/rdma_rm.c
 create mode 100644 hw/rdma/rdma_rm.h
 create mode 100644 hw/rdma/rdma_rm_defs.h
 create mode 100644 hw/rdma/rdma_utils.c
 create mode 100644 hw/rdma/rdma_utils.h
 create mode 100644 hw/rdma/trace-events
 create mode 100644 hw/rdma/vmw/pvrdma.h
 create mode 100644 hw/rdma/vmw/pvrdma_cmd.c
 create mode 100644 hw/rdma/vmw/pvrdma_dev_api.h
 create mode 100644 hw/rdma/vmw/pvrdma_dev_ring.c
 create mode 100644 hw/rdma/vmw/pvrdma_dev_ring.h
 create mode 100644 hw/rdma/vmw/pvrdma_ib_verbs.h
 create mode 100644 hw/rdma/vmw/pvrdma_main.c
 create mode 100644 hw/rdma/vmw/pvrdma_qp_ops.c
 create mode 100644 hw/rdma/vmw/pvrdma_qp_ops.h
 create mode 100644 hw/rdma/vmw/pvrdma_ring.h
 create mode 100644 hw/rdma/vmw/trace-events
 create mode 100644 hw/rdma/vmw/vmw_pvrdma-abi.h

-- 
2.13.5

On 05/02/2018 12:26, Marcel Apfelbaum 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
> 
> ----------------------------------------------------------------

Hi Peter,

Sorry for bothering, I wonder if you saw this pull request.
Since is my first one, I just want to be sure I did everything right.

Thanks,
Marcel

> 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
> 
>  MAINTAINERS                   |   8 +
>  Makefile.objs                 |   2 +
>  backends/hostmem-file.c       |  25 +-
>  backends/hostmem-ram.c        |   4 +-
>  backends/hostmem.c            |  21 ++
>  configure                     |   9 +-
>  docs/pvrdma.txt               | 255 +++++++++++++
>  exec.c                        |  26 +-
>  hw/Makefile.objs              |   1 +
>  hw/rdma/Makefile.objs         |   5 +
>  hw/rdma/rdma_backend.c        | 818 ++++++++++++++++++++++++++++++++++++++++++
>  hw/rdma/rdma_backend.h        |  99 +++++
>  hw/rdma/rdma_backend_defs.h   |  62 ++++
>  hw/rdma/rdma_rm.c             | 544 ++++++++++++++++++++++++++++
>  hw/rdma/rdma_rm.h             |  69 ++++
>  hw/rdma/rdma_rm_defs.h        | 104 ++++++
>  hw/rdma/rdma_utils.c          |  52 +++
>  hw/rdma/rdma_utils.h          |  43 +++
>  hw/rdma/trace-events          |   5 +
>  hw/rdma/vmw/pvrdma.h          | 122 +++++++
>  hw/rdma/vmw/pvrdma_cmd.c      | 656 +++++++++++++++++++++++++++++++++
>  hw/rdma/vmw/pvrdma_dev_api.h  | 602 +++++++++++++++++++++++++++++++
>  hw/rdma/vmw/pvrdma_dev_ring.c | 140 ++++++++
>  hw/rdma/vmw/pvrdma_dev_ring.h |  42 +++
>  hw/rdma/vmw/pvrdma_ib_verbs.h | 433 ++++++++++++++++++++++
>  hw/rdma/vmw/pvrdma_main.c     | 653 +++++++++++++++++++++++++++++++++
>  hw/rdma/vmw/pvrdma_qp_ops.c   | 212 +++++++++++
>  hw/rdma/vmw/pvrdma_qp_ops.h   |  27 ++
>  hw/rdma/vmw/pvrdma_ring.h     | 134 +++++++
>  hw/rdma/vmw/trace-events      |   5 +
>  hw/rdma/vmw/vmw_pvrdma-abi.h  | 311 ++++++++++++++++
>  include/exec/memory.h         |  23 ++
>  include/exec/ram_addr.h       |   3 +-
>  include/hw/pci/pci_ids.h      |   3 +
>  include/qemu/osdep.h          |   2 +-
>  include/sysemu/hostmem.h      |   2 +-
>  include/sysemu/kvm.h          |   2 +-
>  memory.c                      |  16 +-
>  qemu-options.hx               |  10 +-
>  target/s390x/kvm.c            |   4 +-
>  util/oslib-posix.c            |   4 +-
>  util/oslib-win32.c            |   2 +-
>  42 files changed, 5506 insertions(+), 54 deletions(-)
>  create mode 100644 docs/pvrdma.txt
>  create mode 100644 hw/rdma/Makefile.objs
>  create mode 100644 hw/rdma/rdma_backend.c
>  create mode 100644 hw/rdma/rdma_backend.h
>  create mode 100644 hw/rdma/rdma_backend_defs.h
>  create mode 100644 hw/rdma/rdma_rm.c
>  create mode 100644 hw/rdma/rdma_rm.h
>  create mode 100644 hw/rdma/rdma_rm_defs.h
>  create mode 100644 hw/rdma/rdma_utils.c
>  create mode 100644 hw/rdma/rdma_utils.h
>  create mode 100644 hw/rdma/trace-events
>  create mode 100644 hw/rdma/vmw/pvrdma.h
>  create mode 100644 hw/rdma/vmw/pvrdma_cmd.c
>  create mode 100644 hw/rdma/vmw/pvrdma_dev_api.h
>  create mode 100644 hw/rdma/vmw/pvrdma_dev_ring.c
>  create mode 100644 hw/rdma/vmw/pvrdma_dev_ring.h
>  create mode 100644 hw/rdma/vmw/pvrdma_ib_verbs.h
>  create mode 100644 hw/rdma/vmw/pvrdma_main.c
>  create mode 100644 hw/rdma/vmw/pvrdma_qp_ops.c
>  create mode 100644 hw/rdma/vmw/pvrdma_qp_ops.h
>  create mode 100644 hw/rdma/vmw/pvrdma_ring.h
>  create mode 100644 hw/rdma/vmw/trace-events
>  create mode 100644 hw/rdma/vmw/vmw_pvrdma-abi.h
> 

On 7 February 2018 at 17:19, Marcel Apfelbaum <marcel@redhat.com> wrote:
> On 05/02/2018 12:26, Marcel Apfelbaum 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
>>
>> ----------------------------------------------------------------
>
> Hi Peter,
>
> Sorry for bothering, I wonder if you saw this pull request.
> Since is my first one, I just want to be sure I did everything right.

It's in my queue to process, yes.

thanks
-- PMM

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

Hi Peter,

On 08/02/2018 14:59, 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:

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
> 

On 8 February 2018 at 13:38, Marcel Apfelbaum <marcel@redhat.com> wrote:
> Hi Peter,
>
> On 08/02/2018 14:59, Peter Maydell wrote:
>> (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.

If we're using kernel header files, I would recommend using
the scripts/update-headers machinery for this, the way we do
for other kernel headers. Among other things, the cp_portable
function in that script will fix up the type names for you.

thanks
-- PMM

On 08/02/2018 15:54, Peter Maydell wrote:
> On 8 February 2018 at 13:38, Marcel Apfelbaum <marcel@redhat.com> wrote:
>> Hi Peter,
>>
>> On 08/02/2018 14:59, Peter Maydell wrote:
>>> (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.
> 
> If we're using kernel header files, I would recommend using
> the scripts/update-headers machinery for this, the way we do
> for other kernel headers. Among other things, the cp_portable
> function in that script will fix up the type names for you.
> 

I will use the mechanism.
Thanks,
Marcel

> thanks
> -- PMM
> 

On Thu, Feb 08, 2018 at 03:38:59PM +0200, Marcel Apfelbaum wrote:
> 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.

I don't think this is done like this anywhere in code.
Please take a look at how it's done for e.g. input.h

I think that whenever a rule in HACKING, coding style,
etc is ignored it should be accompanied by motivation
in code comments, so this won't spread out any more.

-- 
MST

On 08/02/2018 15:58, Michael S. Tsirkin wrote:
> On Thu, Feb 08, 2018 at 03:38:59PM +0200, Marcel Apfelbaum wrote:
>> 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.
> 
> I don't think this is done like this anywhere in code.

It is used in vmxnet.

> Please take a look at how it's done for e.g. input.h
> 

I will use it as reference.

> I think that whenever a rule in HACKING, coding style,
> etc is ignored it should be accompanied by motivation
> in code comments, so this won't spread out any more.
> 

Agreed, I will update the code accordingly.
Thanks,
Marcel

Hi Marcel,

On 02/08/2018 10:38 AM, Marcel Apfelbaum wrote:
> On 08/02/2018 14:59, Peter Maydell wrote:
>> On 5 February 2018 at 10:26, Marcel Apfelbaum <marcel@redhat.com> wrote:
>>> The following changes since commit f24ee107a07f093bd7ed475dd48d7ba57ea3d8fe:
>>
>> 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.

I spent some time to review this but got lost when it became too
specific (this is not really my area).
I was hoping some of the VMware folks could review this.

KVM related stuffs are hard to test, but we have some qtests (migration
mostly). Adding some tests for this huge code addition would be really
great.

Regards,

Phil.

On 08/02/2018 16:01, Philippe Mathieu-Daudé wrote:
> Hi Marcel,
> 
> On 02/08/2018 10:38 AM, Marcel Apfelbaum wrote:
>> On 08/02/2018 14:59, Peter Maydell wrote:
>>> On 5 February 2018 at 10:26, Marcel Apfelbaum <marcel@redhat.com> wrote:
>>>> The following changes since commit f24ee107a07f093bd7ed475dd48d7ba57ea3d8fe:
>>>
>>> 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.
> 
> I spent some time to review this but got lost when it became too
> specific (this is not really my area).
> I was hoping some of the VMware folks could review this.
> 

The code was reviewed by someone from Mellanox and
we have an RDMA developer from Oracle looking into it to.
For V10 we will have a second RB for the device, it should be enough.

> KVM related stuffs are hard to test, but we have some qtests (migration
> mostly). Adding some tests for this huge code addition would be really
> great.
> 

It is in our plans, yes.
We saw the avocado guys are returning to QEMU, I will ask their help in setting
up an avocado test. It should take some time and we are developing the device
over an year offline making it had to maintain/review, so
we will work on the tests in parallel with the device submission.

Thanks,
Marcel

> Regards,
> 
> Phil.
> 

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