From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52013) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YumG1-0004rO-JU for qemu-devel@nongnu.org; Tue, 19 May 2015 14:28:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YumFy-0002KU-4p for qemu-devel@nongnu.org; Tue, 19 May 2015 14:28:49 -0400 Received: from e17.ny.us.ibm.com ([129.33.205.207]:42209) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YumFx-0002Jo-VS for qemu-devel@nongnu.org; Tue, 19 May 2015 14:28:46 -0400 Received: from /spool/local by e17.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 19 May 2015 14:28:44 -0400 Received: from b01cxnp22035.gho.pok.ibm.com (b01cxnp22035.gho.pok.ibm.com [9.57.198.25]) by d01dlp03.pok.ibm.com (Postfix) with ESMTP id 2DA12C90041 for ; Tue, 19 May 2015 14:19:46 -0400 (EDT) Received: from d01av04.pok.ibm.com (d01av04.pok.ibm.com [9.56.224.64]) by b01cxnp22035.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id t4JISboR49217700 for ; Tue, 19 May 2015 18:28:37 GMT Received: from d01av04.pok.ibm.com (localhost [127.0.0.1]) by d01av04.pok.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id t4JISbCQ006732 for ; Tue, 19 May 2015 14:28:37 -0400 Message-ID: <555B80DE.5030403@linux.vnet.ibm.com> Date: Tue, 19 May 2015 13:28:46 -0500 From: "Michael R. Hines" MIME-Version: 1.0 References: <1429545445-28216-1-git-send-email-dgilbert@redhat.com> <1429545445-28216-5-git-send-email-dgilbert@redhat.com> In-Reply-To: <1429545445-28216-5-git-send-email-dgilbert@redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 04/10] Translate offsets to destination address space List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Dr. David Alan Gilbert (git)" , qemu-devel@nongnu.org Cc: amit.shah@redhat.com, arei.gonglei@huawei.com, mrhines@us.ibm.com, quintela@redhat.com On 04/20/2015 10:57 AM, Dr. David Alan Gilbert (git) wrote: > From: "Dr. David Alan Gilbert" > > The 'offset' field in RDMACompress and 'current_addr' field > in RDMARegister are commented as being offsets within a particular > RAMBlock, however they appear to actually be offsets within the > ram_addr_t space. > > The code currently assumes that the offsets on the source/destination > match, this change removes the need for the assumption for these > structures by translating the addresses into the ram_addr_t space of > the destination host. I don't understand fully: If the offsets are not the same, then why would the RAMBlocks be the same? If a RAMBlock is hot-plugged on one side, shouldn't an identical one be hotplugged on the other side, including the offset into ram_addr_t? (Even if the base address of the ram_addr_t space is different between source and destination, then at least the offsets and list of blocks should be the same, no?) Is hotplugging an asynchronous operation for post-copy or something? > > Note: An alternative would be to change the fields to actually > take the data they're commented for; this would potentially be > simpler but would break stream compatibility for those cases > that currently work. > > Signed-off-by: Dr. David Alan Gilbert > --- > migration/rdma.c | 31 ++++++++++++++++++++++++------- > 1 file changed, 24 insertions(+), 7 deletions(-) > > diff --git a/migration/rdma.c b/migration/rdma.c > index c3814c5..2c0d11b 100644 > --- a/migration/rdma.c > +++ b/migration/rdma.c > @@ -411,7 +411,7 @@ static void network_to_control(RDMAControlHeader *control) > */ > typedef struct QEMU_PACKED { > union QEMU_PACKED { > - uint64_t current_addr; /* offset into the ramblock of the chunk */ > + uint64_t current_addr; /* offset into the ram_addr_t space */ > uint64_t chunk; /* chunk to lookup if unregistering */ > } key; > uint32_t current_index; /* which ramblock the chunk belongs to */ > @@ -419,8 +419,19 @@ typedef struct QEMU_PACKED { > uint64_t chunks; /* how many sequential chunks to register */ > } RDMARegister; The below seems OK, but I would prefer not to do this translation here. Can the source and destination apply the offset calculations outside of the byte-order functions? Like, before register_to_network, the source removes the offset, and then when the message is received, the destination then again re-applies the correct offset? > -static void register_to_network(RDMARegister *reg) > +static void register_to_network(RDMAContext *rdma, RDMARegister *reg) > { > + RDMALocalBlock *local_block; > + local_block = &rdma->local_ram_blocks.block[reg->current_index]; > + > + if (local_block->is_ram_block) { > + /* > + * current_addr as passed in is an address in the local ram_addr_t > + * space, we need to translate this for the destination > + */ > + reg->key.current_addr -= local_block->offset; > + reg->key.current_addr += rdma->dest_blocks[reg->current_index].offset; > + } > reg->key.current_addr = htonll(reg->key.current_addr); > reg->current_index = htonl(reg->current_index); > reg->chunks = htonll(reg->chunks);