Re: [Qemu-devel] [PATCH 01/10] Rename RDMA structures to make destination clear

["Michael R. Hines" <mrhines@linux.vnet.ibm.com>]

On 04/20/2015 10:57 AM, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> RDMA has two data types that are named confusingly;
>     RDMALocalBlock (pointed to indirectly by local_ram_blocks)
>     RDMARemoteBlock (pointed to by block in RDMAContext)
>
> RDMALocalBlocks, as the name suggests is a data strucuture that
> represents the RDMAable RAM Blocks on the current side of the migration
> whichever that is.
>
> RDMARemoteBlocks is always the shape of the RAMBlocks on the
> destination, even on the destination.
>
> Rename:
>       RDMARemoteBlock -> RDMADestBlock
>       context->'block' -> context->dest_blocks
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
>   migration/rdma.c | 66 ++++++++++++++++++++++++++++----------------------------
>   1 file changed, 33 insertions(+), 33 deletions(-)
>
> diff --git a/migration/rdma.c b/migration/rdma.c
> index 77e3444..089adcf 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -236,13 +236,13 @@ typedef struct RDMALocalBlock {
>    * corresponding RDMALocalBlock with
>    * the information needed to perform the actual RDMA.
>    */
> -typedef struct QEMU_PACKED RDMARemoteBlock {
> +typedef struct QEMU_PACKED RDMADestBlock {
>       uint64_t remote_host_addr;
>       uint64_t offset;
>       uint64_t length;
>       uint32_t remote_rkey;
>       uint32_t padding;
> -} RDMARemoteBlock;
> +} RDMADestBlock;
>
>   static uint64_t htonll(uint64_t v)
>   {
> @@ -258,20 +258,20 @@ static uint64_t ntohll(uint64_t v) {
>       return ((uint64_t)ntohl(u.lv[0]) << 32) | (uint64_t) ntohl(u.lv[1]);
>   }
>
> -static void remote_block_to_network(RDMARemoteBlock *rb)
> +static void dest_block_to_network(RDMADestBlock *db)
>   {
> -    rb->remote_host_addr = htonll(rb->remote_host_addr);
> -    rb->offset = htonll(rb->offset);
> -    rb->length = htonll(rb->length);
> -    rb->remote_rkey = htonl(rb->remote_rkey);
> +    db->remote_host_addr = htonll(db->remote_host_addr);
> +    db->offset = htonll(db->offset);
> +    db->length = htonll(db->length);
> +    db->remote_rkey = htonl(db->remote_rkey);
>   }
>
> -static void network_to_remote_block(RDMARemoteBlock *rb)
> +static void network_to_dest_block(RDMADestBlock *db)
>   {
> -    rb->remote_host_addr = ntohll(rb->remote_host_addr);
> -    rb->offset = ntohll(rb->offset);
> -    rb->length = ntohll(rb->length);
> -    rb->remote_rkey = ntohl(rb->remote_rkey);
> +    db->remote_host_addr = ntohll(db->remote_host_addr);
> +    db->offset = ntohll(db->offset);
> +    db->length = ntohll(db->length);
> +    db->remote_rkey = ntohl(db->remote_rkey);
>   }
>
>   /*
> @@ -350,7 +350,7 @@ typedef struct RDMAContext {
>        * Description of ram blocks used throughout the code.
>        */
>       RDMALocalBlocks local_ram_blocks;
> -    RDMARemoteBlock *block;
> +    RDMADestBlock  *dest_blocks;
>
>       /*
>        * Migration on *destination* started.
> @@ -590,7 +590,7 @@ static int qemu_rdma_init_ram_blocks(RDMAContext *rdma)
>       memset(local, 0, sizeof *local);
>       qemu_ram_foreach_block(qemu_rdma_init_one_block, rdma);
>       trace_qemu_rdma_init_ram_blocks(local->nb_blocks);
> -    rdma->block = (RDMARemoteBlock *) g_malloc0(sizeof(RDMARemoteBlock) *
> +    rdma->dest_blocks = (RDMADestBlock *) g_malloc0(sizeof(RDMADestBlock) *
>                           rdma->local_ram_blocks.nb_blocks);
>       local->init = true;
>       return 0;
> @@ -2177,8 +2177,8 @@ static void qemu_rdma_cleanup(RDMAContext *rdma)
>           rdma->connected = false;
>       }
>
> -    g_free(rdma->block);
> -    rdma->block = NULL;
> +    g_free(rdma->dest_blocks);
> +    rdma->dest_blocks = NULL;
>
>       for (idx = 0; idx < RDMA_WRID_MAX; idx++) {
>           if (rdma->wr_data[idx].control_mr) {
> @@ -2967,25 +2967,25 @@ static int qemu_rdma_registration_handle(QEMUFile *f, void *opaque,
>                * their "local" descriptions with what was sent.
>                */
>               for (i = 0; i < local->nb_blocks; i++) {
> -                rdma->block[i].remote_host_addr =
> +                rdma->dest_blocks[i].remote_host_addr =
>                       (uintptr_t)(local->block[i].local_host_addr);
>
>                   if (rdma->pin_all) {
> -                    rdma->block[i].remote_rkey = local->block[i].mr->rkey;
> +                    rdma->dest_blocks[i].remote_rkey = local->block[i].mr->rkey;
>                   }
>
> -                rdma->block[i].offset = local->block[i].offset;
> -                rdma->block[i].length = local->block[i].length;
> +                rdma->dest_blocks[i].offset = local->block[i].offset;
> +                rdma->dest_blocks[i].length = local->block[i].length;
>
> -                remote_block_to_network(&rdma->block[i]);
> +                dest_block_to_network(&rdma->dest_blocks[i]);
>               }
>
>               blocks.len = rdma->local_ram_blocks.nb_blocks
> -                                                * sizeof(RDMARemoteBlock);
> +                                                * sizeof(RDMADestBlock);
>
>
>               ret = qemu_rdma_post_send_control(rdma,
> -                                        (uint8_t *) rdma->block, &blocks);
> +                                        (uint8_t *) rdma->dest_blocks, &blocks);
>
>               if (ret < 0) {
>                   error_report("rdma migration: error sending remote info");
> @@ -3141,7 +3141,7 @@ static int qemu_rdma_registration_stop(QEMUFile *f, void *opaque,
>       if (flags == RAM_CONTROL_SETUP) {
>           RDMAControlHeader resp = {.type = RDMA_CONTROL_RAM_BLOCKS_RESULT };
>           RDMALocalBlocks *local = &rdma->local_ram_blocks;
> -        int reg_result_idx, i, j, nb_remote_blocks;
> +        int reg_result_idx, i, j, nb_dest_blocks;
>
>           head.type = RDMA_CONTROL_RAM_BLOCKS_REQUEST;
>           trace_qemu_rdma_registration_stop_ram();
> @@ -3162,7 +3162,7 @@ static int qemu_rdma_registration_stop(QEMUFile *f, void *opaque,
>               return ret;
>           }
>
> -        nb_remote_blocks = resp.len / sizeof(RDMARemoteBlock);
> +        nb_dest_blocks = resp.len / sizeof(RDMADestBlock);
>
>           /*
>            * The protocol uses two different sets of rkeys (mutually exclusive):
> @@ -3176,7 +3176,7 @@ static int qemu_rdma_registration_stop(QEMUFile *f, void *opaque,
>            * and then propagates the remote ram block descriptions to his local copy.
>            */
>
> -        if (local->nb_blocks != nb_remote_blocks) {
> +        if (local->nb_blocks != nb_dest_blocks) {
>               ERROR(errp, "ram blocks mismatch #1! "
>                           "Your QEMU command line parameters are probably "
>                           "not identical on both the source and destination.");
> @@ -3184,26 +3184,26 @@ static int qemu_rdma_registration_stop(QEMUFile *f, void *opaque,
>           }
>
>           qemu_rdma_move_header(rdma, reg_result_idx, &resp);
> -        memcpy(rdma->block,
> +        memcpy(rdma->dest_blocks,
>               rdma->wr_data[reg_result_idx].control_curr, resp.len);
> -        for (i = 0; i < nb_remote_blocks; i++) {
> -            network_to_remote_block(&rdma->block[i]);
> +        for (i = 0; i < nb_dest_blocks; i++) {
> +            network_to_dest_block(&rdma->dest_blocks[i]);
>
>               /* search local ram blocks */
>               for (j = 0; j < local->nb_blocks; j++) {
> -                if (rdma->block[i].offset != local->block[j].offset) {
> +                if (rdma->dest_blocks[i].offset != local->block[j].offset) {
>                       continue;
>                   }
>
> -                if (rdma->block[i].length != local->block[j].length) {
> +                if (rdma->dest_blocks[i].length != local->block[j].length) {
>                       ERROR(errp, "ram blocks mismatch #2! "
>                           "Your QEMU command line parameters are probably "
>                           "not identical on both the source and destination.");
>                       return -EINVAL;
>                   }
>                   local->block[j].remote_host_addr =
> -                        rdma->block[i].remote_host_addr;
> -                local->block[j].remote_rkey = rdma->block[i].remote_rkey;
> +                        rdma->dest_blocks[i].remote_host_addr;
> +                local->block[j].remote_rkey = rdma->dest_blocks[i].remote_rkey;
>                   break;
>               }
>

Good to get these renamed, thanks.

Reviewed-by: Michael R. Hines <mrhines@us.ibm.com>