Re: [Qemu-trivial] [Qemu-devel] [PATCH 2/3] migration: Fix some 32 bit compiler errors

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Stefan Weil (sw@weilnetz.de) wrote:
> The current code won't compile on 32 bit hosts because there are lots
> of type casts between pointers and 64 bit integers.
> 
> Fix some of them.
> 
> Signed-off-by: Stefan Weil <sw@weilnetz.de>

Please route rdma stuff through migration, not -trivial; it's never
trivial to read this code.

> ---
>  migration/rdma.c |   23 +++++++++++------------
>  1 file changed, 11 insertions(+), 12 deletions(-)
> 
> diff --git a/migration/rdma.c b/migration/rdma.c
> index 67c5701..1512460 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -1104,7 +1104,7 @@ static int qemu_rdma_search_ram_block(RDMAContext *rdma,
>   * to perform the actual RDMA operation.
>   */
>  static int qemu_rdma_register_and_get_keys(RDMAContext *rdma,
> -        RDMALocalBlock *block, uint8_t *host_addr,
> +        RDMALocalBlock *block, uintptr_t host_addr,
>          uint32_t *lkey, uint32_t *rkey, int chunk,
>          uint8_t *chunk_start, uint8_t *chunk_end)

OK, so 'host_addr' seems to only be used in this function to print debug,
so that should be harmless.

>  {
> @@ -1141,11 +1141,12 @@ static int qemu_rdma_register_and_get_keys(RDMAContext *rdma,
>          if (!block->pmr[chunk]) {
>              perror("Failed to register chunk!");
>              fprintf(stderr, "Chunk details: block: %d chunk index %d"
> -                            " start %" PRIu64 " end %" PRIu64 " host %" PRIu64
> -                            " local %" PRIu64 " registrations: %d\n",
> -                            block->index, chunk, (uint64_t) chunk_start,
> -                            (uint64_t) chunk_end, (uint64_t) host_addr,
> -                            (uint64_t) block->local_host_addr,
> +                            " start %" PRIuPTR " end %" PRIuPTR
> +                            " host %" PRIuPTR
> +                            " local %" PRIuPTR " registrations: %d\n",
> +                            block->index, chunk, (uintptr_t)chunk_start,
> +                            (uintptr_t)chunk_end, host_addr,
> +                            (uintptr_t)block->local_host_addr,

OK, although is there any reason not to use %p for most of those?

>                              rdma->total_registrations);
>              return -1;
>          }
> @@ -1932,8 +1933,7 @@ retry:
>              }
>  
>              /* try to overlap this single registration with the one we sent. */
> -            if (qemu_rdma_register_and_get_keys(rdma, block,
> -                                                (uint8_t *) sge.addr,
> +            if (qemu_rdma_register_and_get_keys(rdma, block, sge.addr,
>                                                  &sge.lkey, NULL, chunk,
>                                                  chunk_start, chunk_end)) {

sge.addr comes from /usr/include/infiniband/verbs.h for me:

struct ibv_sge {
        uint64_t                addr;
        uint32_t                length;
        uint32_t                lkey;
};

and that's the same on both 32 bit and 64 bit hosts (Fedora 21).
I'm confused about why this helps you build 32 bit, since that uint64_t gets
passed to your host_addr that's now a unitptr_t that will be 32bit.

Dave

>                  error_report("cannot get lkey");
> @@ -1952,8 +1952,7 @@ retry:
>              block->remote_host_addr = reg_result->host_addr;
>          } else {
>              /* already registered before */
> -            if (qemu_rdma_register_and_get_keys(rdma, block,
> -                                                (uint8_t *)sge.addr,
> +            if (qemu_rdma_register_and_get_keys(rdma, block, sge.addr,
>                                                  &sge.lkey, NULL, chunk,
>                                                  chunk_start, chunk_end)) {
>                  error_report("cannot get lkey!");
> @@ -1965,7 +1964,7 @@ retry:
>      } else {
>          send_wr.wr.rdma.rkey = block->remote_rkey;
>  
> -        if (qemu_rdma_register_and_get_keys(rdma, block, (uint8_t *)sge.addr,
> +        if (qemu_rdma_register_and_get_keys(rdma, block, sge.addr,
>                                                       &sge.lkey, NULL, chunk,
>                                                       chunk_start, chunk_end)) {
>              error_report("cannot get lkey!");
> @@ -3036,7 +3035,7 @@ static int qemu_rdma_registration_handle(QEMUFile *f, void *opaque,
>                  chunk_start = ram_chunk_start(block, chunk);
>                  chunk_end = ram_chunk_end(block, chunk + reg->chunks);
>                  if (qemu_rdma_register_and_get_keys(rdma, block,
> -                            (uint8_t *)host_addr, NULL, &reg_result->rkey,
> +                            (uintptr_t)host_addr, NULL, &reg_result->rkey,
>                              chunk, chunk_start, chunk_end)) {
>                      error_report("cannot get rkey");
>                      ret = -EINVAL;
> -- 
> 1.7.10.4
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK