From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38422) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YZuTE-0004co-Jw for qemu-devel@nongnu.org; Mon, 23 Mar 2015 01:00:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YZuT5-0006IH-2D for qemu-devel@nongnu.org; Mon, 23 Mar 2015 01:00:12 -0400 Received: from ozlabs.org ([103.22.144.67]:41286) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YZuT4-0006Cf-G6 for qemu-devel@nongnu.org; Mon, 23 Mar 2015 01:00:02 -0400 Date: Mon, 23 Mar 2015 16:00:50 +1100 From: David Gibson Message-ID: <20150323050050.GQ25043@voom.fritz.box> References: <1424883128-9841-1-git-send-email-dgilbert@redhat.com> <1424883128-9841-33-git-send-email-dgilbert@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="3wfpuDtTLg8/Vq6g" Content-Disposition: inline In-Reply-To: <1424883128-9841-33-git-send-email-dgilbert@redhat.com> Subject: Re: [Qemu-devel] [PATCH v5 32/45] Page request: Add MIG_RP_CMD_REQ_PAGES reverse command 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, quintela@redhat.com, qemu-devel@nongnu.org, amit.shah@redhat.com, pbonzini@redhat.com, yanghy@cn.fujitsu.com --3wfpuDtTLg8/Vq6g Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Feb 25, 2015 at 04:51:55PM +0000, Dr. David Alan Gilbert (git) wrot= e: > From: "Dr. David Alan Gilbert" >=20 > Add MIG_RP_CMD_REQ_PAGES command on Return path for the postcopy > destination to request a page from the source. >=20 > Signed-off-by: Dr. David Alan Gilbert > --- > include/migration/migration.h | 4 +++ > migration/migration.c | 70 +++++++++++++++++++++++++++++++++++++= ++++++ > trace-events | 1 + > 3 files changed, 75 insertions(+) >=20 > diff --git a/include/migration/migration.h b/include/migration/migration.h > index 2c607e7..2c15d63 100644 > --- a/include/migration/migration.h > +++ b/include/migration/migration.h > @@ -46,6 +46,8 @@ enum mig_rpcomm_cmd { > MIG_RP_CMD_INVALID =3D 0, /* Must be 0 */ > MIG_RP_CMD_SHUT, /* sibling will not send any more RP messag= es */ > MIG_RP_CMD_PONG, /* Response to a PING; data (seq: be32 ) */ > + > + MIG_RP_CMD_REQ_PAGES, /* data (start: be64, len: be64) */ > }; > =20 > /* Postcopy page-map-incoming - data about each page on the inbound side= */ > @@ -253,6 +255,8 @@ void migrate_send_rp_shut(MigrationIncomingState *mis, > uint32_t value); > void migrate_send_rp_pong(MigrationIncomingState *mis, > uint32_t value); > +void migrate_send_rp_req_pages(MigrationIncomingState *mis, const char* = rbname, > + ram_addr_t start, ram_addr_t len); > =20 > 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 bd066f6..2e9d0dd 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -138,6 +138,36 @@ void migrate_send_rp_pong(MigrationIncomingState *mi= s, > migrate_send_rp_message(mis, MIG_RP_CMD_PONG, 4, (uint8_t *)&buf); > } > =20 > +/* 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 t= he 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 =3D (uint64_t *)bufc; > + size_t msglen =3D 16; /* start + len */ > + > + assert(!(len & 1)); > + if (rbname) { > + int rbname_len =3D strlen(rbname); > + assert(rbname_len < 256); > + > + len |=3D 1; /* Flag to say we've got a name */ > + bufc[msglen++] =3D rbname_len; > + memcpy(bufc + msglen, rbname, rbname_len); > + msglen +=3D rbname_len; > + } > + > + buf64[0] =3D cpu_to_be64((uint64_t)start); > + buf64[1] =3D cpu_to_be64((uint64_t)len); > + migrate_send_rp_message(mis, MIG_RP_CMD_REQ_PAGES, msglen, bufc); So.. what's the reason we actually need ramblock names on the wire, rather than working purely from GPAs? It occurs to me that referencing ramblock names from the wire protocol exposes something that's kind of an internal detail, and may limit our options for reworking the memory subsystem in future. > +} > + > void qemu_start_incoming_migration(const char *uri, Error **errp) > { > const char *p; > @@ -789,6 +819,17 @@ static void source_return_path_bad(MigrationState *s) > } > =20 > /* > + * Process a request for pages received on the return path, > + * We're allowed to send more than requested (e.g. to round to our page = size) > + * and we don't need to send pages that have already been sent. > + */ > +static void migrate_handle_rp_req_pages(MigrationState *ms, const char* = rbname, > + ram_addr_t start, ram_addr_t len) > +{ > + trace_migrate_handle_rp_req_pages(start, len); > +} > + > +/* > * Handles messages sent on the return path towards the source VM > * > */ > @@ -800,6 +841,8 @@ static void *source_return_path_thread(void *opaque) > const int max_len =3D 512; > uint8_t buf[max_len]; > uint32_t tmp32; > + ram_addr_t start, len; > + char *tmpstr; > int res; > =20 > trace_source_return_path_thread_entry(); > @@ -815,6 +858,11 @@ static void *source_return_path_thread(void *opaque) > expected_len =3D 4; > break; > =20 > + case MIG_RP_CMD_REQ_PAGES: > + /* 16 byte start/len _possibly_ plus an id str */ > + expected_len =3D 16 + 256; Isn't that the maximum length, rather than the minimum or typical length? > + break; > + > default: > error_report("RP: Received invalid cmd 0x%04x length 0x%04x", > header_com, header_len); > @@ -860,6 +908,28 @@ static void *source_return_path_thread(void *opaque) > trace_source_return_path_thread_pong(tmp32); > break; > =20 > + case MIG_RP_CMD_REQ_PAGES: > + start =3D be64_to_cpup((uint64_t *)buf); > + len =3D be64_to_cpup(((uint64_t *)buf)+1); > + tmpstr =3D NULL; > + if (len & 1) { > + len -=3D 1; /* Remove the flag */ > + /* Now we expect an idstr */ > + tmp32 =3D buf[16]; /* Length of the following idstr */ > + tmpstr =3D (char *)&buf[17]; > + buf[17+tmp32] =3D '\0'; > + expected_len =3D 16+1+tmp32; > + } else { > + expected_len =3D 16; Ah.. so expected_len is changed here. But then what was the point of setting it in the earlier switch? > + } > + if (header_len !=3D expected_len) { > + error_report("RP: Req_Page with length %d expecting %d", > + header_len, expected_len); > + source_return_path_bad(ms); > + } > + migrate_handle_rp_req_pages(ms, tmpstr, start, len); > + break; > + > default: > /* This shouldn't happen because we should catch this above = */ > trace_source_return_path_bad_header_com(); > diff --git a/trace-events b/trace-events > index bcbdef8..9bedee4 100644 > --- a/trace-events > +++ b/trace-events > @@ -1404,6 +1404,7 @@ migrate_fd_error(void) "" > migrate_fd_cancel(void) "" > migrate_pending(uint64_t size, uint64_t max, uint64_t post, uint64_t non= post) "pending size %" PRIu64 " max %" PRIu64 " (post=3D%" PRIu64 " nonpost= =3D%" PRIu64 ")" > migrate_send_rp_message(int cmd, uint16_t len) "cmd=3D%d, len=3D%d" > +migrate_handle_rp_req_pages(size_t start, size_t len) "at %zx for len %z= x" > migration_thread_after_loop(void) "" > migration_thread_file_err(void) "" > migration_thread_setup_complete(void) "" --=20 David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson --3wfpuDtTLg8/Vq6g Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJVD54CAAoJEGw4ysog2bOSE/0P/Re1mwQiGaWbV6lOCVEdtxnf 8YLhTo2LCC8xaieugtdeKSmWSFGhtvqpWvjc9q33KZpH4GUg82bH51Adt5LvqsuD MOUx3CTG0TSrlHCLnQk5NDLZn+ymin9jmrNd0CjqPK/ivjInQm0mSRDGsYnLQtQE bUFq2uPlj9JpBJEBj0yqccOnAEjWa3PTg3AVc9Cn0YIqS/wIFcKEVuzt7MKec1Cs YPsgh+UIWwPjuGy+MIWWRKLPrpmKnVEAL1XIJXYSOwW1DLa649q7YohHxkaVNfKh sk9cGDlRlW/mN8cys0KMuH0dCD4TcH5hC77IjxJ5j963ivVmJxyvqhfmHqZOvG7s zgC992W0KXQ7kWyZ2bcvIlFTIkMLl3ahaqz99j9WqcQ25aOw5unBNHsXfbC0fxsl r1moxZ7ZqIV8qEn+BSMTr7ZUG2NoH7wwlkBx7IsOIXPmIgeB7zNmDKkJvTB2hRaX GpdO+yjaf9LUbzrN80h3DATZnoxDE37/fi5517/3XbaDFU0ytyZFk4Y1plahiULU xRxx/+uy4kY22ZTJ07/vkpwXpOtNjqzqsTQqIZf73XnHAlCc5fNqoQpeIbAaCc+e idbDHG72DhJ9aSSyV0HdXDTzrBuEifPH+DbDlPacrwogUTiFt11/WLTjY5jdJ/Mx XU6TH0nN4UACXEYc6y7H =8cC2 -----END PGP SIGNATURE----- --3wfpuDtTLg8/Vq6g--