From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38663) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZNN84-0004Q9-99 for qemu-devel@nongnu.org; Thu, 06 Aug 2015 11:30:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZNN80-0001uL-Qj for qemu-devel@nongnu.org; Thu, 06 Aug 2015 11:30:48 -0400 Received: from mx1.redhat.com ([209.132.183.28]:59456) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZNN80-0001tw-DX for qemu-devel@nongnu.org; Thu, 06 Aug 2015 11:30:44 -0400 Date: Thu, 6 Aug 2015 15:15:40 +0100 From: "Dr. David Alan Gilbert" Message-ID: <20150806141540.GE2251@work-vm> References: <1434450415-11339-1-git-send-email-dgilbert@redhat.com> <1434450415-11339-31-git-send-email-dgilbert@redhat.com> <87zj30i410.fsf@neno.neno> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87zj30i410.fsf@neno.neno> Subject: Re: [Qemu-devel] [PATCH v7 30/42] Page request: Add MIG_RP_MSG_REQ_PAGES reverse command List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Juan Quintela 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 * Juan Quintela (quintela@redhat.com) wrote: > "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:-) Done. > > +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) Done (fun combination with the change above, but still) > > 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. Done (I made it _ID rather than WITH_ID - it was getting a bit long). Dave > > Later, Juan. -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK