From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53688) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZEdiy-0004t1-8f for qemu-devel@nongnu.org; Mon, 13 Jul 2015 09:24:52 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZEdiw-0001w1-5W for qemu-devel@nongnu.org; Mon, 13 Jul 2015 09:24:48 -0400 Received: from mx1.redhat.com ([209.132.183.28]:52529) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZEdiv-0001vl-UA for qemu-devel@nongnu.org; Mon, 13 Jul 2015 09:24:46 -0400 From: Juan Quintela In-Reply-To: <1434450415-11339-31-git-send-email-dgilbert@redhat.com> (David Alan Gilbert's message of "Tue, 16 Jun 2015 11:26:43 +0100") References: <1434450415-11339-1-git-send-email-dgilbert@redhat.com> <1434450415-11339-31-git-send-email-dgilbert@redhat.com> Date: Mon, 13 Jul 2015 15:24:43 +0200 Message-ID: <87zj30i410.fsf@neno.neno> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH v7 30/42] Page request: Add MIG_RP_MSG_REQ_PAGES reverse command Reply-To: quintela@redhat.com List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Dr. David Alan Gilbert (git)" Cc: aarcange@redhat.com, yamahata@private.email.ne.jp, liang.z.li@intel.com, qemu-devel@nongnu.org, luis@cs.umu.se, amit.shah@redhat.com, pbonzini@redhat.com, david@gibson.dropbear.id.au "Dr. David Alan Gilbert (git)" wrote: > From: "Dr. David Alan Gilbert" > > Add MIG_RP_MSG_REQ_PAGES command on Return path for the postcopy > destination to request a page from the source. > > Signed-off-by: Dr. David Alan Gilbert > --- > include/migration/migration.h | 4 +++ > migration/migration.c | 70 +++++++++++++++++++++++++++++++++++++++++++ > trace-events | 1 + > 3 files changed, 75 insertions(+) > > diff --git a/include/migration/migration.h b/include/migration/migration.h > index 68a1731..8742d53 100644 > --- a/include/migration/migration.h > +++ b/include/migration/migration.h > @@ -47,6 +47,8 @@ enum mig_rp_message_type { > MIG_RP_MSG_INVALID = 0, /* Must be 0 */ > MIG_RP_MSG_SHUT, /* sibling will not send any more RP messages */ > MIG_RP_MSG_PONG, /* Response to a PING; data (seq: be32 ) */ > + > + MIG_RP_MSG_REQ_PAGES, /* data (start: be64, len: be64) */ Not that I really care, buht I think that leng could be 32bits. I am not seing networking getting good at multigigabytes transfers soon O:-) > +void migrate_send_rp_req_pages(MigrationIncomingState *mis, const char* rbname, > + ram_addr_t start, ram_addr_t len); Shouldn't len be a size_t? (yes, I know that migration code is not really consistent about that) > void ram_control_before_iterate(QEMUFile *f, uint64_t flags); > void ram_control_after_iterate(QEMUFile *f, uint64_t flags); > diff --git a/migration/migration.c b/migration/migration.c > index 3e5a7c8..0373b77 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -113,6 +113,36 @@ static void deferred_incoming_migration(Error **errp) > deferred_incoming = true; > } > > +/* Request a range of pages from the source VM at the given > + * start address. > + * rbname: Name of the RAMBlock to request the page in, if NULL it's the same > + * as the last request (a name must have been given previously) > + * Start: Address offset within the RB > + * Len: Length in bytes required - must be a multiple of pagesize > + */ > +void migrate_send_rp_req_pages(MigrationIncomingState *mis, const char *rbname, > + ram_addr_t start, ram_addr_t len) > +{ > + uint8_t bufc[16+1+255]; /* start (8 byte), len (8 byte), rbname upto 256 */ > + uint64_t *buf64 = (uint64_t *)bufc; > + size_t msglen = 16; /* start + len */ > + > + assert(!(len & 1)); ohhhh, why can't we get a real flags field? Scratch that. Seeing the rest of the code, can't we have two commands: MIG_RP_MSG_REQ_PAGES MIG_RP_MSG_REQ_PAGES_WITH_ID I am not really sure that it makes sense getting a command that can be of two different lengths only for that? I am not sure, but having a command with two different payloads look strange. Later, Juan.