Re: [Qemu-devel] [PATCH 09/10] Sort destination RAMBlocks to be the same as the source

[Dr. David Alan Gilbert]

* Michael R. Hines (address@hidden) wrote:
> On 04/20/2015 10:57 AM, Dr. David Alan Gilbert (git) wrote:
> >From: "Dr. David Alan Gilbert" <address@hidden>
> >
> >Use the order of incoming RAMBlocks from the source to record
> >an index number; that then allows us to sort the destination
> >local RAMBlock list to match the source.
> >
> >Now that the RAMBlocks are known to be in the same order, this
> >simplifies the RDMA Registration step which previously tried to
> >match RAMBlocks based on offset (which isn't guaranteed to match).
> 
> OK, so, what's the reason for sorting?

I'm worried that the code might already be using the RAMBlock index
assuming they're the same on both sides and not taking care
to look it up in the RAMBlock list sent over from the destination;
sorting them so they are both the same makes it simple and safe.
For example, 'qemu_rdma_write_one' has a 'current_index' which it does:

  RDMALocalBlock *block = &(rdma->local_ram_blocks.block[current_index]);

then that becomes:
                RDMACompress comp = {
                                        .offset = current_addr,
                                        .value = 0,
                                        .block_idx = current_index,
                                        .length = length,
                                    };

and on the destination:

        case RDMA_CONTROL_COMPRESS:
            comp = (RDMACompress *) rdma->wr_data[idx].control_curr;
            network_to_compress(comp);

            trace_qemu_rdma_registration_handle_compress(comp->length,
                                                         comp->block_idx,
                                                         comp->offset);
            block = &(rdma->local_ram_blocks.block[comp->block_idx]);

So I think that 'current_index' value is assumed to be the same
on both sides.

If, I'm right, and that's wrong, then keeping the destination
RAMBlocks in the same order just makes it work and we don't need
to worry how many other places have the same problem.

> If the offset is not gauranteed to match (based on a new patch
> that I assume you have coming), then we need to index into
> the hashtable based on something that does match, such
> as the name you added or some other key.

or just use a matching index that makes life simple.

Dave

> 
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK