rdma: Fix 32-bit compilation

[Qemu-devel] [PATCH for-2.12] rdma: Fix 32-bit compilation

Posted by Eric Blake 7 years, 6 months ago

Use the correct printf formats, so that a 32-bit compile doesn't
spit out lots of warnings about %lx being incompatible with uint64_t.
Broken since initial commit ef6d4ccd.

Signed-off-by: Eric Blake <eblake@redhat.com>
---

I don't know if 'make vm-build-ubuntu.i368' would catch this (it failed
for me for other reasons); I found it via a 32-bit rawhide VM.

 hw/rdma/rdma_backend.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/rdma/rdma_backend.c b/hw/rdma/rdma_backend.c
index e306fba5344..89020fdcf62 100644
--- a/hw/rdma/rdma_backend.c
+++ b/hw/rdma/rdma_backend.c
@@ -656,7 +656,8 @@ void rdma_backend_destroy_qp(RdmaBackendQP *qp)
 #define CHK_ATTR(req, dev, member, fmt) ({ \
     pr_dbg("%s="fmt","fmt"\n", #member, dev.member, req->member); \
     if (req->member > dev.member) { \
-        warn_report("%s = 0x%lx is higher than host device capability 0x%lx", \
+        warn_report("%s = 0x%" PRIx64 " is higher than host device " \
+                    "capability 0x%" PRIx64, \
                     #member, (uint64_t)req->member, (uint64_t)dev.member); \
         req->member = dev.member; \
     } \
-- 
2.14.3

Re: [Qemu-devel] [PATCH for-2.12] rdma: Fix 32-bit compilation

Posted by Eric Blake 7 years, 6 months ago

On 03/19/2018 04:53 PM, Eric Blake wrote:
> Use the correct printf formats, so that a 32-bit compile doesn't
> spit out lots of warnings about %lx being incompatible with uint64_t.
> Broken since initial commit ef6d4ccd.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
> 
> I don't know if 'make vm-build-ubuntu.i368' would catch this (it failed
> for me for other reasons); I found it via a 32-bit rawhide VM.

That fixes most of the warnings, but not:

/home/dummy/qemu/hw/rdma/rdma_backend.c: In function 
'rdma_backend_create_mr':
/home/dummy/qemu/hw/rdma/rdma_backend.c:409:37: error: cast to pointer 
from integer of different size [-Werror=int-to-pointer-cast]
      m4->ibmr = ibv_reg_m4(pd->ibpd, (void *)addr, length, access);
                                      ^

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH for-2.12] rdma: Fix 32-bit compilation

Posted by Marcel Apfelbaum 7 years, 6 months ago

On 20/03/2018 0:08, Eric Blake wrote:
> On 03/19/2018 04:53 PM, Eric Blake wrote:
>> Use the correct printf formats, so that a 32-bit compile doesn't
>> spit out lots of warnings about %lx being incompatible with uint64_t.
>> Broken since initial commit ef6d4ccd.
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>> ---
>>
>> I don't know if 'make vm-build-ubuntu.i368' would catch this (it failed
>> for me for other reasons); I found it via a 32-bit rawhide VM.
> 
> That fixes most of the warnings,

Thanks!

> but not:
> 
> /home/dummy/qemu/hw/rdma/rdma_backend.c: In function 'rdma_backend_create_mr':
> /home/dummy/qemu/hw/rdma/rdma_backend.c:409:37: error: cast to pointer from integer of different size
> [-Werror=int-to-pointer-cast]
>      m4->ibmr = ibv_reg_m4(pd->ibpd, (void *)addr, length, access);
>                                      ^
> 

So the compilation actually brakes on 32bit machines?
I'll create a VM.

Thanks,
Marcel

Re: [Qemu-devel] [PATCH for-2.12] rdma: Fix 32-bit compilation

Posted by Marcel Apfelbaum 7 years, 6 months ago

Hi Eric,

On 19/03/2018 23:53, Eric Blake wrote:
> Use the correct printf formats, so that a 32-bit compile doesn't
> spit out lots of warnings about %lx being incompatible with uint64_t.
> Broken since initial commit ef6d4ccd.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
> 
> I don't know if 'make vm-build-ubuntu.i368' would catch this (it failed
> for me for other reasons); I found it via a 32-bit rawhide VM.
> 

I couldn't run 'make vm-build-ubuntu.i368' either. (Stuck on "Booting from Hard Disk...")

I run make docker-test-build@debian-win32-cross to be sure it compiles on 32bit arch,
however I found out the docker configuration results in 'RDMA support no', so it will not help.

Fam, is there any way the docker image can be updated with the RDMA libraries
so we can check this too?

>  hw/rdma/rdma_backend.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/rdma/rdma_backend.c b/hw/rdma/rdma_backend.c
> index e306fba5344..89020fdcf62 100644
> --- a/hw/rdma/rdma_backend.c
> +++ b/hw/rdma/rdma_backend.c
> @@ -656,7 +656,8 @@ void rdma_backend_destroy_qp(RdmaBackendQP *qp)
>  #define CHK_ATTR(req, dev, member, fmt) ({ \
>      pr_dbg("%s="fmt","fmt"\n", #member, dev.member, req->member); \
>      if (req->member > dev.member) { \
> -        warn_report("%s = 0x%lx is higher than host device capability 0x%lx", \
> +        warn_report("%s = 0x%" PRIx64 " is higher than host device " \
> +                    "capability 0x%" PRIx64, \
>                      #member, (uint64_t)req->member, (uint64_t)dev.member); \
>          req->member = dev.member; \
>      } \
> 

Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>


Thanks,
Marcel

Re: [Qemu-devel] [PATCH for-2.12] rdma: Fix 32-bit compilation

Posted by Fam Zheng 7 years, 6 months ago

On Tue, 03/20 12:30, Marcel Apfelbaum wrote:
> Hi Eric,
> 
> On 19/03/2018 23:53, Eric Blake wrote:
> > Use the correct printf formats, so that a 32-bit compile doesn't
> > spit out lots of warnings about %lx being incompatible with uint64_t.
> > Broken since initial commit ef6d4ccd.
> > 
> > Signed-off-by: Eric Blake <eblake@redhat.com>
> > ---
> > 
> > I don't know if 'make vm-build-ubuntu.i368' would catch this (it failed
> > for me for other reasons); I found it via a 32-bit rawhide VM.
> > 
> 
> I couldn't run 'make vm-build-ubuntu.i368' either. (Stuck on "Booting from Hard Disk...")
> 
> I run make docker-test-build@debian-win32-cross to be sure it compiles on 32bit arch,
> however I found out the docker configuration results in 'RDMA support no', so it will not help.

Cc'ing Phil and Alex who know this image better than me. Meanwhile I'll take a
look at why make vm-build-ubuntu.i386 is broken. Thanks.

Fam

Re: [Qemu-devel] [PATCH for-2.12] rdma: Fix 32-bit compilation

Posted by Fam Zheng 7 years, 6 months ago

On Tue, 03/20 12:30, Marcel Apfelbaum wrote:
> Hi Eric,
> 
> On 19/03/2018 23:53, Eric Blake wrote:
> > Use the correct printf formats, so that a 32-bit compile doesn't
> > spit out lots of warnings about %lx being incompatible with uint64_t.
> > Broken since initial commit ef6d4ccd.
> > 
> > Signed-off-by: Eric Blake <eblake@redhat.com>
> > ---
> > 
> > I don't know if 'make vm-build-ubuntu.i368' would catch this (it failed
> > for me for other reasons); I found it via a 32-bit rawhide VM.
> > 
> 
> I couldn't run 'make vm-build-ubuntu.i368' either. (Stuck on "Booting from Hard Disk...")

I tried this now and it works for me. Could you delete "$HOME/.cache/qemu-vm"
and do it again from a git tree, then pastbin the full log?

Fam

Re: [Qemu-devel] [PATCH for-2.12] rdma: Fix 32-bit compilation

Posted by Marcel Apfelbaum 7 years, 6 months ago

On 21/03/2018 4:07, Fam Zheng wrote:
> On Tue, 03/20 12:30, Marcel Apfelbaum wrote:
>> Hi Eric,
>>
>> On 19/03/2018 23:53, Eric Blake wrote:
>>> Use the correct printf formats, so that a 32-bit compile doesn't
>>> spit out lots of warnings about %lx being incompatible with uint64_t.
>>> Broken since initial commit ef6d4ccd.
>>>
>>> Signed-off-by: Eric Blake <eblake@redhat.com>
>>> ---
>>>
>>> I don't know if 'make vm-build-ubuntu.i368' would catch this (it failed
>>> for me for other reasons); I found it via a 32-bit rawhide VM.
>>>
>>
>> I couldn't run 'make vm-build-ubuntu.i368' either. (Stuck on "Booting from Hard Disk...")
> 
> I tried this now and it works for me. Could you delete "$HOME/.cache/qemu-vm"
> and do it again from a git tree, then pastbin the full log?
> 

Hi Fam,
I did that but it didn't help.

Here is the log: https://da.gd/8wJHj
I try to run the same command line, and it still stuck in the boot process.
Problem is, the command line looks fine, may be another issue with my PC...
I'll continue to debug it.

Thanks for the help,
Marcel

> Fam
>

Re: [Qemu-devel] [PATCH for-2.12] rdma: Fix 32-bit compilation

Posted by Fam Zheng 7 years, 6 months ago

On Wed, 03/21 11:20, Marcel Apfelbaum wrote:
> On 21/03/2018 4:07, Fam Zheng wrote:
> > On Tue, 03/20 12:30, Marcel Apfelbaum wrote:
> >> Hi Eric,
> >>
> >> On 19/03/2018 23:53, Eric Blake wrote:
> >>> Use the correct printf formats, so that a 32-bit compile doesn't
> >>> spit out lots of warnings about %lx being incompatible with uint64_t.
> >>> Broken since initial commit ef6d4ccd.
> >>>
> >>> Signed-off-by: Eric Blake <eblake@redhat.com>
> >>> ---
> >>>
> >>> I don't know if 'make vm-build-ubuntu.i368' would catch this (it failed
> >>> for me for other reasons); I found it via a 32-bit rawhide VM.
> >>>
> >>
> >> I couldn't run 'make vm-build-ubuntu.i368' either. (Stuck on "Booting from Hard Disk...")
> > 
> > I tried this now and it works for me. Could you delete "$HOME/.cache/qemu-vm"
> > and do it again from a git tree, then pastbin the full log?
> > 
> 
> Hi Fam,
> I did that but it didn't help.
> 
> Here is the log: https://da.gd/8wJHj
> I try to run the same command line, and it still stuck in the boot process.
> Problem is, the command line looks fine, may be another issue with my PC...
> I'll continue to debug it.

Maybe we are using different QEMU/seabios. What I used is built from qemu.git.
The log has the QEMU arguments which you can try manually for debug.

Fam

Re: [Qemu-devel] [PATCH for-2.12] rdma: Fix 32-bit compilation

Posted by Yuval Shaia 7 years, 6 months ago

On Mon, Mar 19, 2018 at 04:53:35PM -0500, Eric Blake wrote:
> Use the correct printf formats, so that a 32-bit compile doesn't
> spit out lots of warnings about %lx being incompatible with uint64_t.
> Broken since initial commit ef6d4ccd.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
> 
> I don't know if 'make vm-build-ubuntu.i368' would catch this (it failed
> for me for other reasons); I found it via a 32-bit rawhide VM.
> 
>  hw/rdma/rdma_backend.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/rdma/rdma_backend.c b/hw/rdma/rdma_backend.c
> index e306fba5344..89020fdcf62 100644
> --- a/hw/rdma/rdma_backend.c
> +++ b/hw/rdma/rdma_backend.c
> @@ -656,7 +656,8 @@ void rdma_backend_destroy_qp(RdmaBackendQP *qp)
>  #define CHK_ATTR(req, dev, member, fmt) ({ \
>      pr_dbg("%s="fmt","fmt"\n", #member, dev.member, req->member); \
>      if (req->member > dev.member) { \
> -        warn_report("%s = 0x%lx is higher than host device capability 0x%lx", \
> +        warn_report("%s = 0x%" PRIx64 " is higher than host device " \
> +                    "capability 0x%" PRIx64, \
>                      #member, (uint64_t)req->member, (uint64_t)dev.member); \

Hmmm, interesting.
I wonder why in first place to cast all members to uint64_t.
Can you try the method that is used above in the call to pr_dbg, i.e. to
use the given argument fmt?

Something like this:
	warn_report("%s = "fmt" is higher than host device capability "fmt, \
	            #member, req->member, dev.member); \

>          req->member = dev.member; \
>      } \
> -- 
> 2.14.3
>

Re: [Qemu-devel] [PATCH for-2.12] rdma: Fix 32-bit compilation

Posted by Marcel Apfelbaum 7 years, 6 months ago

On 20/03/2018 12:43, Yuval Shaia wrote:
> On Mon, Mar 19, 2018 at 04:53:35PM -0500, Eric Blake wrote:
>> Use the correct printf formats, so that a 32-bit compile doesn't
>> spit out lots of warnings about %lx being incompatible with uint64_t.
>> Broken since initial commit ef6d4ccd.
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>> ---
>>
>> I don't know if 'make vm-build-ubuntu.i368' would catch this (it failed
>> for me for other reasons); I found it via a 32-bit rawhide VM.
>>
>>  hw/rdma/rdma_backend.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/rdma/rdma_backend.c b/hw/rdma/rdma_backend.c
>> index e306fba5344..89020fdcf62 100644
>> --- a/hw/rdma/rdma_backend.c
>> +++ b/hw/rdma/rdma_backend.c
>> @@ -656,7 +656,8 @@ void rdma_backend_destroy_qp(RdmaBackendQP *qp)
>>  #define CHK_ATTR(req, dev, member, fmt) ({ \
>>      pr_dbg("%s="fmt","fmt"\n", #member, dev.member, req->member); \
>>      if (req->member > dev.member) { \
>> -        warn_report("%s = 0x%lx is higher than host device capability 0x%lx", \
>> +        warn_report("%s = 0x%" PRIx64 " is higher than host device " \
>> +                    "capability 0x%" PRIx64, \
>>                      #member, (uint64_t)req->member, (uint64_t)dev.member); \
> 
> Hmmm, interesting.
> I wonder why in first place to cast all members to uint64_t.
> Can you try the method that is used above in the call to pr_dbg, i.e. to
> use the given argument fmt?
> > Something like this:
> 	warn_report("%s = "fmt" is higher than host device capability "fmt, \
> 	            #member, req->member, dev.member); \
> 
>>          req->member = dev.member; \
>>      } \


Right, it seems to be a better solution.

Eric/Yuval, will you send a patch for it?

Thanks!
Marcel


>> -- 
>> 2.14.3
>>