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

[Dr. David Alan Gilbert]

* Stefan Weil (address@hidden) wrote:
> Fix type casts between pointers and 64 bit integers.
> Now 32 bit builds are possible again.
> 
> Signed-off-by: Stefan Weil <address@hidden>
> ---
>  migration/rdma.c |   57 
> +++++++++++++++++++++++++++++-------------------------
>  1 file changed, 31 insertions(+), 26 deletions(-)
> 
> diff --git a/migration/rdma.c b/migration/rdma.c
> index 1512460..a68f1f1 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -493,8 +493,8 @@ static inline uint64_t ram_chunk_index(const uint8_t 
> *start,
>  static inline uint8_t *ram_chunk_start(const RDMALocalBlock *rdma_ram_block,
>                                         uint64_t i)
>  {
> -    return (uint8_t *) (((uintptr_t) rdma_ram_block->local_host_addr)
> -                                    + (i << RDMA_REG_CHUNK_SHIFT));
> +    return (uint8_t *)(uintptr_t)(rdma_ram_block->local_host_addr +
> +                                  (i << RDMA_REG_CHUNK_SHIFT));
>  }
>  
>  static inline uint8_t *ram_chunk_end(const RDMALocalBlock *rdma_ram_block,
> @@ -515,7 +515,7 @@ static int __qemu_rdma_add_block(RDMAContext *rdma, void 
> *host_addr,
>  {

This will clash with my 'unbreak dtrace tracing due to double _ in rdma names' 
that's
going through Stefan's tree.

>      RDMALocalBlocks *local = &rdma->local_ram_blocks;
>      RDMALocalBlock *block = g_hash_table_lookup(rdma->blockmap,
> -        (void *) block_offset);
> +        (void *)(uintptr_t)block_offset);
>      RDMALocalBlock *old = local->block;


>  
>      assert(block == NULL);
> @@ -526,9 +526,11 @@ static int __qemu_rdma_add_block(RDMAContext *rdma, void 
> *host_addr,
>          int x;
>  
>          for (x = 0; x < local->nb_blocks; x++) {
> -            g_hash_table_remove(rdma->blockmap, (void *)old[x].offset);
> -            g_hash_table_insert(rdma->blockmap, (void *)old[x].offset,
> -                                                &local->block[x]);
> +            g_hash_table_remove(rdma->blockmap,
> +                                (void *)(uintptr_t)old[x].offset);
> +            g_hash_table_insert(rdma->blockmap,
> +                                (void *)(uintptr_t)old[x].offset,
> +                                &local->block[x]);
>          }
>          memcpy(local->block, old, sizeof(RDMALocalBlock) * local->nb_blocks);
>          g_free(old);
> @@ -549,12 +551,12 @@ static int __qemu_rdma_add_block(RDMAContext *rdma, 
> void *host_addr,
>  
>      block->is_ram_block = local->init ? false : true;
>  
> -    g_hash_table_insert(rdma->blockmap, (void *) block_offset, block);
> +    g_hash_table_insert(rdma->blockmap, (void *)(uintptr_t)block_offset, 
> block);
>  
>      trace___qemu_rdma_add_block(local->nb_blocks,
> -                           (uint64_t) block->local_host_addr, block->offset,
> +                           (uintptr_t)block->local_host_addr, block->offset,
>                             block->length,
> -                           (uint64_t) (block->local_host_addr + 
> block->length),
> +                           (uintptr_t)(block->local_host_addr + 
> block->length),
>                             BITS_TO_LONGS(block->nb_chunks) *
>                                 sizeof(unsigned long) * 8,
>                             block->nb_chunks);
> @@ -595,7 +597,7 @@ static int qemu_rdma_init_ram_blocks(RDMAContext *rdma)
>      return 0;
>  }
>  
> -static int __qemu_rdma_delete_block(RDMAContext *rdma, ram_addr_t 
> block_offset)
> +static int __qemu_rdma_delete_block(RDMAContext *rdma, uintptr_t 
> block_offset)
>  {
>      RDMALocalBlocks *local = &rdma->local_ram_blocks;
>      RDMALocalBlock *block = g_hash_table_lookup(rdma->blockmap,
> @@ -635,7 +637,7 @@ static int __qemu_rdma_delete_block(RDMAContext *rdma, 
> ram_addr_t block_offset)
>      block->remote_keys = NULL;
>  
>      for (x = 0; x < local->nb_blocks; x++) {
> -        g_hash_table_remove(rdma->blockmap, (void *)old[x].offset);
> +        g_hash_table_remove(rdma->blockmap, (void 
> *)(uintptr_t)old[x].offset);
>      }
>  
>      if (local->nb_blocks > 1) {
> @@ -658,9 +660,9 @@ static int __qemu_rdma_delete_block(RDMAContext *rdma, 
> ram_addr_t block_offset)
>      }
>  
>      trace___qemu_rdma_delete_block(local->nb_blocks,
> -                           (uint64_t)block->local_host_addr,
> +                           (uintptr_t)block->local_host_addr,
>                             block->offset, block->length,
> -                           (uint64_t)(block->local_host_addr + 
> block->length),
> +                           (uintptr_t)(block->local_host_addr + 
> block->length),
>                             BITS_TO_LONGS(block->nb_chunks) *
>                                 sizeof(unsigned long) * 8, block->nb_chunks);
>  
> @@ -670,8 +672,9 @@ static int __qemu_rdma_delete_block(RDMAContext *rdma, 
> ram_addr_t block_offset)
>  
>      if (local->nb_blocks) {
>          for (x = 0; x < local->nb_blocks; x++) {
> -            g_hash_table_insert(rdma->blockmap, (void 
> *)local->block[x].offset,
> -                                                &local->block[x]);
> +            g_hash_table_insert(rdma->blockmap,
> +                                (void *)(uintptr_t)local->block[x].offset,
> +                                &local->block[x]);
>          }
>      }
>  
> @@ -1076,7 +1079,7 @@ static int qemu_rdma_reg_whole_ram_blocks(RDMAContext 
> *rdma)
>   * This search cannot fail or the migration will fail.
>   */
>  static int qemu_rdma_search_ram_block(RDMAContext *rdma,
> -                                      uint64_t block_offset,
> +                                      uintptr_t block_offset,
>                                        uint64_t offset,
>                                        uint64_t length,
>                                        uint64_t *block_index,
> @@ -1380,8 +1383,8 @@ static uint64_t qemu_rdma_poll(RDMAContext *rdma, 
> uint64_t *wr_id_out,
>          RDMALocalBlock *block = &(rdma->local_ram_blocks.block[index]);
>  
>          trace_qemu_rdma_poll_write(print_wrid(wr_id), wr_id, rdma->nb_sent,
> -                 index, chunk,
> -                 block->local_host_addr, (void *)block->remote_host_addr);
> +                                   index, chunk, block->local_host_addr,
> +                                   (void 
> *)(uintptr_t)block->remote_host_addr);

I think that's the wrong fix there; remote_host_addr is actually 64bit because
even if this host is 32bit the other end might be 64bit; I think the right
fix is to change the trace to use a uint64_t for the remote_host_addr.

>          clear_bit(chunk, block->transit_bitmap);
>  
> @@ -1524,7 +1527,7 @@ static int qemu_rdma_post_send_control(RDMAContext 
> *rdma, uint8_t *buf,
>      RDMAWorkRequestData *wr = &rdma->wr_data[RDMA_WRID_CONTROL];
>      struct ibv_send_wr *bad_wr;
>      struct ibv_sge sge = {
> -                           .addr = (uint64_t)(wr->control),
> +                           .addr = (uintptr_t)(wr->control),

No, as before, my belief is that the .addr is a uint64_t.

(Although curiously I did a test and found that gcc lets me pass a uint64_t
to a function that has a uintptr_t parameter on a 32bit machine, but I don't
understand why.)

Dave

>                             .length = head->len + sizeof(RDMAControlHeader),
>                             .lkey = wr->control_mr->lkey,
>                           };
> @@ -1578,7 +1581,7 @@ static int qemu_rdma_post_recv_control(RDMAContext 
> *rdma, int idx)
>  {
>      struct ibv_recv_wr *bad_wr;
>      struct ibv_sge sge = {
> -                            .addr = (uint64_t)(rdma->wr_data[idx].control),
> +                            .addr = (uintptr_t)(rdma->wr_data[idx].control),
>                              .length = RDMA_CONTROL_MAX_BUFFER,
>                              .lkey = rdma->wr_data[idx].control_mr->lkey,
>                           };
> @@ -1825,11 +1828,12 @@ static int qemu_rdma_write_one(QEMUFile *f, 
> RDMAContext *rdma,
>                               };
>  
>  retry:
> -    sge.addr = (uint64_t)(block->local_host_addr +
> +    sge.addr = (uintptr_t)(block->local_host_addr +
>                              (current_addr - block->offset));
>      sge.length = length;
>  
> -    chunk = ram_chunk_index(block->local_host_addr, (uint8_t *) sge.addr);
> +    chunk = ram_chunk_index(block->local_host_addr,
> +                            (uint8_t *)(uintptr_t)sge.addr);
>      chunk_start = ram_chunk_start(block, chunk);
>  
>      if (block->is_ram_block) {
> @@ -1882,8 +1886,9 @@ retry:
>               * memset() + madvise() the entire chunk without RDMA.
>               */
>  
> -            if (can_use_buffer_find_nonzero_offset((void *)sge.addr, length)
> -                   && buffer_find_nonzero_offset((void *)sge.addr,
> +            if (can_use_buffer_find_nonzero_offset((void 
> *)(uintptr_t)sge.addr,
> +                                                   length)
> +                   && buffer_find_nonzero_offset((void *)(uintptr_t)sge.addr,
>                                                      length) == length) {
>                  RDMACompress comp = {
>                                          .offset = current_addr,
> @@ -2978,7 +2983,7 @@ static int qemu_rdma_registration_handle(QEMUFile *f, 
> void *opaque,
>               */
>              for (i = 0; i < local->nb_blocks; i++) {
>                  rdma->block[i].remote_host_addr =
> -                    (uint64_t)(local->block[i].local_host_addr);
> +                    (uintptr_t)(local->block[i].local_host_addr);
>  
>                  if (rdma->pin_all) {
>                      rdma->block[i].remote_rkey = local->block[i].mr->rkey;
> @@ -3042,7 +3047,7 @@ static int qemu_rdma_registration_handle(QEMUFile *f, 
> void *opaque,
>                      goto out;
>                  }
>  
> -                reg_result->host_addr = (uint64_t) block->local_host_addr;
> +                reg_result->host_addr = (uintptr_t)block->local_host_addr;
>  
>                  trace_qemu_rdma_registration_handle_register_rkey(
>                                                             reg_result->rkey);
> -- 
> 1.7.10.4
> 
> 
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK